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 0F1373858C20 for ; Thu, 8 Jun 2023 18:16:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0F1373858C20 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=1686248203; 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=hrz2ln3KaXW2/Y+nUTsGrOeolgkQyvk0HncV3Z/wiME=; b=M9Wa4XgHBI6yyX6m6/qmV6GwYK6icubY7hisTXG1S8d9PewoDkK+mbm4h/L5i44bXs3Qfx JzJiaKWZeSE846RN3yH3NpVg+aa3ZOa5eTgqbezsBcZXiQqV3JhCFUrzsnyy34aM0eZbO3 LuzNxf32166zyqU3O914hS0pjo+RKj4= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-128-SMQVFnaLN6Kn67dORc3YEg-1; Thu, 08 Jun 2023 14:16:42 -0400 X-MC-Unique: SMQVFnaLN6Kn67dORc3YEg-1 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-30e6153f0eeso430050f8f.0 for ; Thu, 08 Jun 2023 11:16:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686248201; x=1688840201; 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=hrz2ln3KaXW2/Y+nUTsGrOeolgkQyvk0HncV3Z/wiME=; b=Rvkh1ZEulfn1Vk6hWoxAU32t31iz5YKwIq6Khm2/Ly+Hs7Tja512/bxH0hsKgG6sSB 6UwJZxoJi/bpEMPDBqbShAxMmhAlgIp+5DDVeUyLdWEHwcU7TavjanhVdk/BSAANi3OH 0SAGIPKWCDMjPgyycuHC8dLZFkbeC+RqDphtlgg5qqqWn06Vh0SGADdTGCups82tabqB SQUOv/mV+4+jz3V8c3j9bhUlVpeaCEUJXBkKSn3yHHzk9nwwAXPsTEs8GKyGIAa5nWCM BLf+jjXT2TCwG+lCmYtJEorDdyK7tbBhE4VfRza02lRF/XNvSkQHDk7hMAPx2i/RMrIB 67WA== X-Gm-Message-State: AC+VfDxpn/ucp0ES/IdhGeT7YYV76DDv6U+eavJ1ANDYhkxieJuPTH1j zUQYRyimK9CKzL2xZeIuCDeAp0qOVOR6nzR7cbeZqHF0avWaAPsCXhwBDar7HlwMwS/JYoJ68Ue DaRxHKVSLjhUsVoqxRfL6t4+84FulfA== X-Received: by 2002:a5d:5956:0:b0:30a:dd15:bb69 with SMTP id e22-20020a5d5956000000b0030add15bb69mr7051548wri.18.1686248200806; Thu, 08 Jun 2023 11:16:40 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4O25M/43rtheO8oYdvUKTyIsWm3939M6OFweT5tg6XdWZBFdQG9iw18RHsPmpIZozUIDefeA== X-Received: by 2002:a5d:5956:0:b0:30a:dd15:bb69 with SMTP id e22-20020a5d5956000000b0030add15bb69mr7051537wri.18.1686248200329; Thu, 08 Jun 2023 11:16:40 -0700 (PDT) Received: from localhost (92.40.218.122.threembb.co.uk. [92.40.218.122]) by smtp.gmail.com with ESMTPSA id s14-20020adfeb0e000000b0030ae53550f5sm2249435wrn.51.2023.06.08.11.16.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jun 2023 11:16:39 -0700 (PDT) From: Andrew Burgess To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH 22/31] gdbserver: Queue no-resumed event after thread exit In-Reply-To: <20221212203101.1034916-23-pedro@palves.net> References: <20221212203101.1034916-1-pedro@palves.net> <20221212203101.1034916-23-pedro@palves.net> Date: Thu, 08 Jun 2023 19:16:38 +0100 Message-ID: <877csdzunt.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=-10.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H5,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: > Normally, if the last thread resumed thread on the target exits, the "thread resumed thread" .... I think there's an extra 'thread' in there. Otherwise, LGTM. Reviewed-By: Andrew Burgess Thanks, Andrew > server sends a no-resumed event to GDB. If however, GDB enables the > GDB_THREAD_OPTION_EXIT option on a thread, and, that thread exits, the > server sends a thread exit event for that thread instead. > > In all-stop RSP mode, since events can only be forwarded to GDB one at > a time, and the whole target stops whenever an event is reported, GDB > resumes the target again after getting a THREAD_EXITED event, and then > the server finally reports back a no-resumed event if/when > appropriate. > > For non-stop RSP though, events are asynchronous, and if the server > sends a thread-exit event for the last resumed thread, the no-resumed > event is never sent. This patch makes sure that in non-stop mode, the > server queues a no-resumed event after the thread-exit event if it was > the last resumed thread that exited. > > Without this, we'd see failures in step-over-thread-exit testcases > added later in the series, like so: > > continue > Continuing. > - No unwaited-for children left. > - (gdb) PASS: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: non-stop=on: target-non-stop=on: schedlock=off: ns_stop_all=1: continue stops when thread exits > + FAIL: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: non-stop=on: target-non-stop=on: schedlock=off: ns_stop_all=1: continue stops when thread exits (timeout) > > (and other similar ones) > > Change-Id: I927d78b30f88236dbd5634b051a716f72420e7c7 > --- > gdbserver/linux-low.cc | 47 +++++++++++++++++++++++++----------------- > gdbserver/linux-low.h | 2 ++ > gdbserver/server.cc | 12 ++++++++++- > gdbserver/target.cc | 6 ++++++ > gdbserver/target.h | 6 ++++++ > 5 files changed, 53 insertions(+), 20 deletions(-) > > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc > index ebc3bf34127..bf6dc1d995a 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -2963,7 +2963,6 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus, > int report_to_gdb; > int trace_event; > int in_step_range; > - int any_resumed; > > threads_debug_printf ("[%s]", target_pid_to_str (ptid).c_str ()); > > @@ -2977,23 +2976,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus, > in_step_range = 0; > ourstatus->set_ignore (); > > - auto status_pending_p_any = [&] (thread_info *thread) > - { > - return status_pending_p_callback (thread, minus_one_ptid); > - }; > - > - auto not_stopped = [&] (thread_info *thread) > - { > - return not_stopped_callback (thread, minus_one_ptid); > - }; > - > - /* Find a resumed LWP, if any. */ > - if (find_thread (status_pending_p_any) != NULL) > - any_resumed = 1; > - else if (find_thread (not_stopped) != NULL) > - any_resumed = 1; > - else > - any_resumed = 0; > + bool was_any_resumed = any_resumed (); > > if (step_over_bkpt == null_ptid) > pid = wait_for_event (ptid, &w, options); > @@ -3004,7 +2987,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus, > pid = wait_for_event (step_over_bkpt, &w, options & ~WNOHANG); > } > > - if (pid == 0 || (pid == -1 && !any_resumed)) > + if (pid == 0 || (pid == -1 && !was_any_resumed)) > { > gdb_assert (target_options & TARGET_WNOHANG); > > @@ -6166,6 +6149,32 @@ linux_process_target::thread_stopped (thread_info *thread) > return get_thread_lwp (thread)->stopped; > } > > +bool > +linux_process_target::any_resumed () > +{ > + bool any_resumed; > + > + auto status_pending_p_any = [&] (thread_info *thread) > + { > + return status_pending_p_callback (thread, minus_one_ptid); > + }; > + > + auto not_stopped = [&] (thread_info *thread) > + { > + return not_stopped_callback (thread, minus_one_ptid); > + }; > + > + /* Find a resumed LWP, if any. */ > + if (find_thread (status_pending_p_any) != NULL) > + any_resumed = 1; > + else if (find_thread (not_stopped) != NULL) > + any_resumed = 1; > + else > + any_resumed = 0; > + > + return any_resumed; > +} > + > /* This exposes stop-all-threads functionality to other modules. */ > > void > diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h > index 1c1754d2b59..33a14e15173 100644 > --- a/gdbserver/linux-low.h > +++ b/gdbserver/linux-low.h > @@ -259,6 +259,8 @@ class linux_process_target : public process_stratum_target > > bool thread_stopped (thread_info *thread) override; > > + bool any_resumed () override; > + > void pause_all (bool freeze) override; > > void unpause_all (bool unfreeze) override; > diff --git a/gdbserver/server.cc b/gdbserver/server.cc > index 5099db1ee31..77b4b57466d 100644 > --- a/gdbserver/server.cc > +++ b/gdbserver/server.cc > @@ -4740,7 +4740,17 @@ handle_target_event (int err, gdb_client_data client_data) > } > } > else > - push_stop_notification (cs.last_ptid, cs.last_status); > + { > + push_stop_notification (cs.last_ptid, cs.last_status); > + > + if (cs.last_status.kind () == TARGET_WAITKIND_THREAD_EXITED > + && !target_any_resumed ()) > + { > + target_waitstatus ws; > + ws.set_no_resumed (); > + push_stop_notification (null_ptid, ws); > + } > + } > } > > /* Be sure to not change the selected thread behind GDB's back. > diff --git a/gdbserver/target.cc b/gdbserver/target.cc > index 4584e9b3a8e..833e32a4830 100644 > --- a/gdbserver/target.cc > +++ b/gdbserver/target.cc > @@ -612,6 +612,12 @@ process_stratum_target::thread_stopped (thread_info *thread) > gdb_assert_not_reached ("target op thread_stopped not supported"); > } > > +bool > +process_stratum_target::any_resumed () > +{ > + return true; > +} > + > bool > process_stratum_target::supports_get_tib_address () > { > diff --git a/gdbserver/target.h b/gdbserver/target.h > index e2e818b130b..c3345fc67e8 100644 > --- a/gdbserver/target.h > +++ b/gdbserver/target.h > @@ -320,6 +320,9 @@ class process_stratum_target > /* Return true if THREAD is known to be stopped now. */ > virtual bool thread_stopped (thread_info *thread); > > + /* Return true if any thread is known to be resumed. */ > + virtual bool any_resumed (); > + > /* Return true if the get_tib_address op is supported. */ > virtual bool supports_get_tib_address (); > > @@ -683,6 +686,9 @@ target_read_btrace_conf (struct btrace_target_info *tinfo, > #define target_supports_software_single_step() \ > the_target->supports_software_single_step () > > +#define target_any_resumed() \ > + the_target->any_resumed () > + > ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, > target_wait_flags options, int connected_wait); > > -- > 2.36.0