From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) by sourceware.org (Postfix) with ESMTPS id 838093858D32 for ; Mon, 30 Jan 2023 19:02:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 838093858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f54.google.com with SMTP id t18so12108461wro.1 for ; Mon, 30 Jan 2023 11:02:26 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=pLdmulgdRqIKss2lb2XXAo/ySE6BRLd4lPWCVvC2tNU=; b=tzNtV5EjIpQYH2q9zqcSXJRSGP8YxeNNNJ+nHzrhwQHDvhOK0Vse1eGu+5ul7G5axt v/JeX/SCf6UUW+gBx4VeG+p4SmYXGH721SPV+VN8/KgS6+JhFw/Myf14wxxun4nnuwvf rXfLZlQQ/T2Yg2xLefYYQcBbyBB5L80YHJMVY3gI4lKuZddxkyjsvJp98q0E7szM4oYR aCgCiaATcJ/tZETWGCbMaveClWw4FU2wj0WlYF+UZrlfOYZj0VcIO4a0i4w3pkImgzEK yoDcJ/YKz8HRtTPHLS+nalfM0a2TPE1vfw7bcmkfYWLUKoEAn9qzJReFgDQUaEFYuhQs fjHw== X-Gm-Message-State: AO0yUKW8feCfA38JV3YuHJ99PAVlCRmuJz3CVHBata4essXuO0n40axH kjGImfEZQSqH+/tR+rC9KFY= X-Google-Smtp-Source: AK7set8NLOkvRT2CP+jb4ayQROw9G06X75w7cYVRVKxjlJ6mYdtzqCSipTgTRUt1yGWSPmOelIF3Sg== X-Received: by 2002:a5d:680a:0:b0:2bf:f44b:7a29 with SMTP id w10-20020a5d680a000000b002bff44b7a29mr2467812wru.40.1675105345262; Mon, 30 Jan 2023 11:02:25 -0800 (PST) Received: from ?IPv6:2001:8a0:f92b:9e00::1fe? ([2001:8a0:f92b:9e00::1fe]) by smtp.gmail.com with ESMTPSA id k10-20020adff28a000000b002b9b9445149sm4223770wro.54.2023.01.30.11.02.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 30 Jan 2023 11:02:24 -0800 (PST) Subject: Re: [PATCH v4 8/8] Forced quit cases handled by resetting sync_quit_force_run To: Kevin Buettner , gdb-patches@sourceware.org Cc: simark@simark.ca, tdevries@suse.de References: <20230112015630.32999-1-kevinb@redhat.com> <20230112015630.32999-9-kevinb@redhat.com> From: Pedro Alves Message-ID: <6c35f63b-5eda-20ea-126f-4e5390710a0c@palves.net> Date: Mon, 30 Jan 2023 19:02:23 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20230112015630.32999-9-kevinb@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2023-01-12 1:56 a.m., Kevin Buettner wrote: > During my audit of the use of gdb_exception with regard to QUIT > processing, I found a try/catch in the scoped_switch_fork_info > destructor. > > Static analysis found this call path from the destructor to > maybe_quit(): > > scoped_switch_fork_info::~scoped_switch_fork_info() > -> remove_breakpoints() > -> remove_breakpoint(bp_location*) > -> remove_breakpoint_1(bp_location*, remove_bp_reason) > -> memory_validate_breakpoint(gdbarch*, bp_target_info*) > -> target_read_memory(unsigned long, unsigned char*, long) > -> target_read(target_ops*, target_object, char const*, unsigned char*, unsigned long, long) > -> maybe_quit() > > Since it's not safe to do a 'throw' from a destructor, we simply > call set_quit_flag and, for gdb_exception_forced_quit, also > set sync_quit_force_run. This will cause the appropriate > exception to be rethrown at the next QUIT check. > > Another case is the try / catch in tui_getc() in tui-io.c. The > existing catch swallows the exception. I've added a catch for > 'gdb_exception_forced_quit', which also swallows the exception, > but also sets sync_quit_force_run and calls set_quit_flag in > order to restart forced quit processing at the next QUIT check. > This is required because it isn't safe to throw into/through > readline. > > Thanks to Pedro Alves for suggesting this idea. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761 > --- > gdb/linux-fork.c | 13 +++++++++++++ > gdb/tui/tui-io.c | 9 +++++++++ > 2 files changed, 22 insertions(+) > > diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c > index 61545b859ea..fc2f00d0766 100644 > --- a/gdb/linux-fork.c > +++ b/gdb/linux-fork.c > @@ -430,6 +430,19 @@ class scoped_switch_fork_info > fork_load_infrun_state (m_oldfp); > insert_breakpoints (); > } > + catch (const gdb_exception_quit &ex) > + { > + /* We can't throw from a destructor, so re-set the quit flag > + for later QUIT checking. */ > + set_quit_flag (); > + } > + catch (const gdb_exception_forced_quit &ex) > + { > + /* Like above, but (eventually) cause GDB to terminate by > + setting sync_quit_force_run. */ > + sync_quit_force_run = 1; > + set_quit_flag (); I think it'd be nice if we had a function that did both of these for us. Something like "set_force_quit_flag()". > + } > catch (const gdb_exception &ex) > { > warning (_("Couldn't restore checkpoint state in %s: %s"), > diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c > index 2f39e34df2f..ac3c7296499 100644 > --- a/gdb/tui/tui-io.c > +++ b/gdb/tui/tui-io.c > @@ -1275,6 +1275,15 @@ tui_getc (FILE *fp) > { > return tui_getc_1 (fp); > } > + catch (const gdb_exception_forced_quit &ex) > + { > + /* As noted below, it's not safe to let an exception escape > + to newline, so, for this case, reset the quit flag for Spurious double space before "reset". > + later QUIT checking. */ > + sync_quit_force_run = 1; > + set_quit_flag (); > + return 0; > + } > catch (const gdb_exception &ex) > { > /* Just in case, don't ever let an exception escape to readline. >