From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id JZnvMjmBqGQEpBkAWB0awg (envelope-from ) for ; Fri, 07 Jul 2023 17:18:49 -0400 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=EUorD9h4; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id C11241E0BD; Fri, 7 Jul 2023 17:18:49 -0400 (EDT) Received: from server2.sourceware.org (server2.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 AFBFC1E0AC for ; Fri, 7 Jul 2023 17:18:47 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 27F2C386C49E for ; Fri, 7 Jul 2023 21:18:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 27F2C386C49E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1688764727; bh=FjdeIADpzgRfrqjeFEjSu3JxcPtIrnKtEuWwVmD7KKE=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=EUorD9h4sqJNFgVRYDyYX9Jfx39g4VRDPH0sXpaFf0lZJIX+HO5tFEY3im+cgdFyN MK4OaFX8AOL7Uqxlr3mVDs6GoXzFeI/ZgNziVuJw0LOnsrKFZPZHQ5SSV3a8gD8gsk qHQa9JFIgytiF7O6H5312LVffXaokco9KahxzUOc= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id C7228385AF9D for ; Fri, 7 Jul 2023 21:18:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C7228385AF9D Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-70-PH2laVTQNFqpxpnEDJCsvQ-1; Fri, 07 Jul 2023 17:18:25 -0400 X-MC-Unique: PH2laVTQNFqpxpnEDJCsvQ-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-3fbe4cecd66so12476955e9.1 for ; Fri, 07 Jul 2023 14:18:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688764704; x=1691356704; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=FjdeIADpzgRfrqjeFEjSu3JxcPtIrnKtEuWwVmD7KKE=; b=l6K6/uSYtpqNxc4/gMHF+r1gwk8NLJIpNKDtFsLKPRnWV14zoEY63PgvgPThUhhNpW hA1NVFurNEBC70Lp3JybWKWk1dY0QJ/99JGFXJMVbbOS6WNCCtA8FsZ5mU79iRfqfVDV lCbdvAQa3hYxNGzaXAvtklYj3ygf7AeKTrdP34R1KA7/kGGZmAcCzBApI4SPXamijmKs gAVEOr5ejHdeZZKMsZ3ZuXq2zm4gm5y7F0JJ5neSTolK5T2fYGjGt9J8bUH1N8vTYX6a 5OD7iHvknEaDBNmWFGkPmddUfJVcMlS7h4PciDkDh96vsf82NS3DyI5m3WWKqXtf5MCB WQDw== X-Gm-Message-State: ABy/qLZ5REof9TSNUnuwMZKm2nyRIf/QnCZ14eEhd7hEh0zNgahn3H/S TZJ4Rog2dwqcfTM8NobN6M+OrPoupiPkbzhQrGVIbavKyEYGvaFXHU7x2d9nRa3Nm4hLWzBNGOt wSOB2HQ0RUTKBeCvp4d0UF9aM+xOG6Q== X-Received: by 2002:a05:600c:2283:b0:3fb:e4ce:cc65 with SMTP id 3-20020a05600c228300b003fbe4cecc65mr4767263wmf.25.1688764704058; Fri, 07 Jul 2023 14:18:24 -0700 (PDT) X-Google-Smtp-Source: APBJJlFJ6n9s2WCXsS/vwK//MnpQssnsAbD0QKzyLLtCgG3DGlnd/bhkl6SI09DZBjAiyCMCDvJ+ow== X-Received: by 2002:a05:600c:2283:b0:3fb:e4ce:cc65 with SMTP id 3-20020a05600c228300b003fbe4cecc65mr4767257wmf.25.1688764703803; Fri, 07 Jul 2023 14:18:23 -0700 (PDT) Received: from localhost (2.72.115.87.dyn.plus.net. [87.115.72.2]) by smtp.gmail.com with ESMTPSA id x4-20020a05600c21c400b003fa74bff02asm3417405wmj.26.2023.07.07.14.18.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 07 Jul 2023 14:18:23 -0700 (PDT) To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCHv5 05/11] gdb: don't always print breakpoint location after failed condition check In-Reply-To: <796e9bc1-6272-3528-93b9-f1463e6b8156@palves.net> References: <6fd4aa13-6003-2563-5841-e80d5a55d59e@palves.net> <796e9bc1-6272-3528-93b9-f1463e6b8156@palves.net> Date: Fri, 07 Jul 2023 22:18:22 +0100 Message-ID: <87mt07ju8x.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: , From: Andrew Burgess via Gdb-patches Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Pedro Alves writes: > On 2023-07-07 16:20, Pedro Alves wrote: >> I'd think the second *stopped shouldn't even be there at all, following the >> logic used to remove the stop from the CLI. >> >> Maybe we should try moving or copying this "don't report this stop" logic to >> fetch_inferior_event, to avoid calling the second normal_stop at all. >> I gave it a quick try, but what I tried without thinking much about it >> just hung GDB. I didn't dig too much as I want to move on to review the rest >> of your series. > > Here's the totally broken naive patch I tried. > > From e1fde974fa58351de9f6dc0661162d77fc8be078 Mon Sep 17 00:00:00 2001 > From: Pedro Alves > Date: Fri, 7 Jul 2023 16:10:29 +0100 > Subject: [PATCH] mi > > Change-Id: Ie49a36dc67b9584c40daabfffae1f87f6dd98c5c > --- > gdb/infrun.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 3dd24fccf6e..ab1fd13abe0 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -4395,6 +4395,8 @@ fetch_inferior_event () > auto defer_delete_threads > = make_scope_exit (delete_just_stopped_threads_infrun_breakpoints); > > + stop_context saved_context; > + > /* Now figure out what to do with the result of the result. */ > handle_inferior_event (&ecs); > > @@ -4415,16 +4417,17 @@ fetch_inferior_event () > } > else > { > - bool should_notify_stop = true; > bool proceeded = false; > > stop_all_threads_if_all_stop_mode (); > > clean_up_just_stopped_threads_fsms (&ecs); > > - if (thr != nullptr && thr->thread_fsm () != nullptr) > - should_notify_stop > - = thr->thread_fsm ()->should_notify_stop (); > + bool should_notify_stop > + = (!saved_context.changed () > + && thr != nullptr > + && thr->thread_fsm () != nullptr > + && thr->thread_fsm ()->should_notify_stop ()); So, I got this working, as in, not locking up GDB. The problem was that thr->thread_fsm() is often nullptr, so we end up never calling normal_stop. However, I don't think this is going to work out. There's a couple of problems. First, at the point fetch_inferior_event is called, there is no thread selected, so the ptid and thread captured in saved_context are always null_ptid and nullptr respectively. By the time we call saved_context.changed() we do have a thread selected, so the context appears to have always changed. However, I figured I could work around this be replacing the full stop_context with a simple 'int stop_id = get_stop_id ();', I figured this might be enough, this will tell me if GDB has notified the user of a stop somewhere within the handle_inferior_event code. This works fine in the sense that I do now correctly spot that the user has been notified of a stop and don't call normal_stop again... ... but as a consequence, GDB now doesn't realise that is has stopped, and doesn't display a prompt back to the user. I'm going to have to leave this for the weekend now, but will continue looking at this next week. I've included my WIP hack below. Would be interested on whether you see any problems with just using get_stop_id() to determine if the user has already seen a stop or not? Thanks, Andrew > > if (should_notify_stop) > { > > base-commit: c0c3bb70f2f13e07295041cdf24a4d2997fe99a4 > prerequisite-patch-id: c39ea2983f5abc3fac4664e7ea78420409d3df86 > -- > 2.34.1 --- diff --git a/gdb/infrun.c b/gdb/infrun.c index 3dd24fccf6e..83c5c6c51d6 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4395,6 +4395,8 @@ fetch_inferior_event () auto defer_delete_threads = make_scope_exit (delete_just_stopped_threads_infrun_breakpoints); + int stop_id = get_stop_id (); + /* Now figure out what to do with the result of the result. */ handle_inferior_event (&ecs); @@ -4415,7 +4420,7 @@ fetch_inferior_event () } else { - bool should_notify_stop = true; + bool should_notify_stop = stop_id == get_stop_id (); bool proceeded = false; stop_all_threads_if_all_stop_mode (); @@ -4424,7 +4435,8 @@ fetch_inferior_event () if (thr != nullptr && thr->thread_fsm () != nullptr) should_notify_stop - = thr->thread_fsm ()->should_notify_stop (); + = (should_notify_stop + && thr->thread_fsm ()->should_notify_stop ()); if (should_notify_stop) {