From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id PtVxCcMDAWR9fQIAWB0awg (envelope-from ) for ; Thu, 02 Mar 2023 15:14:59 -0500 Received: by simark.ca (Postfix, from userid 112) id 182691E223; Thu, 2 Mar 2023 15:14:59 -0500 (EST) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=hbySUIMf; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_HI,RDNS_DYNAMIC,URIBL_BLOCKED,WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 438C21E0D3 for ; Thu, 2 Mar 2023 15:14:55 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3E99C3858439 for ; Thu, 2 Mar 2023 20:14:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3E99C3858439 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677788094; bh=3/OlKr0lreLJx/NxT8F4H2pnk18fVFwmlbLWOyTzVo8=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=hbySUIMfn1vMDFhu26lDRSl3vkqmQSCsfQ/EfrrfhgHLt2PtB1qzXk0l65bSDBUYg xNPC2Acq3fCVHr88tSCyo7QGnnwL35N0S5+zJD5+od/mN/u2S3/0EBa9gnpJ8B3G0P P/tWrbJg0OKUkJ6SuAyK1jvWqFaTWJFG/R1pT7Y4= Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 4A7C43858D37 for ; Thu, 2 Mar 2023 20:14:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4A7C43858D37 Received: from [172.16.0.192] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id BB4B61E0D3; Thu, 2 Mar 2023 15:14:32 -0500 (EST) Message-ID: <53aa80dd-767f-6587-8990-002272ff6689@simark.ca> Date: Thu, 2 Mar 2023 15:14:32 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH v5 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit Content-Language: fr To: Kevin Buettner , gdb-patches@sourceware.org Cc: pedro@palves.net, Tom de Vries References: <20230222234613.29662-1-kevinb@redhat.com> <20230222234613.29662-3-kevinb@redhat.com> In-Reply-To: <20230222234613.29662-3-kevinb@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" 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 > Approved-by: Pedro Alves 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