Subject: compatibility and emulation fixes for exec_elf
To: None <tech-kern@netbsd.org, port-mips@netbsd.org>
From: Matthias Drochner <M.Drochner@fz-juelich.de>
List: port-mips
Date: 10/28/2003 20:08:48
This is a multipart MIME message.

--==_Exmh_46410829694750
Content-Type: text/plain; charset=us-ascii


Hi -
the ELF_INTERP_NON_RELOCATABLE case in kern/exec_elf32.c
is broken. Here is a plan how this could be fixed and some
ideas for further improvement:

We have 3 cases for mapping shared library loaders:
1. it is relocatable; we can put it anywhere
2. it must be loaded at the address it was linked for
3. the emulation has an idea where to place it

(1) is the default case, it should have the shortest code path
(2) is needed in the native case for mips && COMPAT_16
(unfortunately, also -current on mips is affected if COMPAT_16 is
defined; this could eg be fixed by appending a version number to
ld.elf_so)

COMPAT_IRIX does (3) by filling in the return argument in the
elf_probe_func return argument. Looking at the code, this seems to
do the same like (2), only at a higher cost.

ELF_INTERP_NON_RELOCATABLE, as implemented currently, just doesn't
work. For me, it seems completely wrong to have these #ifdefs in
elf_makecmds() -- it should be emulation dependant.
I'll append a patch which fixes it. (Before committing, I'd replace
the magic "-2" by a macro...)

But there is room for more improvement:
-if my assumption about COMPAT_IRIX is correct, irix_load_addr()
 can be nuked
-since (1) above (the NO_ADDR case) is default, we could communicate
 this from the "probe" function to "elf_makecmds()" by just leaving
 the return pointer alone, saving code and cycles
-In principle, case (3) above wouldn't be needed anymore, and the
 address return argument to the "probe" function could be replaced
 by just a flag bit. Wouldn't gain much however, so we could as well
 leave the "vaddr *", maybe it will be needed in the future...
-There is no need to allocate the "interp" pointer in elf_makecmds()
 if no PT_INTERP is present, speeding up the static binary case.
 The "if (itp[0])" tests in the emulation's "probe" functions would
 change to "if (itp)" then - also some small gain.

Comments?

best regards
Matthias



--==_Exmh_46410829694750
Content-Type: text/plain ; name="elf.txt"; charset=us-ascii
Content-Description: elf.txt
Content-Disposition: attachment; filename="elf.txt"

--- exec_elf32.c.~1.94.~	Mon Aug 11 12:28:42 2003
+++ exec_elf32.c	Tue Oct 28 18:38:29 2003
@@ -399,8 +399,6 @@ ELFNAME(load_file)(struct proc *p, struc
 	if ((error = exec_read_from(p, vp, eh.e_phoff, ph, phsize)) != 0)
 		goto bad;
 
-	/* this breaks on, e.g., OpenBSD-compatible mips shared binaries. */
-#ifndef ELF_INTERP_NON_RELOCATABLE
 	/*
 	 * If no position to load the interpreter was set by a probe
 	 * function, pick the same address that a non-fixed mmap(0, ..)
@@ -429,7 +427,6 @@ ELFNAME(load_file)(struct proc *p, struc
 		addr = VM_DEFAULT_ADDRESS(epp->ep_daddr,
 		    round_page(limit) - trunc_page(base_ph->p_vaddr));
 	} else
-#endif	/* !ELF_INTERP_NON_RELOCATABLE */
 		addr = *last;
 
 	/*
@@ -452,6 +449,8 @@ ELFNAME(load_file)(struct proc *p, struc
 				 */
 				base_ph = ph0;
 				flags = VMCMD_BASE;
+				if (addr == (Elf_Addr)-2)
+					addr = ph0->p_vaddr;
 				if (p->p_vmspace->vm_map.flags & VM_MAP_TOPDOWN)
 					addr = ELF_TRUNC(addr, ph0->p_align);
 				else
@@ -779,6 +778,10 @@ ELFNAME2(netbsd,probe)(struct proc *p, s
 
 	if ((error = ELFNAME2(netbsd,signature)(p, epp, eh)) != 0)
 		return error;
+#ifdef ELF_INTERP_NON_RELOCATABLE
+	*pos = (vaddr_t)-2;
+#else
 	*pos = ELFDEFNNAME(NO_ADDR);
+#endif
 	return 0;
 }

--==_Exmh_46410829694750--