This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: PR corefiles/11467: amd64 gdb generates corrupted 32bit core file


On Tue, Apr 13, 2010 at 11:40 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> Date: Mon, 12 Apr 2010 11:50:47 -0700
>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>
>> On Mon, Apr 12, 2010 at 11:23 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> Date: Mon, 12 Apr 2010 06:22:25 -0700
>> >> From: "H.J. Lu" <hongjiu.lu@intel.com>
>> >>
>> >> On Sun, Apr 11, 2010 at 01:52:50PM -0700, H.J. Lu wrote:
>> >> > Hi,
>> >> >
>> >> > Thanks for Mark's pointer. Solution is very simple. We just need to
>> >> > make sure that we call the right fill_gregset for 32bit executable
>> >> > on both Linux/x86-64 and Linux/i386. ?OK to install?
>> >> >
>> >> > Thanks.
>> >> >
>> >> >
>> >>
>> >> Small update to use tdep->gregset_reg_offset instead of
>> >> i386_linux_gregset_reg_offset. ?OK to install?
>> >
>> > No. ?Like I explained in an earlier mail, we're not supposed to end up
>> > in fetch_gregset() in the first place.
>> >
>>
>> fetch_gregset is always defined and used to fill .reg section in
>> coredump on Linux/x86. i386-tdep.c has
>
> No, that's not how it's supposed to be. ?And unless there is a bug
> somewhere, this is not what happens for the 64x64 and 32x32 cases.
>
>> const struct regset *
>> i386_regset_from_core_section (struct gdbarch *gdbarch,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *sect_name, size_t sect_size)
>> {
>> ? struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>>
>> ? if (strcmp (sect_name, ".reg") == 0 && sect_size == tdep->sizeof_gregset)
>> ? ? {
>> ? ? ? if (tdep->gregset == NULL)
>> ? ? ? ? tdep->gregset = regset_alloc (gdbarch, i386_supply_gregset,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? i386_collect_gregset);
>> ? ? ? return tdep->gregset;
>> ? ? }
>>
>> For Linux, ?tdep->sizeof_gregset != the size of .reg section.
>
> Then there is your bug.
>
>> In fact, they have nothing to do with each other. sizeof_gregset
>> includes SSE and AVX registers, which have offset == -1 since they
>> aren't general-purpose registers. I don't believe we should set
>> tdep->gregset since general-purpose registers is a special case for
>> Linux/x86. They are handled differently. I don't want to change it.
>
> You're wrong here. ?This code used to work just fine. ?And I believe
> it still works fine for the 32x32 and 64x64 cases.

It works in the 32x32 and 64x64 cases because sect_size != tdep->sizeof_gregset
on Linux.

> I see that in i386-linux-tdep.c:i386_linux_regset_sections[] specifies
> the size of the regset as 144 and sets tdep->sizeof_gregset to 17 * 4
> = 68. ?That can't be right. ?Given that
> amd64-linux-tdep.c:amd64_linux_regset_sections[] specifies the size as
> 144 as well, I'm betting 68 is the right valaue for i386. ?I think

I tried to set it to 68 and gcore generated the incorrect coredump.

> somebody got confused here, since the size of the NT_PRSTATUS note in
> 32-bit core dumps happens to be 144, but the actual size of the space
> reserved for storing the registers in there is smaller.
>

There are

lfcore_write_prstatus (bfd *abfd,
                        char *buf,
                        int *bufsiz,
                        long pid,
                        int cursig,
                        const void *gregs)
{
  const char *note_name = "CORE";
  const struct elf_backend_data *bed = get_elf_backend_data (abfd);

  if (bed->elf_backend_write_core_note != NULL)
    {
      char *ret;
      ret = (*bed->elf_backend_write_core_note) (abfd, buf, bufsiz,
                                                 NT_PRSTATUS,
                                                 pid, cursig, gregs);
      if (ret != NULL)
        return ret;
    }

#if defined (HAVE_PRSTATUS32_T)
  if (bed->s->elfclass == ELFCLASS32)
    {
      prstatus32_t prstat;

      memset (&prstat, 0, sizeof (prstat));
      prstat.pr_pid = pid;
      prstat.pr_cursig = cursig;
      memcpy (&prstat.pr_reg, gregs, sizeof (prstat.pr_reg));
      return elfcore_write_note (abfd, buf, bufsiz, note_name,
                                 NT_PRSTATUS, &prstat, sizeof (prstat));
    }
  else
#endif
    {
      prstatus_t prstat;

      memset (&prstat, 0, sizeof (prstat));
      prstat.pr_pid = pid;
      prstat.pr_cursig = cursig;
      memcpy (&prstat.pr_reg, gregs, sizeof (prstat.pr_reg));
      return elfcore_write_note (abfd, buf, bufsiz, note_name,
                                 NT_PRSTATUS, &prstat, sizeof (prstat));
    }
}

tdep->sizeof_gregset ==  sizeof (prstat.pr_reg) and *bufize is the
section size:

(gdb) gcore good

Breakpoint 3, elfcore_write_prstatus (abfd=0xc49230, buf=0xc727e0 "\005",
    bufsiz=0x7fffffffdb2c, pid=15819, cursig=5, gregs=0x7fffffffd6b0)
    at /export/gnu/import/git/gdb/bfd/elf.c:8644
8644	{
(top-gdb) p *bufsiz
$1 = 144
(top-gdb)

Checking sect_size == tdep->sizeof_gregset  doesn't  make
any senses on Linux. It doesn't make any senses to any OSes
with

struct elf_prstatus32
  {
    struct elf_siginfo pr_info;         /* Info associated with signal.  */
    short int pr_cursig;                /* Current signal.  */
    unsigned int pr_sigpend;            /* Set of pending signals.  */
    unsigned int pr_sighold;            /* Set of held signals.  */
    __pid_t pr_pid;
    __pid_t pr_ppid;
    __pid_t pr_pgrp;
    __pid_t pr_sid;
    struct prstatus32_timeval pr_utime;         /* User time.  */
    struct prstatus32_timeval pr_stime;         /* System time.  */
    struct prstatus32_timeval pr_cutime;        /* Cumulative user time.  */
    struct prstatus32_timeval pr_cstime;        /* Cumulative system time.  */
    elf_gregset32_t pr_reg;             /* GP registers.  */
    int pr_fpvalid;                     /* True if math copro being used.  */
  };

where

elf_gregset32_t pr_reg;             /* GP registers.  */

is in the middle of NT_PRSTATUS section.


-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]