Re: [Qemu-devel] Re: [PING 0.14] Missing patches (mostly fixes)

Re: [Qemu-devel] Re: [PING 0.14] Missing patches (mostly fixes)

[Laurent Vivier]

>On Sat, Feb 05, 2011 at 10:49:51PM +0100, Laurent Vivier wrote:
>> >On 02/03/2011 12:16 PM, Laurent Vivier wrote:
>> >> And this one ?
>> >>
>> >> linux-user: correct core dump format
>> >>
>> >> http://patchwork.ozlabs.org/patch/78464/
>
>Actually that patch is not ok. The issues you fix on m68k appear
>on arm/eabi after applying your patch. bswap part appears ok, but
>the padding is needed atleast on arm.

It is strange as we have in gdb, if I remember correctly, an explicit check of the size of prstatus:

gdb/bfd/elf.c:

    if defined (HAVE_PRSTATUS_T)

    static bfd_boolean
    elfcore_grok_prstatus (bfd *abfd, Elf_Internal_Note *note)
...
      if (note->descsz == sizeof (prstatus_t))
...

How do you test this patch ? Do you use native gdb on ARM ? Because, for m68k, cross-compiled gdb does not work (it cannot have sizeof(prstatus_t) for m68k).

Regards,
Laurent


-- 
--------------------- Laurent@vivier.eu  ---------------------
"Tout ce qui est impossible reste à accomplir"    Jules Verne
"Things are only impossible until they're not" Jean-Luc Picard

Re: [Qemu-devel] Re: [PING 0.14] Missing patches (mostly fixes)

[Peter Maydell]

On 9 February 2011 09:20, Laurent Vivier <Laurent@vivier.eu> wrote:
> It is strange as we have in gdb, if I remember correctly, an explicit check of the size of prstatus:
>
> gdb/bfd/elf.c:
>
>    if defined (HAVE_PRSTATUS_T)
>
>    static bfd_boolean
>    elfcore_grok_prstatus (bfd *abfd, Elf_Internal_Note *note)
> ...
>      if (note->descsz == sizeof (prstatus_t))
> ...
>
> How do you test this patch ? Do you use native gdb on ARM ? Because, for m68k, cross-compiled gdb does not work (it cannot have sizeof(prstatus_t) for m68k).

I dunno about m68k, but if ARM gdb behaves differently natively
versus the cross-tools version (ie hosted on x86 to target ARM)
then that sounds like a bug we in Linaro would like to know
about :-)

-- PMM

Re: [Qemu-devel] Re: [PING 0.14] Missing patches (mostly fixes)

[Riku Voipio]

> >Actually that patch is not ok. The issues you fix on m68k appear
> >on arm/eabi after applying your patch. bswap part appears ok, but
> >the padding is needed atleast on arm.
 
> How do you test this patch ? Do you use native gdb on ARM ? Because, for m68k, cross-compiled gdb does not work (it cannot have sizeof(prstatus_t) for m68k).

I tried both native and cross-gdb, version 7.2. Both fail with your patch applied
and work fine without it.

Before your patch, objdump (CodeSourcery cs2009q3 cross) output looks like:

-snip-
Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 note0         000001bc  00000000  00000000  000000d4  2**0
                  CONTENTS, READONLY
  1 .reg/23968    00000048  00000000  00000000  00000130  2**2
                  CONTENTS
  2 .reg          00000048  00000000  00000000  00000130  2**2
                  CONTENTS
  3 .auxv         00000070  00000000  00000000  00000220  2**2
                  CONTENTS
  4 load1         00000000  00008000  00000000  00001000  2**12
                  ALLOC, READONLY, CODE
-snip-

after:

-snip-
Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 note0         000001bc  00000000  00000000  000000d4  2**0
                  CONTENTS, READONLY
  1 .auxv         00000070  00000000  00000000  00000220  2**2
                  CONTENTS
  2 load1         00000000  00008000  00000000  00001000  2**12
                  ALLOC, READONLY, CODE
                  CONTENTS, ALLOC, LOAD
-snip-

[Qemu-devel] [PATCH 0/2] correct core dump format

[Laurent Vivier]

This is the v2 of my patch correcting the core dump format.

It introduces a new parameter of the target: the alignment size.
For the moment, it seems m68k is the only one that doesn't have 32bit
address alignment but 16bit one.

[PATCH 1/2] Define target alignment size
[PATCH 2/2] linux-user: correct core dump format

[Qemu-devel] [PATCH 1/2] Define target alignment size

[Laurent Vivier]

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 configure |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 2bf7f34..b71035b 100755
--- a/configure
+++ b/configure
@@ -2902,6 +2902,7 @@ target_nptl="no"
 interp_prefix1=`echo "$interp_prefix" | sed "s/%M/$target_arch2/g"`
 echo "CONFIG_QEMU_INTERP_PREFIX=\"$interp_prefix1\"" >> $config_target_mak
 gdb_xml_files=""
+target_alignment=4
 
 TARGET_ARCH="$target_arch2"
 TARGET_BASE_ARCH=""
@@ -2934,6 +2935,7 @@ case "$target_arch2" in
     bflt="yes"
     gdb_xml_files="cf-core.xml cf-fp.xml"
     target_phys_bits=32
+    target_alignment=2
   ;;
   microblaze)
     bflt="yes"
@@ -3012,6 +3014,7 @@ case "$target_arch2" in
     exit 1
   ;;
 esac
+echo "TARGET_ALIGNMENT=$target_alignment" >> $config_target_mak
 echo "TARGET_ARCH=$TARGET_ARCH" >> $config_target_mak
 target_arch_name="`echo $TARGET_ARCH | tr '[:lower:]' '[:upper:]'`"
 echo "TARGET_$target_arch_name=y" >> $config_target_mak
-- 
1.7.1

[Qemu-devel] [PATCH 2/2] linux-user: correct core dump format

[Laurent Vivier]

This patch allows to really use the core dumped by qemu with guest
architecture tools.

- it adds a missing bswap_phdr() for the program headers
  of memory regions.

  "objdump -x" sample:

BEFORE:

0x1000000 off    0x00200000 vaddr 0x00000400 paddr 0x00000000 align 2**21
         filesz 0x00000000 memsz 0x00100000 flags ---
0x1000000 off    0x00200000 vaddr 0x00100400 paddr 0x00000000 align 2**21
         filesz 0x00000000 memsz 0x00080000 flags --- 6000000

AFTER:

    LOAD off    0x00002000 vaddr 0x00040000 paddr 0x00000000 align 2**13
         filesz 0x00000000 memsz 0x00001000 flags ---
    LOAD off    0x00002000 vaddr 0x00041000 paddr 0x00000000 align 2**13
         filesz 0x00000000 memsz 0x00000800 flags rw-

- it doesn't pad the note size to sizeof(int32_t).
  On m68k the NT_PRSTATUS note size is 154 and
  must not be rounded up to 156, because this value is checked by
  objdump and gdb.

  "gdb" symptoms:

      "warning: Couldn't find general-purpose registers in core file."

  "objdump -x" sample:

BEFORE:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 note0         000001c4  00000000  00000000  000003b4  2**0
                  CONTENTS, READONLY
  1 .auxv         00000070  00000000  00000000  00000508  2**2
                  CONTENTS
  2 proc1         00100000  00000400  00000000  00200000  2**10
                  READONLY

AFTER:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 note0         000001c4  00000000  00000000  000003b4  2**0
                  CONTENTS, READONLY
  1 .reg/19022    00000050  00000000  00000000  0000040e  2**2
                  CONTENTS
  2 .reg          00000050  00000000  00000000  0000040e  2**2
                  CONTENTS
  3 .auxv         00000070  00000000  00000000  00000508  2**2
                  CONTENTS
  4 load1         00000000  00040000  00000000  00002000  2**13
                  ALLOC, READONLY

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/elfload.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 33d776d..61f65d6 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1688,6 +1688,7 @@ struct memelfnote {
     size_t     namesz_rounded;
     int        type;
     size_t     datasz;
+    size_t     datasz_rounded;
     void       *data;
     size_t     notesz;
 };
@@ -1713,7 +1714,7 @@ struct target_elf_prstatus {
     struct target_timeval pr_cstime; /* XXX Cumulative system time */
     target_elf_gregset_t      pr_reg;       /* GP registers */
     int                pr_fpvalid;   /* XXX */
-};
+} __attribute__((__aligned__(TARGET_ALIGNMENT))) __attribute__((packed));
 
 #define ELF_PRARGSZ     (80) /* Number of chars for args */
 
@@ -1963,7 +1964,9 @@ static void fill_note(struct memelfnote *note, const char *name, int type,
     note->namesz = namesz;
     note->namesz_rounded = roundup(namesz, sizeof (int32_t));
     note->type = type;
-    note->datasz = roundup(sz, sizeof (int32_t));;
+    note->datasz = sz;
+    note->datasz_rounded = roundup(sz, sizeof (int32_t));
+
     note->data = data;
 
     /*
@@ -1971,7 +1974,7 @@ static void fill_note(struct memelfnote *note, const char *name, int type,
      * ELF document.
      */
     note->notesz = sizeof (struct elf_note) +
-        note->namesz_rounded + note->datasz;
+        note->namesz_rounded + note->datasz_rounded;
 }
 
 static void fill_elf_header(struct elfhdr *elf, int segs, uint16_t machine,
@@ -2191,7 +2194,7 @@ static int write_note(struct memelfnote *men, int fd)
         return (-1);
     if (dump_write(fd, men->name, men->namesz_rounded) != 0)
         return (-1);
-    if (dump_write(fd, men->data, men->datasz) != 0)
+    if (dump_write(fd, men->data, men->datasz_rounded) != 0)
         return (-1);
 
     return (0);
@@ -2407,7 +2410,7 @@ static int elf_core_dump(int signr, const CPUState *env)
      * ELF specification wants data to start at page boundary so
      * we align it here.
      */
-    offset = roundup(offset, ELF_EXEC_PAGESIZE);
+    data_offset = offset = roundup(offset, ELF_EXEC_PAGESIZE);
 
     /*
      * Write program headers for memory regions mapped in
@@ -2430,6 +2433,7 @@ static int elf_core_dump(int signr, const CPUState *env)
             phdr.p_flags |= PF_X;
         phdr.p_align = ELF_EXEC_PAGESIZE;
 
+        bswap_phdr(&phdr, 1);
         dump_write(fd, &phdr, sizeof (phdr));
     }
 
@@ -2441,8 +2445,6 @@ static int elf_core_dump(int signr, const CPUState *env)
         goto out;
 
     /* align data to page boundary */
-    data_offset = lseek(fd, 0, SEEK_CUR);
-    data_offset = TARGET_PAGE_ALIGN(data_offset);
     if (lseek(fd, data_offset, SEEK_SET) != data_offset)
         goto out;
 
-- 
1.7.1

[Qemu-devel] [PATCH 2/2] linux-user: correct core dump format

[Laurent Vivier]

This patch allows to really use the core dumped by qemu with guest
architecture tools.

- it adds a missing bswap_phdr() for the program headers
  of memory regions.

  "objdump -x" sample:

BEFORE:

0x1000000 off    0x00200000 vaddr 0x00000400 paddr 0x00000000 align 2**21
         filesz 0x00000000 memsz 0x00100000 flags ---
0x1000000 off    0x00200000 vaddr 0x00100400 paddr 0x00000000 align 2**21
         filesz 0x00000000 memsz 0x00080000 flags --- 6000000

AFTER:

    LOAD off    0x00002000 vaddr 0x00040000 paddr 0x00000000 align 2**13
         filesz 0x00000000 memsz 0x00001000 flags ---
    LOAD off    0x00002000 vaddr 0x00041000 paddr 0x00000000 align 2**13
         filesz 0x00000000 memsz 0x00000800 flags rw-

- it doesn't pad the note size to sizeof(int32_t).
  On m68k the NT_PRSTATUS note size is 154 and
  must not be rounded up to 156, because this value is checked by
  objdump and gdb.

  "gdb" symptoms:

      "warning: Couldn't find general-purpose registers in core file."

  "objdump -x" sample:

BEFORE:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 note0         000001c4  00000000  00000000  000003b4  2**0
                  CONTENTS, READONLY
  1 .auxv         00000070  00000000  00000000  00000508  2**2
                  CONTENTS
  2 proc1         00100000  00000400  00000000  00200000  2**10
                  READONLY

AFTER:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 note0         000001c4  00000000  00000000  000003b4  2**0
                  CONTENTS, READONLY
  1 .reg/19022    00000050  00000000  00000000  0000040e  2**2
                  CONTENTS
  2 .reg          00000050  00000000  00000000  0000040e  2**2
                  CONTENTS
  3 .auxv         00000070  00000000  00000000  00000508  2**2
                  CONTENTS
  4 load1         00000000  00040000  00000000  00002000  2**13
                  ALLOC, READONLY

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
v2: use a predefined alignment size for target_elf_prstatus
v3: use target_<type> aligned according target properties

 linux-user/elfload.c |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 2de83e4..fe5410e 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -103,13 +103,13 @@ enum {
 
 typedef target_ulong    target_elf_greg_t;
 #ifdef USE_UID16
-typedef uint16_t        target_uid_t;
-typedef uint16_t        target_gid_t;
+typedef target_ushort   target_uid_t;
+typedef target_ushort   target_gid_t;
 #else
-typedef uint32_t        target_uid_t;
-typedef uint32_t        target_gid_t;
+typedef target_uint     target_uid_t;
+typedef target_uint     target_gid_t;
 #endif
-typedef int32_t         target_pid_t;
+typedef target_int      target_pid_t;
 
 #ifdef TARGET_I386
 
@@ -1761,19 +1761,20 @@ struct memelfnote {
     size_t     namesz_rounded;
     int        type;
     size_t     datasz;
+    size_t     datasz_rounded;
     void       *data;
     size_t     notesz;
 };
 
 struct target_elf_siginfo {
-    int  si_signo; /* signal number */
-    int  si_code;  /* extra code */
-    int  si_errno; /* errno */
+    target_int  si_signo; /* signal number */
+    target_int  si_code;  /* extra code */
+    target_int  si_errno; /* errno */
 };
 
 struct target_elf_prstatus {
     struct target_elf_siginfo pr_info;      /* Info associated with signal */
-    short              pr_cursig;    /* Current signal */
+    target_short       pr_cursig;    /* Current signal */
     target_ulong       pr_sigpend;   /* XXX */
     target_ulong       pr_sighold;   /* XXX */
     target_pid_t       pr_pid;
@@ -1785,7 +1786,7 @@ struct target_elf_prstatus {
     struct target_timeval pr_cutime; /* XXX Cumulative user time */
     struct target_timeval pr_cstime; /* XXX Cumulative system time */
     target_elf_gregset_t      pr_reg;       /* GP registers */
-    int                pr_fpvalid;   /* XXX */
+    target_int         pr_fpvalid;   /* XXX */
 };
 
 #define ELF_PRARGSZ     (80) /* Number of chars for args */
@@ -2036,7 +2037,9 @@ static void fill_note(struct memelfnote *note, const char *name, int type,
     note->namesz = namesz;
     note->namesz_rounded = roundup(namesz, sizeof (int32_t));
     note->type = type;
-    note->datasz = roundup(sz, sizeof (int32_t));;
+    note->datasz = sz;
+    note->datasz_rounded = roundup(sz, sizeof (int32_t));
+
     note->data = data;
 
     /*
@@ -2044,7 +2047,7 @@ static void fill_note(struct memelfnote *note, const char *name, int type,
      * ELF document.
      */
     note->notesz = sizeof (struct elf_note) +
-        note->namesz_rounded + note->datasz;
+        note->namesz_rounded + note->datasz_rounded;
 }
 
 static void fill_elf_header(struct elfhdr *elf, int segs, uint16_t machine,
@@ -2264,7 +2267,7 @@ static int write_note(struct memelfnote *men, int fd)
         return (-1);
     if (dump_write(fd, men->name, men->namesz_rounded) != 0)
         return (-1);
-    if (dump_write(fd, men->data, men->datasz) != 0)
+    if (dump_write(fd, men->data, men->datasz_rounded) != 0)
         return (-1);
 
     return (0);
@@ -2480,7 +2483,7 @@ static int elf_core_dump(int signr, const CPUState *env)
      * ELF specification wants data to start at page boundary so
      * we align it here.
      */
-    offset = roundup(offset, ELF_EXEC_PAGESIZE);
+    data_offset = offset = roundup(offset, ELF_EXEC_PAGESIZE);
 
     /*
      * Write program headers for memory regions mapped in
@@ -2503,6 +2506,7 @@ static int elf_core_dump(int signr, const CPUState *env)
             phdr.p_flags |= PF_X;
         phdr.p_align = ELF_EXEC_PAGESIZE;
 
+        bswap_phdr(&phdr, 1);
         dump_write(fd, &phdr, sizeof (phdr));
     }
 
@@ -2514,8 +2518,6 @@ static int elf_core_dump(int signr, const CPUState *env)
         goto out;
 
     /* align data to page boundary */
-    data_offset = lseek(fd, 0, SEEK_CUR);
-    data_offset = TARGET_PAGE_ALIGN(data_offset);
     if (lseek(fd, data_offset, SEEK_SET) != data_offset)
         goto out;
 
-- 
1.7.1