From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1688764706; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=FjdeIADpzgRfrqjeFEjSu3JxcPtIrnKtEuWwVmD7KKE=; b=EDhSkwXp9HOfZO++8RUXu3RwaRpFdlXvdBYzGip6CF19rDzSQI3c8y+dKSPnEDPl9kG1Vm uzG7LWNeLhFvgvuGH50NQs4eENnnamyBh0EbcSzSyZII0LdKocIJJzsh01GIPN3DKYgGw7 M1nTT0Q1Vv/SFMQOSHBMH0PkQb6b/sY= 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) From: Andrew Burgess 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 List-Id: 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) {