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 0/2][v3] correct core dump format

[Laurent Vivier]

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

It introduces a new parameter of the target: the datatype alignment size.

Targets like i386, mips or ppc align (short, int, long, long long) on
(2, 4, 4, 8), target like x86_64 aligns on (2, 4, 8, 8)

but arm aligns on (2, 4, 4, 4) and m68k (680x0) on (2, 2, 2, 2).

And this knowledge is needed to correctly generate a core dump.

For other targets, please update the patch with your favorite one.

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

[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