[RFA] core files: wrong signal number with threaded program on sparc-solaris

[RFA] core files: wrong signal number with threaded program on sparc-solaris

[Joel Brobecker]

From: brobecke <brobecke@f8352e7e-cb20-0410-8ce7-b5d9e71c585c>

Hello,

We noticed the following problem. Given a threaded program that terminated
because of a SIGABRT (and produced a core file), GDB prints the following
information when loading this core file:

        % gdb crash
        (gdb) core core
        [...]
        Core was generated by `./crash'.
        Program terminated with signal 9, Killed.
                                ^^^^^^^^^^^^^^^^
        #0  0xffffffff7eec9128 in kill () from /lib/64/libc.so.1

The expected behavior is to tell the user that the program terminated
because of a SIGABRT:

        Core was generated by `./crash'.
        Program terminated with signal 6, Aborted.
        #0  0xff2c559c in kill () from /lib/libc.so.1

This issue started appearing after the following change was introduced:

        * bfd.m4 (BFD_HAVE_SYS_PROCFS_TYPE): Define _STRUCTURE_PROC
        before including procfs.h.
        (BFD_HAVE_SYS_PROCFS_TYPE_MEMBER): Likewise.
        * configure.in: Added autoconf probe for the pr_fpreg member.
        * configure: Regenerated.
        * config.in: Regenerated.
        * elf.c: Define _STRUCTURE_PROC before including procfs.h.

Basically, instead of using the NT_PRSTATUS notes, we now use the
NT_LWPSTATUS ones.  And what happens in our case is that we have
several such notes - I suspect one per thread.  The first note has
a signal set to 6, whereas the next one has a signal set to 9.

Interestingly, the same is true iw the NT_PRSTATUS notes, and digging
a little further, I found the following code in elfcore_grok_prstatus
(this is the function that handles NT_PRSTATUS notes):

      /* Do not overwrite the core signal if it
         has already been set by another thread.  */
      if (elf_tdata (abfd)->core_signal == 0)
        elf_tdata (abfd)->core_signal = prstat.pr_cursig;

It looks like the same logic should be applied in elfcore_grok_lwpstatus.
This is what this patch does.

bfd/ChangeLog:

        * elf.c (elfcore_grok_lwpstatus): Do not overwrite the core signal
        if it has already been set.

Tested on sparc-solaris by running the GDB testsuite. No regression.
OK to apply?

Thanks,
-- 
Joel

---
 bfd/ChangeLog.GNAT |    5 +++++
 bfd/elf.c          |    5 ++++-
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/bfd/ChangeLog.GNAT b/bfd/ChangeLog.GNAT
index ea483ec..93b4e85 100644
--- a/bfd/ChangeLog.GNAT
+++ b/bfd/ChangeLog.GNAT
@@ -1,3 +1,8 @@
+2010-01-28  Joel Brobecker  <brobecker@adacore.com>
+
+	* elf.c (elfcore_grok_lwpstatus): Do not overwrite the core signal
+	if it has already been set.
+
 2010-01-01  Joel Brobecker  <brobecker@adacore.com>
 
 	Revert a local change that was unnecessary.
diff --git a/bfd/elf.c b/bfd/elf.c
index aac3314..19e4695 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -7800,7 +7800,10 @@ elfcore_grok_lwpstatus (bfd *abfd, Elf_Internal_Note *note)
   memcpy (&lwpstat, note->descdata, sizeof (lwpstat));
 
   elf_tdata (abfd)->core_lwpid = lwpstat.pr_lwpid;
-  elf_tdata (abfd)->core_signal = lwpstat.pr_cursig;
+  /* Do not overwrite the core signal if it has already been set by
+     another thread.  */
+  if (elf_tdata (abfd)->core_signal == 0)
+    elf_tdata (abfd)->core_signal = lwpstat.pr_cursig;
 
   /* Make a ".reg/999" section.  */
 
-- 
1.6.3.3

Re: [RFA] core files: wrong signal number with threaded program on sparc-solaris

[Alan Modra]

On Thu, Jan 28, 2010 at 04:36:28PM +0400, Joel Brobecker wrote:
>         * elf.c (elfcore_grok_lwpstatus): Do not overwrite the core signal
>         if it has already been set.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [RFA] core files: wrong signal number with threaded program on sparc-solaris

[Joel Brobecker]

> On Thu, Jan 28, 2010 at 04:36:28PM +0400, Joel Brobecker wrote:
> >         * elf.c (elfcore_grok_lwpstatus): Do not overwrite the core signal
> >         if it has already been set.
> 
> OK.

Thanks, checked in.

Reviews are really fast on binutils - a big thank you to the team
of maintainers.

-- 
Joel