On Mon, 2007-10-01 at 04:35 +0100, Thiemo Seufer wrote:
> J. Mayer wrote:
> > On Sun, 2007-09-30 at 17:09 +0200, J. Mayer wrote:
> > > On Sun, 2007-09-30 at 14:38 +0100, Thiemo Seufer wrote:
> > > > J. Mayer wrote:
> > > > > Following what I've done in the syscalls emulation routines, it 
> > > > > appeared
> > > > > to me that there seems to be a lot of confusions between host and 
> > > > > target
> > > > > long in the ELF loader.
> > > > 
> > > > But the ELF fields are tied to the ELFCLASS of the supported ABI, not to
> > > > the register width of the machine emulation. If anything they should
> > > > become the ELF types.
> > > > 
> > > > (Your approach will e.g. break down for MIPS N32, where "long" is 
> > > > smaller
> > > > thant the register width, and the ABI uses ELFCLASS32.)
> > > 
> > > OK, I will try to rework this.
> > 
> > I did check in the linux kernel and it appears that all variables I
> > changed from unsigned long to target_ulong seem to be unsigned long in the
> > kernel. This looks fine for me as they are addresses in the process virtual 
> > address space, 
> > so they should fit in a target_ulong, whatever the target registers size 
> > could be.
> 
> As long as eventual sign extensions are done properly this is ok.
> (Some places in the kernel have casts to elf_greg_t for that purpose.)
> 
> > What seems bugged to me (but I did not make any change in that part) is
> > the auxiliary vectors generation. This seems to me to be the only place
> > where Linux explicitelly uses elf_addr_t so Qemu should do the same.
> > Then, it seems that my patch is not so bad and should not break anything
> > but is not complete as it does not fix the auxiliary vectors.
> > 
> > Do you agree with this, or did I miss something else ?

Following this discussion, I reworked my patch a little bit in order to
use the elf_addr_t type for the aux infos creation. It appears to me
that it functionally changes nothing as elf_addr_t is defined to be
Elf32_OFf / Elf64_Off. But this makes the elf_create_tables function use
the same types as the Linux kernel one.

> The kernel loader (fs/binfmt_elf.c) is somewhat geared towards single
> ABI architectures. PowerPC, MIPS, SPARC etc. have all their own kludge
> (in arch/$ARCH/kernel/binfmt_elf*.c) to redefine some essential bits
> and then #include fs/binfmt_elf.c.
> 
> The whole thing looks a bit scary, I'm not sure we should import it in
> Qemu. OTOH it is generally a good idea to stay in sync with the kernel
> implementation.

The thing I see is different is that the n32 ABI redefines elf_greg_t
and elf_caddr_t as 32 bits. Maybe I missed something but those types
seem not to be used by the ELF loader (or maybe I should look in a more
recent kernel ;-) ).
Then, I have seen no apparent issue with the patch and I'm quite sure
that, even if it's not correct for some specific ABI, it would bring no
regression.

The only point where there could be an evident regression is for the ARM
target:
regs->ARM_r10 = infop->start_data;
As start_data were not computed the same way as it's done in the Linux
kernel, I did change this. I hope this is not to break anything for ARM
emulation but I don't know too much of the ARM ABI so I would be glad if
some ones who know could tell me if what I did might really break ARM
binaries registers initialisation.

-- 
J. Mayer <[EMAIL PROTECTED]>
Never organized
Index: elf.h
===================================================================
RCS file: /sources/qemu/qemu/elf.h,v
retrieving revision 1.10
diff -u -d -d -p -r1.10 elf.h
--- elf.h	16 Sep 2007 21:07:49 -0000	1.10
+++ elf.h	4 Oct 2007 03:12:51 -0000
@@ -1130,6 +1130,7 @@ typedef struct elf64_note {
 #define elf_note	elf32_note
 #define elf_shdr	elf32_shdr
 #define elf_sym		elf32_sym
+#define elf_addr_t	Elf32_Off
 
 #ifdef ELF_USES_RELOCA
 # define ELF_RELOC      Elf32_Rela
@@ -1144,6 +1145,7 @@ typedef struct elf64_note {
 #define elf_note	elf64_note
 #define elf_shdr	elf64_shdr
 #define elf_sym		elf64_sym
+#define elf_addr_t	Elf64_Off
 
 #ifdef ELF_USES_RELOCA
 # define ELF_RELOC      Elf64_Rela
Index: linux-user/elfload.c
===================================================================
RCS file: /sources/qemu/qemu/linux-user/elfload.c,v
retrieving revision 1.48
diff -u -d -d -p -r1.48 elfload.c
--- linux-user/elfload.c	27 Sep 2007 04:10:43 -0000	1.48
+++ linux-user/elfload.c	4 Oct 2007 03:12:51 -0000
@@ -124,6 +124,7 @@ static inline void init_thread(struct ta
     /* XXX: it seems that r0 is zeroed after ! */
     regs->ARM_r0 = 0;
     /* For uClinux PIC binaries.  */
+    /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
     regs->ARM_r10 = infop->start_data;
 }
 
@@ -516,8 +517,8 @@ static void bswap_sym(struct elf_sym *sy
  * to be put directly into the top of new user memory.
  *
  */
-static unsigned long copy_elf_strings(int argc,char ** argv, void **page,
-                                      target_ulong p)
+static target_ulong copy_elf_strings(int argc,char ** argv, void **page,
+                                     target_ulong p)
 {
     char *tmp, *tmp1, *pag = NULL;
     int len, offset = 0;
@@ -566,8 +567,8 @@ static unsigned long copy_elf_strings(in
     return p;
 }
 
-unsigned long setup_arg_pages(target_ulong p, struct linux_binprm * bprm,
-					      struct image_info * info)
+target_ulong setup_arg_pages(target_ulong p, struct linux_binprm * bprm,
+                             struct image_info * info)
 {
     target_ulong stack_base, size, error;
     int i;
@@ -605,7 +606,7 @@ unsigned long setup_arg_pages(target_ulo
     return p;
 }
 
-static void set_brk(unsigned long start, unsigned long end)
+static void set_brk(target_ulong start, target_ulong end)
 {
 	/* page-align the start and end addresses... */
         start = HOST_PAGE_ALIGN(start);
@@ -624,9 +625,9 @@ static void set_brk(unsigned long start,
 /* We need to explicitly zero any fractional pages after the data
    section (i.e. bss).  This would contain the junk from the file that
    should not be in memory. */
-static void padzero(unsigned long elf_bss, unsigned long last_bss)
+static void padzero(target_ulong elf_bss, target_ulong last_bss)
 {
-        unsigned long nbyte;
+        target_ulong nbyte;
 
 	if (elf_bss >= last_bss)
 		return;
@@ -637,12 +638,12 @@ static void padzero(unsigned long elf_bs
            patch target_mmap(), but it is more complicated as the file
            size must be known */
         if (qemu_real_host_page_size < qemu_host_page_size) {
-            unsigned long end_addr, end_addr1;
+            target_ulong end_addr, end_addr1;
             end_addr1 = (elf_bss + qemu_real_host_page_size - 1) &
                 ~(qemu_real_host_page_size - 1);
             end_addr = HOST_PAGE_ALIGN(elf_bss);
             if (end_addr1 < end_addr) {
-                mmap((void *)end_addr1, end_addr - end_addr1,
+                mmap((void *)g2h(end_addr1), end_addr - end_addr1,
                      PROT_READ|PROT_WRITE|PROT_EXEC,
                      MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
             }
@@ -659,18 +660,18 @@ static void padzero(unsigned long elf_bs
 }
 
 
-static unsigned long create_elf_tables(target_ulong p, int argc, int envc,
-                                       struct elfhdr * exec,
-                                       unsigned long load_addr,
-                                       unsigned long load_bias,
-                                       unsigned long interp_load_addr, int ibcs,
-                                       struct image_info *info)
+static target_ulong create_elf_tables(target_ulong p, int argc, int envc,
+                                      struct elfhdr * exec,
+                                      target_ulong load_addr,
+                                      target_ulong load_bias,
+                                      target_ulong interp_load_addr, int ibcs,
+                                      struct image_info *info)
 {
         target_ulong sp;
         int size;
         target_ulong u_platform;
         const char *k_platform;
-        const int n = sizeof(target_ulong);
+        const int n = sizeof(elf_addr_t);
 
         sp = p;
         u_platform = 0;
@@ -697,10 +698,20 @@ static unsigned long create_elf_tables(t
         if (size & 15)
             sp -= 16 - (size & 15);
 
+        /* This is correct because Linux defines
+         * elf_addr_t as Elf32_Off / Elf64_Off
+         */
+#if ELF_CLASS == ELFCLASS32
 #define NEW_AUX_ENT(id, val) do { \
-            sp -= n; tputl(sp, val); \
-            sp -= n; tputl(sp, id); \
+            sp -= n; tput32(sp, val); \
+            sp -= n; tput32(sp, id); \
           } while(0)
+#else
+#define NEW_AUX_ENT(id, val) do { \
+            sp -= n; tput64(sp, val); \
+            sp -= n; tput64(sp, id); \
+          } while(0)
+#endif
         NEW_AUX_ENT (AT_NULL, 0);
 
         /* There must be exactly DLINFO_ITEMS entries here.  */
@@ -732,17 +743,17 @@ static unsigned long create_elf_tables(t
 }
 
 
-static unsigned long load_elf_interp(struct elfhdr * interp_elf_ex,
-				     int interpreter_fd,
-				     unsigned long *interp_load_addr)
+static target_ulong load_elf_interp(struct elfhdr * interp_elf_ex,
+                                    int interpreter_fd,
+                                    target_ulong *interp_load_addr)
 {
 	struct elf_phdr *elf_phdata  =  NULL;
 	struct elf_phdr *eppnt;
-	unsigned long load_addr = 0;
+	target_ulong load_addr = 0;
 	int load_addr_set = 0;
 	int retval;
-	unsigned long last_bss, elf_bss;
-	unsigned long error;
+	target_ulong last_bss, elf_bss;
+	target_ulong error;
 	int i;
 
 	elf_bss = 0;
@@ -818,8 +829,8 @@ static unsigned long load_elf_interp(str
 	  if (eppnt->p_type == PT_LOAD) {
 	    int elf_type = MAP_PRIVATE | MAP_DENYWRITE;
 	    int elf_prot = 0;
-	    unsigned long vaddr = 0;
-	    unsigned long k;
+	    target_ulong vaddr = 0;
+	    target_ulong k;
 
 	    if (eppnt->p_flags & PF_R) elf_prot =  PROT_READ;
 	    if (eppnt->p_flags & PF_W) elf_prot |= PROT_WRITE;
@@ -884,7 +895,7 @@ static unsigned long load_elf_interp(str
 	free(elf_phdata);
 
 	*interp_load_addr = load_addr;
-	return ((unsigned long) interp_elf_ex->e_entry) + load_addr;
+	return ((target_ulong) interp_elf_ex->e_entry) + load_addr;
 }
 
 /* Best attempt to load symbols from this ELF object. */
@@ -972,22 +983,22 @@ int load_elf_binary(struct linux_binprm 
     struct elfhdr interp_elf_ex;
     struct exec interp_ex;
     int interpreter_fd = -1; /* avoid warning */
-    unsigned long load_addr, load_bias;
+    target_ulong load_addr, load_bias;
     int load_addr_set = 0;
     unsigned int interpreter_type = INTERPRETER_NONE;
     unsigned char ibcs2_interpreter;
     int i;
-    unsigned long mapped_addr;
+    target_ulong mapped_addr;
     struct elf_phdr * elf_ppnt;
     struct elf_phdr *elf_phdata;
-    unsigned long elf_bss, k, elf_brk;
+    target_ulong elf_bss, k, elf_brk;
     int retval;
     char * elf_interpreter;
-    unsigned long elf_entry, interp_load_addr = 0;
+    target_ulong elf_entry, interp_load_addr = 0;
     int status;
-    unsigned long start_code, end_code, end_data;
-    unsigned long reloc_func_desc = 0;
-    unsigned long elf_stack;
+    target_ulong start_code, end_code, start_data, end_data;
+    target_ulong reloc_func_desc = 0;
+    target_ulong elf_stack;
     char passed_fileno[6];
 
     ibcs2_interpreter = 0;
@@ -1047,6 +1057,7 @@ int load_elf_binary(struct linux_binprm 
     elf_interpreter = NULL;
     start_code = ~0UL;
     end_code = 0;
+    start_data = 0;
     end_data = 0;
 
     for(i=0;i < elf_ex.e_phnum; i++) {
@@ -1180,9 +1191,9 @@ int load_elf_binary(struct linux_binprm 
     /* OK, This is the point of no return */
     info->end_data = 0;
     info->end_code = 0;
-    info->start_mmap = (unsigned long)ELF_START_MMAP;
+    info->start_mmap = (target_ulong)ELF_START_MMAP;
     info->mmap = 0;
-    elf_entry = (unsigned long) elf_ex.e_entry;
+    elf_entry = (target_ulong) elf_ex.e_entry;
 
     /* Do this so that we can load the interpreter, if need be.  We will
        change some of these later */
@@ -1199,7 +1210,7 @@ int load_elf_binary(struct linux_binprm 
     for(i = 0, elf_ppnt = elf_phdata; i < elf_ex.e_phnum; i++, elf_ppnt++) {
         int elf_prot = 0;
         int elf_flags = 0;
-        unsigned long error;
+        target_ulong error;
 
 	if (elf_ppnt->p_type != PT_LOAD)
             continue;
@@ -1257,6 +1268,8 @@ int load_elf_binary(struct linux_binprm 
         k = elf_ppnt->p_vaddr;
         if (k < start_code)
             start_code = k;
+        if (start_data < k)
+            start_data = k;
         k = elf_ppnt->p_vaddr + elf_ppnt->p_filesz;
         if (k > elf_bss)
             elf_bss = k;
@@ -1273,7 +1286,7 @@ int load_elf_binary(struct linux_binprm 
     elf_brk += load_bias;
     start_code += load_bias;
     end_code += load_bias;
-    //    start_data += load_bias;
+    start_data += load_bias;
     end_data += load_bias;
 
     if (elf_interpreter) {
@@ -1320,7 +1333,7 @@ int load_elf_binary(struct linux_binprm 
     info->start_brk = info->brk = elf_brk;
     info->end_code = end_code;
     info->start_code = start_code;
-    info->start_data = end_code;
+    info->start_data = start_data;
     info->end_data = end_data;
     info->start_stack = bprm->p;
 
Index: linux-user/main.c
===================================================================
RCS file: /sources/qemu/qemu/linux-user/main.c,v
retrieving revision 1.127
diff -u -d -d -p -r1.127 main.c
--- linux-user/main.c	4 Oct 2007 01:54:44 -0000	1.127
+++ linux-user/main.c	4 Oct 2007 03:12:51 -0000
@@ -1950,14 +1950,17 @@ int main(int argc, char **argv)
     if (loglevel) {
         page_dump(logfile);
 
-        fprintf(logfile, "start_brk   0x%08lx\n" , info->start_brk);
-        fprintf(logfile, "end_code    0x%08lx\n" , info->end_code);
-        fprintf(logfile, "start_code  0x%08lx\n" , info->start_code);
-        fprintf(logfile, "start_data  0x%08lx\n" , info->start_data);
-        fprintf(logfile, "end_data    0x%08lx\n" , info->end_data);
-        fprintf(logfile, "start_stack 0x%08lx\n" , info->start_stack);
-        fprintf(logfile, "brk         0x%08lx\n" , info->brk);
-        fprintf(logfile, "entry       0x%08lx\n" , info->entry);
+        fprintf(logfile, "start_brk   0x" TARGET_FMT_lx "\n", info->start_brk);
+        fprintf(logfile, "end_code    0x" TARGET_FMT_lx "\n", info->end_code);
+        fprintf(logfile, "start_code  0x" TARGET_FMT_lx "\n",
+                info->start_code);
+        fprintf(logfile, "start_data  0x" TARGET_FMT_lx "\n",
+                info->start_data);
+        fprintf(logfile, "end_data    0x" TARGET_FMT_lx "\n", info->end_data);
+        fprintf(logfile, "start_stack 0x" TARGET_FMT_lx "\n",
+                info->start_stack);
+        fprintf(logfile, "brk         0x" TARGET_FMT_lx "\n", info->brk);
+        fprintf(logfile, "entry       0x" TARGET_FMT_lx "\n", info->entry);
     }
 
     target_set_brk(info->brk);
Index: linux-user/qemu.h
===================================================================
RCS file: /sources/qemu/qemu/linux-user/qemu.h,v
retrieving revision 1.37
diff -u -d -d -p -r1.37 qemu.h
--- linux-user/qemu.h	30 Sep 2007 14:32:45 -0000	1.37
+++ linux-user/qemu.h	4 Oct 2007 03:12:51 -0000
@@ -17,18 +17,18 @@
  * task_struct fields in the kernel
  */
 struct image_info {
-        target_ulong    load_addr;
-	unsigned long	start_code;
-	unsigned long	end_code;
-        unsigned long   start_data;
-	unsigned long	end_data;
-	unsigned long	start_brk;
-	unsigned long	brk;
-	unsigned long	start_mmap;
-	unsigned long	mmap;
-	unsigned long	rss;
-	unsigned long	start_stack;
-	unsigned long	entry;
+	target_ulong	load_addr;
+	target_ulong	start_code;
+	target_ulong	end_code;
+        target_ulong    start_data;
+	target_ulong	end_data;
+	target_ulong	start_brk;
+	target_ulong	brk;
+	target_ulong	start_mmap;
+	target_ulong	mmap;
+	target_ulong	rss;
+	target_ulong	start_stack;
+	target_ulong	entry;
         target_ulong    code_offset;
         target_ulong    data_offset;
         char            **host_argv;
@@ -105,7 +105,7 @@ extern const char *qemu_uname_release;
 struct linux_binprm {
         char buf[128];
         void *page[MAX_ARG_PAGES];
-        unsigned long p;
+        target_ulong p;
 	int fd;
         int e_uid, e_gid;
         int argc, envc;

Reply via email to