[PATCHv2,0/6] GDB Synchronous Signal Handling

Message ID cover.1626890878.git.andrew.burgess@embecosm.com
Headers show
Series
  • GDB Synchronous Signal Handling
Related show

Message

Andrew Burgess July 21, 2021, 6:08 p.m.
Thanks for all of the great feedback on v1 of the patch.

For patch #1, I have not made any attempt to "recover" after the fatal
signal.  I think Pedro said everything I would want to say, basically,
I'm not sure there's a general way (i.e. covering all of GDB) that we
could recover without leaving GDB is a pretty broken state.  If anyone
knows different then I'm happy to revisit this idea.

Patch #2 is unchanged.

Patch #3 is changed to rename the function as Pedro suggested, and to
remove the dead code that was pointed out.

Patch #4 I've extended the message printed along with the backtrace to
include words that are similar to those given when we hit an internal
error.  There's an example of this output in the commit message.  The
backtrace is now on by default.  As was discussed in the comment
thread for this patch, querying the user isn't really possible from
within the signal handler, and even if we could somehow "recover" from
the signal handler back to GDB's main loop, dumping a core file at
that point would be pretty pointless.

Patch #5 is mostly unchanged, small tweak to the commit message.

Patch #6 is new, addresses the fact that the new backtrace was being
produced when we hit an internal error, which was never my original
intention.

---

Andrew Burgess (6):
  gdb: terminate upon receipt of SIGFPE
  gdb: register signal handler after setting up event token
  gdb: rename async_init_signals to gdb_init_signals
  gdb: print backtrace on fatal SIGSEGV
  gdb: register SIGBUS, SIGFPE, and SIGABRT handlers
  gdb: don't print backtrace when dumping core after an internal error

 gdb/NEWS                                      |   7 +
 gdb/config.in                                 |   6 +
 gdb/configure                                 |  51 ++++
 gdb/configure.ac                              |  22 ++
 gdb/doc/gdb.texinfo                           |  18 ++
 gdb/event-top.c                               | 227 +++++++++++++-----
 gdb/event-top.h                               |   2 +-
 gdb/testsuite/gdb.base/bt-on-fatal-signal.c   |  22 ++
 gdb/testsuite/gdb.base/bt-on-fatal-signal.exp | 173 +++++++++++++
 gdb/top.c                                     |   2 +-
 gdb/ui-file.h                                 |   9 +
 gdb/utils.c                                   |   5 +
 12 files changed, 478 insertions(+), 66 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/bt-on-fatal-signal.c
 create mode 100644 gdb/testsuite/gdb.base/bt-on-fatal-signal.exp

-- 
2.25.4

Comments

Tom Tromey July 27, 2021, 6:54 p.m. | #1
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:


Andrew> Thanks for all of the great feedback on v1 of the patch.
Andrew> For patch #1, I have not made any attempt to "recover" after the fatal
Andrew> signal.  I think Pedro said everything I would want to say, basically,
Andrew> I'm not sure there's a general way (i.e. covering all of GDB) that we
Andrew> could recover without leaving GDB is a pretty broken state.  If anyone
Andrew> knows different then I'm happy to revisit this idea.

I read through these and they seem fine to me.

I had initially wondered whether SIGFPE could be caught and contained,
but the subsequent thread convinced me that having gdb simply die is
fine.

thanks,
Tom
Andrew Burgess Aug. 10, 2021, 9:33 a.m. | #2
Pedro,

You had a lot of great feedback on v1, I wondered if you had any
thoughts on the v2 series?

Thanks,
Andrew


* Andrew Burgess <andrew.burgess@embecosm.com> [2021-07-21 19:08:47 +0100]:

> Thanks for all of the great feedback on v1 of the patch.

> 

> For patch #1, I have not made any attempt to "recover" after the fatal

> signal.  I think Pedro said everything I would want to say, basically,

> I'm not sure there's a general way (i.e. covering all of GDB) that we

> could recover without leaving GDB is a pretty broken state.  If anyone

> knows different then I'm happy to revisit this idea.

> 

> Patch #2 is unchanged.

> 

> Patch #3 is changed to rename the function as Pedro suggested, and to

> remove the dead code that was pointed out.

> 

> Patch #4 I've extended the message printed along with the backtrace to

> include words that are similar to those given when we hit an internal

> error.  There's an example of this output in the commit message.  The

> backtrace is now on by default.  As was discussed in the comment

> thread for this patch, querying the user isn't really possible from

> within the signal handler, and even if we could somehow "recover" from

> the signal handler back to GDB's main loop, dumping a core file at

> that point would be pretty pointless.

> 

> Patch #5 is mostly unchanged, small tweak to the commit message.

> 

> Patch #6 is new, addresses the fact that the new backtrace was being

> produced when we hit an internal error, which was never my original

> intention.

> 

> ---

> 

> Andrew Burgess (6):

>   gdb: terminate upon receipt of SIGFPE

>   gdb: register signal handler after setting up event token

>   gdb: rename async_init_signals to gdb_init_signals

>   gdb: print backtrace on fatal SIGSEGV

>   gdb: register SIGBUS, SIGFPE, and SIGABRT handlers

>   gdb: don't print backtrace when dumping core after an internal error

> 

>  gdb/NEWS                                      |   7 +

>  gdb/config.in                                 |   6 +

>  gdb/configure                                 |  51 ++++

>  gdb/configure.ac                              |  22 ++

>  gdb/doc/gdb.texinfo                           |  18 ++

>  gdb/event-top.c                               | 227 +++++++++++++-----

>  gdb/event-top.h                               |   2 +-

>  gdb/testsuite/gdb.base/bt-on-fatal-signal.c   |  22 ++

>  gdb/testsuite/gdb.base/bt-on-fatal-signal.exp | 173 +++++++++++++

>  gdb/top.c                                     |   2 +-

>  gdb/ui-file.h                                 |   9 +

>  gdb/utils.c                                   |   5 +

>  12 files changed, 478 insertions(+), 66 deletions(-)

>  create mode 100644 gdb/testsuite/gdb.base/bt-on-fatal-signal.c

>  create mode 100644 gdb/testsuite/gdb.base/bt-on-fatal-signal.exp

> 

> -- 

> 2.25.4

>
Pedro Alves Aug. 10, 2021, 6:56 p.m. | #3
Hi Andrew,

On 2021-08-10 10:33 a.m., Andrew Burgess wrote:
> Pedro,

> 

> You had a lot of great feedback on v1, I wondered if you had any

> thoughts on the v2 series?


Thanks.  I read the v2 series now, and it all looks good to me.

I just had a tiny nit comment in reply to patch #4.  No need to repost for that.