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.129.124]) by sourceware.org (Postfix) with ESMTPS id 8FD523858D3C for ; Wed, 22 Mar 2023 15:46:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8FD523858D3C 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=1679499966; 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=ELtYqV3ooa5STpHFwFzVJBwnN+9T9A31591/aiAO6Wk=; b=EpGktXlLobBsb8U/f0SKFEBN+inBcsQWS8udvKRkeNqYdBrFqht5SeY8ECVtFATkwfPllF /tmXE9t1g5r/a3ud5l2cl1MkeQbNXkbrkGB0SL8/JSK/yN7ZdONF/e5fet2fyu0Qz/xFRl bwwdK7Y69djpND4d8FI+bPPJSS/7IO8= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-91-zCVO2qV_P16v_tBMOEYxiA-1; Wed, 22 Mar 2023 11:46:05 -0400 X-MC-Unique: zCVO2qV_P16v_tBMOEYxiA-1 Received: by mail-qk1-f199.google.com with SMTP id 72-20020a37044b000000b0074694114c09so3898023qke.4 for ; Wed, 22 Mar 2023 08:46:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679499964; 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=ELtYqV3ooa5STpHFwFzVJBwnN+9T9A31591/aiAO6Wk=; b=MB6/C2VsD5PJ6LFgqcjRlm1fXyfRVznzaVKWMps6InJ4LaZlgbhlx5ZvpA7mXTjX4X OiPucaLcnAu3xw+q0huwepXx55m7kHQtGlMUhvoFK2VDj4xAd65aUU0W1xU3dN6duWSw g0Y/+4ospbLEVoUF8XaJct+6kC/hcZwm3wX+Ox8Pblf6kO4XVZw/RAsvBtKZR7H9e6ZE 46AAgd7mZNS29XZSHUxJZ6Nc6Q2pd5LhN0rb8Q4emiMQ6N3+dRJpV3hTmytffhcBbEZ8 MkgKTUolv/HpzYprbeJ7K+/XYDFdXBacct++XC8DpgzleqAxBZjskqXh+bv7BRTfzng3 d3sg== X-Gm-Message-State: AO0yUKWkJDTgouZHSjA8CVY3ZYDbN7fjnHmqhiA8vAlCS57I/cNBXovh 2d2h6XYifV0emAiB/ANh3uaL4saW278O6NU7MO6nwO1XejsVnkwDX3opvMmoysum2LKLc7SnnYh k2ajkIVML7t+1/oF/qn6PHe9bE7lTCw== X-Received: by 2002:a05:622a:1d1:b0:3a7:e625:14f with SMTP id t17-20020a05622a01d100b003a7e625014fmr7343206qtw.9.1679499964087; Wed, 22 Mar 2023 08:46:04 -0700 (PDT) X-Google-Smtp-Source: AK7set+6JnYRqV6OvipEckj4/9rfbKxNShRjFyxy/uHDzKKLBxcfQGKW9V4YdKCUx85kqYYwXmGmpQ== X-Received: by 2002:a05:622a:1d1:b0:3a7:e625:14f with SMTP id t17-20020a05622a01d100b003a7e625014fmr7343160qtw.9.1679499963666; Wed, 22 Mar 2023 08:46:03 -0700 (PDT) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id j68-20020a37b947000000b0074577e835f2sm11528921qkf.48.2023.03.22.08.46.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Mar 2023 08:46:03 -0700 (PDT) From: Andrew Burgess To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH 05/31] Support clone events in the remote protocol In-Reply-To: <20221212203101.1034916-6-pedro@palves.net> References: <20221212203101.1034916-1-pedro@palves.net> <20221212203101.1034916-6-pedro@palves.net> Date: Wed, 22 Mar 2023 15:46:01 +0000 Message-ID: <87cz50dc1y.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.7 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_H2,SPF_HELO_NONE,SPF_NONE,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: Pedro Alves writes: > The previous patch taught GDB about a new > TARGET_WAITKIND_THREAD_CLONED event kind, and made the Linux target > report clone events. > > A following patch will teach Linux GDBserver to do the same thing. > > But before we get there, we need to teach the remote protocol about > TARGET_WAITKIND_THREAD_CLONED. That's what this patch does. Clone is > very similar to vfork and fork, and the new stop reply is likewise > handled similarly. The stub reports "T05clone:...". > > GDBserver core is taught to handle TARGET_WAITKIND_THREAD_CLONED and > forward it to GDB in this patch, but no backend actually emits it yet. > That will be done in a following patch. > > Documentation for this new remote protocol feature is included in a > documentation patch later in the series. > > Change-Id: If271f20320d864f074d8ac0d531cc1a323da847f > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19675 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830 > --- > gdb/remote.c | 99 ++++++++++++++++++++++++++------------- > gdbserver/remote-utils.cc | 26 ++++++++-- > gdbserver/server.cc | 3 +- > 3 files changed, 91 insertions(+), 37 deletions(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index 53c4f19c5a4..a7430eb79dd 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -672,6 +672,7 @@ class remote_target : public process_stratum_target > const struct btrace_config *btrace_conf (const struct btrace_target_info *) override; > bool augmented_libraries_svr4_read () override; > void follow_fork (inferior *, ptid_t, target_waitkind, bool, bool) override; > + void follow_clone (ptid_t child_ptid) override; > void follow_exec (inferior *, ptid_t, const char *) override; > int insert_fork_catchpoint (int) override; > int remove_fork_catchpoint (int) override; > @@ -765,7 +766,7 @@ class remote_target : public process_stratum_target > > void remote_btrace_maybe_reopen (); > > - void remove_new_fork_children (threads_listing_context *context); > + void remove_new_children (threads_listing_context *context); > void kill_new_fork_children (inferior *inf); > void discard_pending_stop_replies (struct inferior *inf); > int stop_reply_queue_length (); > @@ -2579,9 +2580,10 @@ remote_target::remote_add_thread (ptid_t ptid, bool running, bool executing, > else > thread = add_thread (this, ptid); > > - /* We start by assuming threads are resumed. That state then gets updated > - when we process a matching stop reply. */ > - get_remote_thread_info (thread)->set_resumed (); > + /* We start by assuming threads are resumed. That state then gets > + updated when we process a matching stop reply. */ If feels like that comment is now out of date. We surely aren't assuming anything anymore, rather marking the thread as resumed when its executing. Otherwise, this commit looks good. Reviewed-By: Andrew Burgess Thanks, Andrew > + if (executing) > + get_remote_thread_info (thread)->set_resumed (); > > set_executing (this, ptid, executing); > set_running (this, ptid, running); > @@ -3983,10 +3985,11 @@ remote_target::update_thread_list () > } > } > > - /* Remove any unreported fork child threads from CONTEXT so > - that we don't interfere with follow fork, which is where > - creation of such threads is handled. */ > - remove_new_fork_children (&context); > + /* Remove any unreported fork/vfork/clone child threads from > + CONTEXT so that we don't interfere with follow > + fork/vfork/clone, which is where creation of such threads is > + handled. */ > + remove_new_children (&context); > > /* And now add threads we don't know about yet to our list. */ > for (thread_item &item : context.items) > @@ -4940,6 +4943,8 @@ remote_target::start_remote_1 (int from_tty, int extended_p) > } > else > switch_to_thread (find_thread_ptid (this, curr_thread)); > + > + get_remote_thread_info (inferior_thread ())->set_resumed (); > } > > /* init_wait_for_inferior should be called before get_offsets in order > @@ -5893,16 +5898,25 @@ is_fork_status (target_waitkind kind) > || kind == TARGET_WAITKIND_VFORKED); > } > > -/* Return THREAD's pending status if it is a pending fork parent, else > - return nullptr. */ > +/* Return a reference to the field where a pending child status, if > + there's one, is recorded. If there's no child event pending, the > + returned waitstatus has TARGET_WAITKIND_IGNORE kind. */ > + > +static const target_waitstatus & > +thread_pending_status (struct thread_info *thread) > +{ > + return (thread->has_pending_waitstatus () > + ? thread->pending_waitstatus () > + : thread->pending_follow); > +} > + > +/* Return THREAD's pending status if it is a pending fork/vfork (but > + not clone) parent, else return nullptr. */ > > static const target_waitstatus * > thread_pending_fork_status (struct thread_info *thread) > { > - const target_waitstatus &ws > - = (thread->has_pending_waitstatus () > - ? thread->pending_waitstatus () > - : thread->pending_follow); > + const target_waitstatus &ws = thread_pending_status (thread); > > if (!is_fork_status (ws.kind ())) > return nullptr; > @@ -5910,6 +5924,20 @@ thread_pending_fork_status (struct thread_info *thread) > return &ws; > } > > +/* Return THREAD's pending status if is is a pending fork/vfork/clone > + event, else return nullptr. */ > + > +static const target_waitstatus * > +thread_pending_child_status (thread_info *thread) > +{ > + const target_waitstatus &ws = thread_pending_status (thread); > + > + if (!is_new_child_status (ws.kind ())) > + return nullptr; > + > + return &ws; > +} > + > /* Detach the specified process. */ > > void > @@ -6075,6 +6103,12 @@ remote_target::follow_fork (inferior *child_inf, ptid_t child_ptid, > } > } > > +void > +remote_target::follow_clone (ptid_t child_ptid) > +{ > + remote_add_thread (child_ptid, false, false, false); > +} > + > /* Target follow-exec function for remote targets. Save EXECD_PATHNAME > in the program space of the new inferior. */ > > @@ -6807,10 +6841,10 @@ remote_target::commit_resumed () > if (priv->get_resume_state () == resume_state::RESUMED_PENDING_VCONT) > any_pending_vcont_resume = true; > > - /* If a thread is the parent of an unfollowed fork, then we > - can't do a global wildcard, as that would resume the fork > - child. */ > - if (thread_pending_fork_status (tp) != nullptr) > + /* If a thread is the parent of an unfollowed fork/vfork/clone, > + then we can't do a global wildcard, as that would resume the > + pending child. */ > + if (thread_pending_child_status (tp) != nullptr) > may_global_wildcard_vcont = false; > } > > @@ -7276,22 +7310,22 @@ struct notif_client notif_client_stop = > REMOTE_NOTIF_STOP, > }; > > -/* If CONTEXT contains any fork child threads that have not been > - reported yet, remove them from the CONTEXT list. If such a > - thread exists it is because we are stopped at a fork catchpoint > - and have not yet called follow_fork, which will set up the > - host-side data structures for the new process. */ > +/* If CONTEXT contains any fork/vfork/clone child threads that have > + not been reported yet, remove them from the CONTEXT list. If such > + a thread exists it is because we are stopped at a fork/vfork/clone > + catchpoint and have not yet called follow_fork/follow_clone, which > + will set up the host-side data structures for the new child. */ > > void > -remote_target::remove_new_fork_children (threads_listing_context *context) > +remote_target::remove_new_children (threads_listing_context *context) > { > struct notif_client *notif = ¬if_client_stop; > > - /* For any threads stopped at a fork event, remove the corresponding > - fork child threads from the CONTEXT list. */ > + /* For any threads stopped at a (v)fork/clone event, remove the > + corresponding child threads from the CONTEXT list. */ > for (thread_info *thread : all_non_exited_threads (this)) > { > - const target_waitstatus *ws = thread_pending_fork_status (thread); > + const target_waitstatus *ws = thread_pending_child_status (thread); > > if (ws == nullptr) > continue; > @@ -7299,13 +7333,12 @@ remote_target::remove_new_fork_children (threads_listing_context *context) > context->remove_thread (ws->child_ptid ()); > } > > - /* Check for any pending fork events (not reported or processed yet) > - in process PID and remove those fork child threads from the > - CONTEXT list as well. */ > + /* Check for any pending (v)fork/clone events (not reported or > + processed yet) in process PID and remove those child threads from > + the CONTEXT list as well. */ > remote_notif_get_pending_events (notif); > for (auto &event : get_remote_state ()->stop_reply_queue) > - if (event->ws.kind () == TARGET_WAITKIND_FORKED > - || event->ws.kind () == TARGET_WAITKIND_VFORKED) > + if (is_new_child_status (event->ws.kind ())) > context->remove_thread (event->ws.child_ptid ()); > else if (event->ws.kind () == TARGET_WAITKIND_THREAD_EXITED) > context->remove_thread (event->ptid); > @@ -7634,6 +7667,8 @@ Packet: '%s'\n"), > event->ws.set_forked (read_ptid (++p1, &p)); > else if (strprefix (p, p1, "vfork")) > event->ws.set_vforked (read_ptid (++p1, &p)); > + else if (strprefix (p, p1, "clone")) > + event->ws.set_thread_cloned (read_ptid (++p1, &p)); > else if (strprefix (p, p1, "vforkdone")) > { > event->ws.set_vfork_done (); > diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc > index 2ddb275bd15..6a673cb1ca8 100644 > --- a/gdbserver/remote-utils.cc > +++ b/gdbserver/remote-utils.cc > @@ -1062,6 +1062,7 @@ prepare_resume_reply (char *buf, ptid_t ptid, const target_waitstatus &status) > case TARGET_WAITKIND_FORKED: > case TARGET_WAITKIND_VFORKED: > case TARGET_WAITKIND_VFORK_DONE: > + case TARGET_WAITKIND_THREAD_CLONED: > case TARGET_WAITKIND_EXECD: > case TARGET_WAITKIND_THREAD_CREATED: > case TARGET_WAITKIND_SYSCALL_ENTRY: > @@ -1071,13 +1072,30 @@ prepare_resume_reply (char *buf, ptid_t ptid, const target_waitstatus &status) > struct regcache *regcache; > char *buf_start = buf; > > - if ((status.kind () == TARGET_WAITKIND_FORKED && cs.report_fork_events) > + if ((status.kind () == TARGET_WAITKIND_FORKED > + && cs.report_fork_events) > || (status.kind () == TARGET_WAITKIND_VFORKED > - && cs.report_vfork_events)) > + && cs.report_vfork_events) > + || status.kind () == TARGET_WAITKIND_THREAD_CLONED) > { > enum gdb_signal signal = GDB_SIGNAL_TRAP; > - const char *event = (status.kind () == TARGET_WAITKIND_FORKED > - ? "fork" : "vfork"); > + > + auto kind_remote_str = [] (target_waitkind kind) > + { > + switch (kind) > + { > + case TARGET_WAITKIND_FORKED: > + return "fork"; > + case TARGET_WAITKIND_VFORKED: > + return "vfork"; > + case TARGET_WAITKIND_THREAD_CLONED: > + return "clone"; > + default: > + gdb_assert_not_reached ("unhandled kind"); > + } > + }; > + > + const char *event = kind_remote_str (status.kind ()); > > sprintf (buf, "T%02x%s:", signal, event); > buf += strlen (buf); > diff --git a/gdbserver/server.cc b/gdbserver/server.cc > index aaef38e0062..56b7f97a388 100644 > --- a/gdbserver/server.cc > +++ b/gdbserver/server.cc > @@ -236,7 +236,8 @@ in_queued_stop_replies_ptid (struct notif_event *event, ptid_t filter_ptid) > > /* Don't resume fork children that GDB does not know about yet. */ > if ((vstop_event->status.kind () == TARGET_WAITKIND_FORKED > - || vstop_event->status.kind () == TARGET_WAITKIND_VFORKED) > + || vstop_event->status.kind () == TARGET_WAITKIND_VFORKED > + || vstop_event->status.kind () == TARGET_WAITKIND_THREAD_CLONED) > && vstop_event->status.child_ptid ().matches (filter_ptid)) > return true; > > -- > 2.36.0