| Message ID | 20100423002659.199E0101C@are.twiddle.net |
|---|---|
| State | New |
| Headers | show |
On 4/23/10, Richard Henderson <rth@twiddle.net> wrote: > The ABI-specific types used by linux_binprm and image_info > are different after forcing TARGET_ABI32 on. Which means > that the parameters that load_elf_binary_multi sees are not > those that loader_exec passed. This is inherently broken > and is more trouble than it's worth fixing. Nack. How is this inherently broken? The problem that elfload32 solves is that the CPU is 64 bit, but the ABI and the binaries loaded are still 32 bits. It works nicely for sparc32plus binaries (ELFCLASS32, but only for V9 CPUs). > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > Makefile.target | 1 - > configure | 6 ------ > linux-user/elfload32.c | 30 ------------------------------ > linux-user/linuxload.c | 6 +----- > linux-user/qemu.h | 5 ----- > 5 files changed, 1 insertions(+), 47 deletions(-) > delete mode 100644 linux-user/elfload32.c > > diff --git a/Makefile.target b/Makefile.target > index 5897051..65beed5 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -88,7 +88,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \ > elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o > > obj-$(TARGET_HAS_BFLT) += flatload.o > -obj-$(TARGET_HAS_ELFLOAD32) += elfload32.o > > obj-$(TARGET_I386) += vm86.o > > diff --git a/configure b/configure > index 7c06719..e2dadb0 100755 > --- a/configure > +++ b/configure > @@ -2413,7 +2413,6 @@ ln -s $source_path/Makefile.target $target_dir/Makefile > echo "# Automatically generated by configure - do not modify" > $config_target_mak > > bflt="no" > -elfload32="no" > target_nptl="no" > interp_prefix1=`echo "$interp_prefix" | sed "s/%M/$target_arch2/g"` > echo "CONFIG_QEMU_PREFIX=\"$interp_prefix1\"" >> $config_target_mak > @@ -2510,7 +2509,6 @@ case "$target_arch2" in > ;; > sparc64) > TARGET_BASE_ARCH=sparc > - elfload32="yes" > target_phys_bits=64 > ;; > sparc32plus) > @@ -2609,10 +2607,6 @@ if test "$target_user_only" = "yes" \ > -a "$nptl" = "yes" -a "$target_nptl" = "yes"; then > echo "CONFIG_USE_NPTL=y" >> $config_target_mak > fi > -# 32 bit ELF loader in addition to native 64 bit loader? > -if test "$target_user_only" = "yes" -a "$elfload32" = "yes"; then > - echo "TARGET_HAS_ELFLOAD32=y" >> $config_target_mak > -fi > if test "$target_user_only" = "yes" -a "$guest_base" = "yes"; then > echo "CONFIG_USE_GUEST_BASE=y" >> $config_target_mak > fi > diff --git a/linux-user/elfload32.c b/linux-user/elfload32.c > deleted file mode 100644 > index 4b4648c..0000000 > --- a/linux-user/elfload32.c > +++ /dev/null > @@ -1,30 +0,0 @@ > -#define TARGET_ABI32 > -#define load_elf_binary load_elf_binary32 > -#define do_init_thread do_init_thread32 > - > -#include "elfload.c" > - > -#undef load_elf_binary > -#undef do_init_thread > - > -int load_elf_binary(struct linux_binprm *bprm, struct target_pt_regs *regs, > - struct image_info *info); > - > -int load_elf_binary_multi(struct linux_binprm *bprm, > - struct target_pt_regs *regs, > - struct image_info *info) > -{ > - struct elfhdr *elf_ex; > - int retval; > - > - elf_ex = (struct elfhdr *) bprm->buf; /* exec-header */ > - if (elf_ex->e_ident[EI_CLASS] == ELFCLASS64) { > - retval = load_elf_binary(bprm, regs, info); > - } else { > - retval = load_elf_binary32(bprm, regs, info); > - if (personality(info->personality) == PER_LINUX) > - info->personality = PER_LINUX32; > - } > - > - return retval; > -} > diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c > index 2d778a2..13ad9aa 100644 > --- a/linux-user/linuxload.c > +++ b/linux-user/linuxload.c > @@ -184,11 +184,7 @@ int loader_exec(const char * filename, char ** argv, char ** envp, > && bprm->buf[1] == 'E' > && bprm->buf[2] == 'L' > && bprm->buf[3] == 'F') { > -#ifndef TARGET_HAS_ELFLOAD32 > - retval = load_elf_binary(bprm,regs,infop); > -#else > - retval = load_elf_binary_multi(bprm, regs, infop); > -#endif > + retval = load_elf_binary(bprm, regs, infop); > #if defined(TARGET_HAS_BFLT) > } else if (bprm->buf[0] == 'b' > && bprm->buf[1] == 'F' > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index 47fc686..dab3597 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -171,11 +171,6 @@ int load_elf_binary(struct linux_binprm * bprm, struct target_pt_regs * regs, > struct image_info * info); > int load_flt_binary(struct linux_binprm * bprm, struct target_pt_regs * regs, > struct image_info * info); > -#ifdef TARGET_HAS_ELFLOAD32 > -int load_elf_binary_multi(struct linux_binprm *bprm, > - struct target_pt_regs *regs, > - struct image_info *info); > -#endif > > abi_long memcpy_to_target(abi_ulong dest, const void *src, > unsigned long len); > > -- > 1.6.6.1 > >
On 04/23/2010 11:36 AM, Blue Swirl wrote: > On 4/23/10, Richard Henderson <rth@twiddle.net> wrote: >> The ABI-specific types used by linux_binprm and image_info >> are different after forcing TARGET_ABI32 on. Which means >> that the parameters that load_elf_binary_multi sees are not >> those that loader_exec passed. This is inherently broken >> and is more trouble than it's worth fixing. > > Nack. How is this inherently broken? sizeof(abi_ulong) is different in elfload32.c and linuxload.c, which means the two files cannot communicate with any type affected by this change. Which is both linux_binprm and image_info. > The problem that elfload32 solves is that the CPU is 64 bit, but the > ABI and the binaries loaded are still 32 bits. It works nicely for > sparc32plus binaries (ELFCLASS32, but only for V9 CPUs). And yet we have a separate sparc32plus-linux-user/qemu-sparc32plus binary that does that job. Do we really need qemu-sparc64 to do both jobs? Because it doesn't. The only thing that happens is that qemu crashes immediately because it sees linux_binprm.e_gid at the offset it expects to see linux_binprm.argc, and fails to copy gid=rth(5000) entries from the argv array. r~
On 4/25/10, Richard Henderson <rth@twiddle.net> wrote: > On 04/23/2010 11:36 AM, Blue Swirl wrote: > > On 4/23/10, Richard Henderson <rth@twiddle.net> wrote: > >> The ABI-specific types used by linux_binprm and image_info > >> are different after forcing TARGET_ABI32 on. Which means > >> that the parameters that load_elf_binary_multi sees are not > >> those that loader_exec passed. This is inherently broken > >> and is more trouble than it's worth fixing. > > > > Nack. How is this inherently broken? > > > sizeof(abi_ulong) is different in elfload32.c and linuxload.c, > which means the two files cannot communicate with any type > affected by this change. Which is both linux_binprm and image_info. > > > > The problem that elfload32 solves is that the CPU is 64 bit, but the > > ABI and the binaries loaded are still 32 bits. It works nicely for > > sparc32plus binaries (ELFCLASS32, but only for V9 CPUs). > > > And yet we have a separate sparc32plus-linux-user/qemu-sparc32plus > binary that does that job. > > Do we really need qemu-sparc64 to do both jobs? Because it doesn't. > The only thing that happens is that qemu crashes immediately because > it sees linux_binprm.e_gid at the offset it expects to see > linux_binprm.argc, and fails to copy gid=rth(5000) entries from the > argv array. I see. Thanks, applied.
diff --git a/Makefile.target b/Makefile.target index 5897051..65beed5 100644 --- a/Makefile.target +++ b/Makefile.target @@ -88,7 +88,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \ elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o obj-$(TARGET_HAS_BFLT) += flatload.o -obj-$(TARGET_HAS_ELFLOAD32) += elfload32.o obj-$(TARGET_I386) += vm86.o diff --git a/configure b/configure index 7c06719..e2dadb0 100755 --- a/configure +++ b/configure @@ -2413,7 +2413,6 @@ ln -s $source_path/Makefile.target $target_dir/Makefile echo "# Automatically generated by configure - do not modify" > $config_target_mak bflt="no" -elfload32="no" target_nptl="no" interp_prefix1=`echo "$interp_prefix" | sed "s/%M/$target_arch2/g"` echo "CONFIG_QEMU_PREFIX=\"$interp_prefix1\"" >> $config_target_mak @@ -2510,7 +2509,6 @@ case "$target_arch2" in ;; sparc64) TARGET_BASE_ARCH=sparc - elfload32="yes" target_phys_bits=64 ;; sparc32plus) @@ -2609,10 +2607,6 @@ if test "$target_user_only" = "yes" \ -a "$nptl" = "yes" -a "$target_nptl" = "yes"; then echo "CONFIG_USE_NPTL=y" >> $config_target_mak fi -# 32 bit ELF loader in addition to native 64 bit loader? -if test "$target_user_only" = "yes" -a "$elfload32" = "yes"; then - echo "TARGET_HAS_ELFLOAD32=y" >> $config_target_mak -fi if test "$target_user_only" = "yes" -a "$guest_base" = "yes"; then echo "CONFIG_USE_GUEST_BASE=y" >> $config_target_mak fi diff --git a/linux-user/elfload32.c b/linux-user/elfload32.c deleted file mode 100644 index 4b4648c..0000000 --- a/linux-user/elfload32.c +++ /dev/null @@ -1,30 +0,0 @@ -#define TARGET_ABI32 -#define load_elf_binary load_elf_binary32 -#define do_init_thread do_init_thread32 - -#include "elfload.c" - -#undef load_elf_binary -#undef do_init_thread - -int load_elf_binary(struct linux_binprm *bprm, struct target_pt_regs *regs, - struct image_info *info); - -int load_elf_binary_multi(struct linux_binprm *bprm, - struct target_pt_regs *regs, - struct image_info *info) -{ - struct elfhdr *elf_ex; - int retval; - - elf_ex = (struct elfhdr *) bprm->buf; /* exec-header */ - if (elf_ex->e_ident[EI_CLASS] == ELFCLASS64) { - retval = load_elf_binary(bprm, regs, info); - } else { - retval = load_elf_binary32(bprm, regs, info); - if (personality(info->personality) == PER_LINUX) - info->personality = PER_LINUX32; - } - - return retval; -} diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c index 2d778a2..13ad9aa 100644 --- a/linux-user/linuxload.c +++ b/linux-user/linuxload.c @@ -184,11 +184,7 @@ int loader_exec(const char * filename, char ** argv, char ** envp, && bprm->buf[1] == 'E' && bprm->buf[2] == 'L' && bprm->buf[3] == 'F') { -#ifndef TARGET_HAS_ELFLOAD32 - retval = load_elf_binary(bprm,regs,infop); -#else - retval = load_elf_binary_multi(bprm, regs, infop); -#endif + retval = load_elf_binary(bprm, regs, infop); #if defined(TARGET_HAS_BFLT) } else if (bprm->buf[0] == 'b' && bprm->buf[1] == 'F' diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 47fc686..dab3597 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -171,11 +171,6 @@ int load_elf_binary(struct linux_binprm * bprm, struct target_pt_regs * regs, struct image_info * info); int load_flt_binary(struct linux_binprm * bprm, struct target_pt_regs * regs, struct image_info * info); -#ifdef TARGET_HAS_ELFLOAD32 -int load_elf_binary_multi(struct linux_binprm *bprm, - struct target_pt_regs *regs, - struct image_info *info); -#endif abi_long memcpy_to_target(abi_ulong dest, const void *src, unsigned long len);
The ABI-specific types used by linux_binprm and image_info are different after forcing TARGET_ABI32 on. Which means that the parameters that load_elf_binary_multi sees are not those that loader_exec passed. This is inherently broken and is more trouble than it's worth fixing. Signed-off-by: Richard Henderson <rth@twiddle.net> --- Makefile.target | 1 - configure | 6 ------ linux-user/elfload32.c | 30 ------------------------------ linux-user/linuxload.c | 6 +----- linux-user/qemu.h | 5 ----- 5 files changed, 1 insertions(+), 47 deletions(-) delete mode 100644 linux-user/elfload32.c