Subject: Re: MIPS shared linker running at virtual address 0!
To: Matthias Drochner <M.Drochner@fz-juelich.de>
From: Simon Burge <simonb@wasabisystems.com>
List: tech-kern
Date: 10/06/2005 02:55:58
Bah, I've totally flaked out on this.

On Fri, Jul 29, 2005 at 08:42:49PM +0200, Matthias Drochner wrote:

> simonb@wasabisystems.com said:
> > I'll ignore comments along the lines of "eww, that's ugly"
> 
> eew, that's ugly:-)
> It seems that I'm the last person who touched this stuff
> (2 years ago), so I should feel responsible.
> 
> > +	 * Evil hack:  Only MIPS should be non-relocatable, and the
> > [...]
> > +	if (*last == ELF_LINK_ADDR && (ph->p_vaddr & 0xffff0000) == 0)
> 
> In principle, that patch is not too bad. Just that if this
> applies to MIPS only, I'd also put an "#ifdef __mips__" or
> so around. In particular, the p_vaddr check is mips specific.

The p_vaddr check is just something approximating "is this a low
address".  It's not really MIPS specific.

> And here I have a little concern: The patch just tests the first
> program header, no matter whether PT_LOAD or not. If I look at
> a -current ld.elf_so:
> 
> $ objdump -p /usr/libexec/ld.elf_so
> /usr/libexec/ld.elf_so:     file format elf32-tradlittlemips
> Program Header:
> 0x70000000 off    0x000000d4 vaddr 0x000000d4 paddr 0x000000d4 align 2**2
>          filesz 0x00000018 memsz 0x00000018 flags r--
>     LOAD off    0x00000000 vaddr 0x00000000 paddr 0x00000000 align 2**16
>          filesz 0x0000d918 memsz 0x0000d918 flags r-x
>     LOAD off    0x0000d920 vaddr 0x0004d920 paddr 0x0004d920 align 2**16
>          filesz 0x00000c28 memsz 0x00001610 flags rw-
>  DYNAMIC off    0x000000ec vaddr 0x000000ec paddr 0x000000ec align 2**2
>          filesz 0x00002107 memsz 0x00002107 flags rwx
>     NOTE off    0x0000d900 vaddr 0x0000d900 paddr 0x0000d900 align 2**2
>          filesz 0x00000018 memsz 0x00000018 flags r--
> 
> There is a non-PT_LOAD section at the beginning.
> 
> It might be cleaner to do the address check after the phdrs are
> walked through, a couple of lines later. Of course the "if" at
> the beginning would have to be something like
> 
> [ ... ]

I've not had a chance to investigate this path yet.  For now, I'd like
to propose that we use the current patch for a couple of reasons:

 1- It's been well tested.  I've been using this patch in a source tree
    that is used to build for multiple architectures (although it
    obviously only affects MIPS) since the start of the year.  It's been
    tested on MIPS boxes both with and without COMPAT_16 and works as
    expected.

 2- The NetBSD 3.0 release is approaching, and there's no better
    solution available (and tested!).

 3- I'm also concerned that a "better" solution will end up looking
    even worse than the current patch :-/

I can add to the comment block that we should be testing the first
loadable program header, and not just the first one in the file.

You also suggested using "#ifdef __mips__" around the patch.  It's worth
noting that the ELF_INTERP_NON_RELOCATABLE in really a mips-only patch
and probably is named that way to stop having an explicit mips check in
MI code.  It's _extremely_ unlikely that any other architecture will
ever use it.

Apologies again for the delay in following up on this.

Simon.
--
Simon Burge                                   <simonb@wasabisystems.com>
NetBSD Development, Support and Service:   http://www.wasabisystems.com/