Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Sergio Durigan Junior <sergiodj@redhat.com>
Cc: Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>,
	       "'GDB Patches'" <gdb-patches@sourceware.org>
Subject: Re: [RFC/PATCH] New convenience variable $_exitsignal
Date: Mon, 16 Sep 2013 18:04:00 -0000	[thread overview]
Message-ID: <52374823.4010203@redhat.com> (raw)
In-Reply-To: <m37ghqn1as.fsf@redhat.com>

On 06/19/2013 05:59 AM, Sergio Durigan Junior wrote:
> On Monday, June 17 2013, I wrote:
> 
>> On Monday, June 17 2013, Pierre Muller wrote:
>>
>>>   Hi Sergio,
>>>
>>>   Is there a reason why you don't handle
>>> corelow.c anymore in your new patch?
>>
>> Hi Pierre,
>>
>> Yes, corelow.c is not important to this patch because (as Pedro
>> explained on
>> <http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>)
>> $_exitsignal should not be set for corefiles, because the inferior has
>> not exited.
>>
>> corelow.c will be touched in my next patch, which will add $_signo (but
>> with the modifications proposed by Pedro).
> 
> I've been thinking about this answer I gave to Pierre.  After
> investigating how corefiles handle the signal, I guess the right choice
> would indeed be to set $_exitsignal in corelow.c as well.  This is my
> rationale.

Looks like I completely missed this email.  Sorry about that.

> 1) Single-threaded program + generate-core-file
> 
> In this case, NT_SIGINFO will not be filled by GDB's generate-core-file
> (bug) because PRSTATUS generation does not contemplate that yet (which
> reminds me of the PRPSINFO work I did few months ago, and the PRSTATUS
> work I still need to do, which will fix this bug).  So, in this case,
> "print $_siginfo.si_signo" will not display the correct signal, and we
> can only rely on "bfd_core_file_failing_signal" (called inside
> corelow.c).  Thus, setting $_signo to "bfd_core_file_failing_signal" is
> the logical choice (of course, if we want to avoid having to use
> NT_SIGINFO, that is the *only* choice).
> 
> 2) Single-threaded program + SIGSEGV (or another "Core" signal)
> 
> In this case, the Linux kernel correctly generates the NT_SIGINFO, which
> can be displayed by $_siginfo.  However, we don't want to use
> NT_SIGINFO, so "bfd_core_file_failing_signal" is the only choice again.
> 
> 3) Multi-threaded program + generate-core-file
> 
> Again, NT_SIGINFO is not generated by GDB.  Again,
> "bfd_core_file_failing_signal" is the only choice.  (Back to this case
> later)
> 
> 4) Multi-threaded program + SIGSEGV (or another "Core" signal)
> 
> Linux kernel generated NT_SIGINFO, but we don't want to use it.
> However, the kernel put in NT_SIGINFO the same signal number (which
> killed the process) for all threads.

Really?  That's ......, to say the least.  ;-)

Actually, in my 3.9.10-100.fc17.x86_64 kernel (Fedora 17),
what I see is that the kernel only generates the NT_SIGINFO
note for the thread that actually crashed.

Hmm, actually, for a program with 3 threads, that has one thread
call abort, I get:

$ readelf -n ~/gdb/tests/threads-crash.core.32195

Displaying notes found at file offset 0x000005f0 with length 0x00001998:
  Owner                 Data size       Description
  CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
  CORE                 0x00000088       NT_PRPSINFO (prpsinfo structure)
  CORE                 0x00000080       NT_SIGINFO (siginfo_t data)
  CORE                 0x00000130       NT_AUXV (auxiliary vector)
  CORE                 0x000002aa       NT_FILE (mapped files)
...
  CORE                 0x00000200       NT_FPREGSET (floating point registers)
  LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)
  CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
  CORE                 0x00000200       NT_FPREGSET (floating point registers)
  LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)
  CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
  CORE                 0x00000200       NT_FPREGSET (floating point registers)
  LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)

Which kind of makes sense, given the other threads didn't actually get
any signal.

> Thus, using
> "bfd_core_file_failing_signal" is OK since there is no concept of "this
> signal number killed only this thread".
> 
> 
> Case (3) is the most difficult IMO.  I don't know how we are going to
> handle it when I/we implement NT_SIGINFO generation on PRSTATUS.  My
> first reaction is to do it using the same logic as the Linux kernel,
> i.e., putting the same signal number in every thread's siginfo.  But I
> don't think we should bikeshed too much now, so I'm stopping my e-mail
> here.
> 
> I'd like to hear opinions.

I can't say I really understand how any of that argues against my
original rationale for not setting $_exitsignal on corefiles (because
the inferior has not really exited at the point the core has been
generated), rather than point at implementation choices.

Now, if one were to instead argue that _user interface_ -wise, it'd
make sense to set $_exitsignal, because we also print
"Program terminated with signal", (emphasis on "terminated"), then
I'd agree:

  siggy = bfd_core_file_failing_signal (core_bfd);
  if (siggy > 0)
    {
...
      printf_filtered (_("Program terminated with signal %s, %s.\n"),
		       gdb_signal_to_name (sig), gdb_signal_to_string (sig));
    }

-- 
Pedro Alves


  reply	other threads:[~2013-09-16 18:04 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-16  6:30 Sergio Durigan Junior
2013-06-16 16:22 ` Doug Evans
2013-06-17  3:33 ` Eli Zaretskii
2013-06-17  7:32 ` Pierre Muller
2013-06-17 17:55   ` Sergio Durigan Junior
2013-06-19  5:26     ` Sergio Durigan Junior
2013-09-16 18:04       ` Pedro Alves [this message]
2013-09-17  0:11         ` Sergio Durigan Junior
2013-09-17 16:19           ` Pedro Alves
2013-09-17 18:39         ` Tom Tromey
2013-09-17 18:53           ` Philippe Waroquiers
2013-09-17 18:59             ` Tom Tromey
2013-09-17 18:59             ` Sergio Durigan Junior
2013-09-17 19:08             ` Pedro Alves
2013-09-17 19:02           ` Pedro Alves
2013-09-17 19:09             ` Tom Tromey
2013-07-18 16:48     ` Tom Tromey
2013-06-17 17:28 ` Pedro Alves
2013-06-17 17:31   ` Pedro Alves
2013-06-17 17:41   ` Sergio Durigan Junior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52374823.4010203@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pierre.muller@ics-cnrs.unistra.fr \
    --cc=sergiodj@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox