Patchwork linux-user: Remove ELFLOAD32.

login
register
mail settings
Submitter Richard Henderson
Date April 23, 2010, 12:24 a.m.
Message ID <20100423002659.199E0101C@are.twiddle.net>
Download mbox | patch
Permalink /patch/50773/
State New
Headers show

Comments

Richard Henderson - April 23, 2010, 12:24 a.m.
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
Blue Swirl - April 23, 2010, 6:36 p.m.
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
>
>
Richard Henderson - April 25, 2010, 12:36 a.m.
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~
Blue Swirl - April 25, 2010, 3:08 p.m.
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.

Patch

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);