From: Simon Marchi <simark@simark.ca>
To: Kevin Buettner <kevinb@redhat.com>, gdb-patches@sourceware.org
Cc: pedro@palves.net, Tom de Vries <tdevries@suse.de>
Subject: Re: [PATCH v5 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit
Date: Thu, 2 Mar 2023 15:14:32 -0500 [thread overview]
Message-ID: <53aa80dd-767f-6587-8990-002272ff6689@simark.ca> (raw)
In-Reply-To: <20230222234613.29662-3-kevinb@redhat.com>
On 2/22/23 18:46, Kevin Buettner via Gdb-patches wrote:
> When a GDB process receives the SIGTERM signal, handle_sigterm() in
> event-top.c is called. The global variable 'sync_quit_force_run' is
> set by this signal handler. It does some other things too, but the
> setting of this global is the important bit for the SIGTERM part of
> this discussion.
>
> GDB will periodically check to see whether a Ctrl-C or SIGTERM has
> been received. This is performed via use of the QUIT macro in
> GDB's code. QUIT is defined to invoke maybe_quit(), which will be
> periodically called during any lengthy operation. This is supposed to
> ensure that the user won't have to wait too long for a Ctrl-C or
> SIGTERM to be acted upon.
>
> When a Ctrl-C / SIGINT is received, quit_handler() will decide whether
> to pass the SIGINT onto the inferior or to call quit() which causes
> gdb_exception_quit to be thrown. This exception (usually) propagates
> to the top level. Control is then returned to the top level event
> loop.
>
> At the moment, SIGTERM is handled very differently. Instead of
> throwing an exception, quit_force() is called. This does eventually
> cause GDB to exit(), but prior to that happening, the inferiors
> are killed or detached and other target related cleanup occurs.
> As shown in this discussion between Pedro Alves and myself...
>
> https://sourceware.org/pipermail/gdb-patches/2021-July/180802.html
> https://sourceware.org/pipermail/gdb-patches/2021-July/180902.html
> https://sourceware.org/pipermail/gdb-patches/2021-July/180903.html
>
> ...we found that it is possible for inferior_ptid and current_thread_
> to get out of sync. When that happens, the "current_thread_ != nullptr"
> assertion in inferior_thread() can fail resulting in a GDB internal
> error.
>
> Pedro recommended that we "let the normal quit exception propagate all
> the way to the top level, and then have the top level call quit_force
> if sync_quit_force_run is set." However, after the v2 series for this
> patch set, we tweaked that idea by introducing a new exception for
> handling SIGTERM.
>
> This commit implements the obvious part of Pedro's suggestion:
> Instead of calling quit_force from quit(), throw_forced_quit() is now
> called instead. This causes the new exception 'gdb_exception_forced_quit'
> to be thrown.
>
> At the top level, I changed catch_command_errors() and captured_main()
> to catch gdb_exception_forced_quit and then call quit_force() from the
> catch block. I also changed start_event_loop() to also catch
> gdb_exception_forced_quit; while we could also call quit_force() from
> that catch block, it's sufficient to simply rethrow the exception
> since it'll be caught by the newly added code in captured_main().
>
> Making these changes fixed the failure / regression that I was seeing
> for gdb.base/gdb-sigterm.exp when run on a machine with glibc-2.34.
> However, there are many other paths back to the top level which this
> test case does not test. I did an audit of all of the try / catch
> code in GDB in which calls in the try-block might (eventually) call
> QUIT. I found many cases where gdb_exception_quit and the new
> gdb_exception_forced_quit will be swallowed. (When using GDB, have
> you ever hit Ctrl-C and not have it do anything; if so, it could be
> due to a swallowed gdb_exception_quit in one of the cases I've
> identified.) The rest of the patches in this series deal with this
> concern.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
> Tested-by: Tom de Vries <tdevries@suse.de>
> Approved-by: Pedro Alves <pedro@palves.net>
I see this failure following this commit:
$ make check TESTS="gdb.base/quit-live.exp" RUNTESTFLAGS="--target_board=native-gdbserver"
FAIL: gdb.base/quit-live.exp: appear_how=run: extra_inferior=0: quit_how=sigterm: quit with SIGTERM (GDB internal error)
FAIL: gdb.base/quit-live.exp: appear_how=run: extra_inferior=1: quit_how=sigterm: quit with SIGTERM (GDB internal error)
In gdb.log:
(gdb) continue
Continuing.
Breakpoint 1, main () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/quit-live.c:23
23 int secs = 30;
(gdb) Executing on host: kill -TERM 1989873 (timeout = 300)
builtin_spawn -ignore SIGHUP kill -TERM 1989873
SIGTERM
/home/smarchi/src/binutils-gdb/gdb/exceptions.c:100: internal-error: Bad switch.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
FAIL: gdb.base/quit-live.exp: appear_how=run: extra_inferior=0: quit_how=sigterm: quit with SIGTERM (GDB internal error)
Resyncing due to internal error.
0x557cb89b93f0 gdb_internal_backtrace_1
/home/smarchi/src/binutils-gdb/gdb/bt-utils.c:122
0x557cb89b93f0 _Z22gdb_internal_backtracev
/home/smarchi/src/binutils-gdb/gdb/bt-utils.c:168
0x557cb8e118f4 internal_vproblem
/home/smarchi/src/binutils-gdb/gdb/utils.c:401
0x557cb8e11bb0 _Z15internal_verrorPKciS0_P13__va_list_tag
/home/smarchi/src/binutils-gdb/gdb/utils.c:481
0x557cb8f666c4 _Z18internal_error_locPKciS0_z
/home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:58
0x557cb8aece58 print_exception
/home/smarchi/src/binutils-gdb/gdb/exceptions.c:100
0x557cb8d9f363 _Z10quit_forcePii
/home/smarchi/src/binutils-gdb/gdb/top.c:1849
0x557cb8977cdb _Z28invoke_async_signal_handlersv
/home/smarchi/src/binutils-gdb/gdb/async-event.c:233
0x557cb8f67ad7 _Z16gdb_do_one_eventi
/home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:199
0x557cb8bed709 start_event_loop
/home/smarchi/src/binutils-gdb/gdb/main.c:411
0x557cb8bed709 captured_command_loop
/home/smarchi/src/binutils-gdb/gdb/main.c:475
0x557cb8bef2f4 captured_main
/home/smarchi/src/binutils-gdb/gdb/main.c:1318
0x557cb8bef2f4 _Z8gdb_mainP18captured_main_args
/home/smarchi/src/binutils-gdb/gdb/main.c:1337
0x557cb890d87f main
/home/smarchi/src/binutils-gdb/gdb/gdb.c:32
---------------------
/home/smarchi/src/binutils-gdb/gdb/exceptions.c:100: internal-error: Bad switch.
Simon
next prev parent reply other threads:[~2023-03-02 20:14 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-22 23:46 [PATCH v5 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
2023-02-22 23:46 ` [PATCH v5 1/8] Introduce gdb_exception_forced_quit Kevin Buettner
2023-02-22 23:46 ` [PATCH v5 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit Kevin Buettner
2023-03-02 20:14 ` Simon Marchi [this message]
2023-02-22 23:46 ` [PATCH v5 3/8] Catch gdb_exception_error instead of gdb_exception (in many places) Kevin Buettner
2023-02-22 23:46 ` [PATCH v5 4/8] Python QUIT processing updates Kevin Buettner
2023-02-22 23:46 ` [PATCH v5 5/8] Guile " Kevin Buettner
2023-02-22 23:46 ` [PATCH v5 6/8] QUIT processing w/ explicit throw for gdb_exception_forced_quit Kevin Buettner
2023-02-22 23:46 ` [PATCH v5 7/8] Introduce set_force_quit_flag and change type of sync_quit_force_run Kevin Buettner
2023-02-22 23:46 ` [PATCH v5 8/8] Forced quit cases handled by resetting sync_quit_force_run Kevin Buettner
2023-02-23 13:01 ` [PATCH v5 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Pedro Alves
2023-02-27 23:22 ` Kevin Buettner
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=53aa80dd-767f-6587-8990-002272ff6689@simark.ca \
--to=simark@simark.ca \
--cc=gdb-patches@sourceware.org \
--cc=kevinb@redhat.com \
--cc=pedro@palves.net \
--cc=tdevries@suse.de \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).