From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gateway23.websitewelcome.com (gateway23.websitewelcome.com [192.185.50.108]) by sourceware.org (Postfix) with ESMTPS id 999813858402 for ; Wed, 27 Oct 2021 18:40:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 999813858402 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=tromey.com Received: from cm13.websitewelcome.com (cm13.websitewelcome.com [100.42.49.6]) by gateway23.websitewelcome.com (Postfix) with ESMTP id 7A90828DD2 for ; Wed, 27 Oct 2021 13:39:59 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id fnqBmvwCpG0jLfnqBm0oi8; Wed, 27 Oct 2021 13:39:59 -0500 X-Authority-Reason: nr=8 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date:References :Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=5WmxH75rkTXFzT/laoCqyF4m0znzwf10Bes1DbDnalk=; b=nC2kmEzKaNQ2iFst9zq32eksYz cFNPK5JAVzISgTnVbW7C1uAcpyqGaMV7ya7tld7saspGuIwPMCRzzcTYu3dK7zrKzt0xax4KQZyG6 uWNeKCv6yEeOqI/ah6TCC/nYg; Received: from 174-16-0-219.hlrn.qwest.net ([174.16.0.219]:54092 helo=murgatroyd) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mfnqB-003fUh-2I; Wed, 27 Oct 2021 12:39:59 -0600 From: Tom Tromey To: Simon Marchi Cc: Tom Tromey , Simon Marchi via Gdb-patches , Patrick Monnerat Subject: Re: [PATCH] Replace deprecated_target_wait_hook by observers References: <20210826144044.146397-1-patrick@monnerat.net> <8240f04b-eef9-f14b-1d1f-18b2a187cc65@polymtl.ca> <87o88j3vfh.fsf@tromey.com> X-Attribution: Tom Date: Wed, 27 Oct 2021 12:39:58 -0600 In-Reply-To: (Simon Marchi's message of "Mon, 25 Oct 2021 15:02:24 -0400") Message-ID: <871r46pb0h.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 174.16.0.219 X-Source-L: No X-Exim-ID: 1mfnqB-003fUh-2I X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 174-16-0-219.hlrn.qwest.net (murgatroyd) [174.16.0.219]:54092 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-Spam-Status: No, score=-3031.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, JMQ_SPF_NEUTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org 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: , X-List-Received-Date: Wed, 27 Oct 2021 18:40:11 -0000 We discussed this a bit on irc but I thought I'd move the discussion here. Simon> I started experimenting with this, asserting if an observer throws: Simon> https://review.lttng.org/c/binutils-gdb/+/6527 Simon> But I am seeing some failures. As long as we are not enforcing this, i Simon> would favor the safer approach. I tried this too, using: diff --git a/gdbsupport/observable.h b/gdbsupport/observable.h index 8ed56612ad0..5eccbed48bc 100644 --- a/gdbsupport/observable.h +++ b/gdbsupport/observable.h @@ -139,7 +139,7 @@ class observable } /* Notify all observers that are attached to this observable. */ - void notify (T... args) const + void notify (T... args) const noexcept { OBSERVER_SCOPED_DEBUG_START_END ("observable %s notify() called", m_name); Then I ran the test suite. I found a few failures this way, and I tracked most of them down to get_frame_address_in_block_if_available. This hack fixed those: diff --git a/gdb/frame.c b/gdb/frame.c index b121892f799..2ec3c161529 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -2668,9 +2668,7 @@ get_frame_address_in_block_if_available (frame_info *this_frame, } catch (const gdb_exception_error &ex) { - if (ex.error == NOT_AVAILABLE_ERROR) - return false; - throw; + return false; } return true; However, I still get this one: terminate called after throwing an instance of 'gdb_exception_quit' [...] 0xba17eb tui_on_normal_stop ../../binutils-gdb/gdb/tui/tui-interp.c:99 [...] 0x804b10 _ZNK3gdb9observers10observableIJP7bpstatsiEE6notifyES3_i ../../binutils-gdb/gdb/../gdbsupport/observable.h:150 0x7ffee6 _Z11normal_stopv ../../binutils-gdb/gdb/infrun.c:8602 This one is disturbing to me, because it is a C-c that's being handled by an observer. As I said on irc, my mental model for observers is that they are a "veil of ignorance" way for one module to subscribe to state changes from another module. They shouldn't normally be "slow" or block -- they should not require interruption. Furthermore, each observer should be independent (notwithstanding the dependency scheme -- in most cases this isn't applicable). This latter requirement explains why an observer should not throw an exception: if one does, then different orders of observer will have different effects in gdb, and anyway this would make an observer an unreliable thing. But if they are unreliable, then that's very bad, because it means they're unusable for things like cache flushing. So one idea here is to suppress quits when notifying an observer. (Or, say, suppressing quits in tui_on_normal_stop at least.) However, I looked very briefly and found this in remote.c: /* Hook into new objfile notification. */ gdb::observers::new_objfile.attach (remote_new_objfile, "remote"); The implementation of this observer sends packets to the remote (via remote_check_symbols)... surely this is the kind of thing that ought to be interruptible. (I don't know if it is right now.) So I'm not really sure what route is best. One idea would be to ignore and log ordinary exceptions from observers, and then re-throw on a quit. I'm not confident this will be correct though. Tom