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 BFC003858D33 for ; Tue, 6 Jun 2023 13:29:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BFC003858D33 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=1686058196; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Wbe3nTtGQcfnWzkC8dFuSgGZgF/py4QHqTjKvGoTgw0=; b=VqUbkf0l7XVNXspkVnavhpGJ0WzLEqnGI5H5cn4yX7Jd29JUou27SmvuSHHLkTT769wskA blOtZAUjb2P6HQT0RMCX2wYVEAXxTR6mqdtNycTt1mJqUub0DXFCSbfTfO/YjEAuoGPAKJ LGC16cFtGhW17gCkQ+N/MQdP6fIyvGM= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-300-iuuq_5RkOeym2Ms1wvPNYA-1; Tue, 06 Jun 2023 09:29:55 -0400 X-MC-Unique: iuuq_5RkOeym2Ms1wvPNYA-1 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-30af00323b0so2851799f8f.2 for ; Tue, 06 Jun 2023 06:29:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686058194; x=1688650194; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=alhiUTHusuPsV3NqwTbfxCvib0dgC25DtOO207Db89U=; b=BE34NZJAEUWMSryL7fPmG4HJ3J8cKPrMkcQgyyvZL66xsmwPKkeXzTeDOCNQViswu9 FELTendeuz1niA6yvIa1KeLq8tDgur1vsgEh4KCBisQYIWnrPe3OSMW/YsLc8sYYZiCg rTnsfEdLHiwXgdCf81tUc/Wgbg05tLBSPlxgXTOo/Rie06yYc6neZ0sUR5B9ydd9j4Cs NXXlMizdQhQbtHNWySKE79NDqdMPGuVtJ54RIOe3hhnyYH+LcN/FUJMPyPgsxNqxsK5z 5u9JtRDJvGPgEmUsQPx3V/5DVXnXT3fnF+SbCLB21vnrkYaF6ZYOJOgY2XFCbyD5Tbpr ef8g== X-Gm-Message-State: AC+VfDxBwldP0JjKRk0QKmjDJcLf4d83rgFjQ1XNnkecLYDLGGB+3Ox5 X1miJRcr64zayuf/QsL+uniNZ8ISrOvPp+neUkPe7y2+gfVCYTCzTXRiQ9FxA578KF2Xgd/TBbw ZWSh5nSjk90UTnIRdISlpMuyAaQ7LOw== X-Received: by 2002:a5d:6449:0:b0:306:28f4:963c with SMTP id d9-20020a5d6449000000b0030628f4963cmr1710095wrw.23.1686058193816; Tue, 06 Jun 2023 06:29:53 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5kaOy6gtOUWXrC8vlzGOigJdPYdIqwGSL+hfbGVs0L2jp1u0la7kkW7KoRj9GtuSfbO8GOZQ== X-Received: by 2002:a5d:6449:0:b0:306:28f4:963c with SMTP id d9-20020a5d6449000000b0030628f4963cmr1710083wrw.23.1686058193393; Tue, 06 Jun 2023 06:29:53 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id 6-20020a05600c228600b003f7ec896cefsm1052952wmf.8.2023.06.06.06.29.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Jun 2023 06:29:52 -0700 (PDT) From: Andrew Burgess To: Pedro Alves , Lancelot SIX Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 08/31] Thread options & clone events (core + remote) In-Reply-To: <47bca489-7d80-f2ab-f0df-f36d550304c4@palves.net> References: <20221212203101.1034916-1-pedro@palves.net> <20221212203101.1034916-9-pedro@palves.net> <20230131122503.x6aameca5zjuu7kp@ubuntu.lan> <47bca489-7d80-f2ab-f0df-f36d550304c4@palves.net> Date: Tue, 06 Jun 2023 14:29:51 +0100 Message-ID: <87mt1c3ei8.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_LOTSOFHASH,RCVD_IN_DNSWL_NONE,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-01-31 12:25 p.m., Lancelot SIX wrote: >> Hi, >>=20 >>> diff --git a/gdb/remote.c b/gdb/remote.c >>> index 41348a65dc4..9de8ed8a068 100644 >>> --- a/gdb/remote.c >>> +++ b/gdb/remote.c >>> @@ -14534,6 +14601,77 @@ remote_target::thread_events (int enable) >>> } >>> } >>> =20 >>> +/* Implementation of the supports_set_thread_options target >>> + method. */ >>> + >>> +bool >>> +remote_target::supports_set_thread_options (gdb_thread_options options= ) >>> +{ >>> + remote_state *rs =3D get_remote_state (); >>> + return (packet_support (PACKET_QThreadOptions) =3D=3D PACKET_ENABLE >>> +=09 && (rs->supported_thread_options & options) =3D=3D options); >>> +} >>> + >>> +/* For coalescing reasons, actually sending the options to the target >>> + happens at resume time, via this function. See target_resume for >>> + all-stop, and target_commit_resumed for non-stop. */ >>> + >>> +void >>> +remote_target::commit_requested_thread_options () >>> +{ >>> + struct remote_state *rs =3D get_remote_state (); >>> + >>> + if (packet_support (PACKET_QThreadOptions) !=3D PACKET_ENABLE) >>> + return; >>> + >>> + char *p =3D rs->buf.data (); >>> + char *endp =3D p + get_remote_packet_size (); >>> + >>> + /* Clear options for all threads by default. Note that unlike >>> + vCont, the rightmost options that match a thread apply, so we >>> + don't have to worry about whether we can use wildcard ptids. */ >>> + strcpy (p, "QThreadOptions;0"); >>> + p +=3D strlen (p); >>> + >>> + /* Now set non-zero options for threads that need them. We don't >>> + bother with the case of all threads of a process wanting the same >>> + non-zero options as that's not an expected scenario. */ >>> + for (thread_info *tp : all_non_exited_threads (this)) >>> + { >>> + gdb_thread_options options =3D tp->thread_options (); >>> + >>> + if (options =3D=3D 0) >>> +=09continue; >>> + >>> + *p++ =3D ';'; >>> + p +=3D xsnprintf (p, endp - p, "%s", phex_nz (options, sizeof (o= ptions))); >>=20 >> I am not super familiar with how big the buffer is guaranteed to be. >> Can we imagine a situation where the number of thread and options to >> send exceed the packet size capacity? If so, this seems dangerous. 'p' >> would be incremented by the size which would have been necessary to do >> the print, so it means it could now point past the end of the buffer. > > Note that xsnprintf has an assertion that ensures that the string fits: > > int > xsnprintf (char *str, size_t size, const char *format, ...) > { > va_list args; > int ret; > > va_start (args, format); > ret =3D vsnprintf (str, size, format, args); > gdb_assert (ret < size); <<<< here > va_end (args); > > >> Even the `*p++'=3D ';'` above and similar `*p++ =3D` below are subject t= o >> overflow if the number of options to encode grow too high. >>=20 >> See man vsnprintf(3) which is used by xsnprintf: >>=20 >> The functions snprintf() and vsnprintf() do not write more than size >> bytes[...]. If the output was truncated due to this limit, then >> the return value is the number of characters [...] which would have >> been written to the final string if enough space had been >> available. >>=20 >> As I do not feel that we can have a guaranty regarding the maximum >> number of non exited threads with non-0 options (I might be wrong, but >> the set of options can be extended so this can show in the future), >> I=E2=80=AFwould check the returned value of xsnprintf before adding it t= o p (the >> same might apply to remote_target::write_ptid, and other increments to p= ). >>=20 >> Did I miss some precondition which guarantee the buffer to be big enough= ? > > Nope. You've missed my laziness. :-) > > Here's a version of the patch that sends QThreadOptions packets increment= ally > if needed. This is the same thing we do for vCont actions (in vcont_buil= der::push_action). > > I've tested the flush/restart path with a local hack to force the path al= l the time: > > size_t osize =3D obuf_p - obuf; > - if (osize > endp - p) > + if (1 || osize > endp - p) > { > > I force-pushed the whole series to users/palves/step-over-thread-exit-v3.= 1, > with this updated patch. > > Let me know what you think. > > Pedro Alves > > From 10cf06f133fb69b093dc74a515db34410be8af40 Mon Sep 17 00:00:00 2001 > From: Pedro Alves > Date: Tue, 23 Nov 2021 20:35:12 +0000 > Subject: [PATCH] Thread options & clone events (core + remote) > > A 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. > > However, for remote debugging, it wouldn't be ideal for GDBserver to > report every clone event to GDB, when GDB only cares about such events > in some specific situations. Reporting clone events all the time > would be potentially chatty. We don't enable thread create/exit > events all the time for the same reason. Instead we have the > QThreadEvents packet. QThreadEvents is target-wide, though. > > This patch makes GDB instead explicitly request that the target > reports clone events or not, on a per-thread basis. > > In order to be able to do that with GDBserver, we need a new remote > protocol feature. Since a following patch will want to enable thread > exit events on per-thread basis too, the packet introduced here is > more generic than just for clone events. It lets you enable/disable a > set of options at once, modelled on Linux ptrace's PTRACE_SETOPTIONS. > > IOW, this commit introduces a new QThreadOptions packet, that lets you > specify a set of per-thread event options you want to enable. The > packet accepts a list of options/thread-id pairs, similarly to vCont, > processed left to right, with the options field being a number > interpreted as a bit mask of options. The only option defined in this > commit is GDB_THREAD_OPTION_CLONE (0x1), which ask the remote target > to report clone events. Another patch later in the series will > introduce another option. > > For example, this packet sets option "1" (clone events) on thread > p1000.2345: > > QThreadOptions;1:p1000.2345 > > and this clears options for all threads of process 1000, and then sets > option "1" (clone events) on thread p1000.2345: > > QThreadOptions;0:p1000.-1;1:p1000.2345 > > This clears options of all threads of all processes: > > QThreadOptions;0 > > The target reports the set of supported options by including > "QThreadOptions=3D" in its qSupported response. > > infrun is then tweaked to enable GDB_THREAD_OPTION_CLONE when stepping > over a breakpoint. > > Unlike PTRACE_SETOPTIONS, fork/vfork/clone children do NOT inherit > their parent's thread options. This is so that GDB can send e.g., > "QThreadOptions;0;1:TID" without worrying about threads it doesn't > know about yet. > > Documentation for this new remote protocol feature is included in a > documentation patch later in the series. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D19675 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D27830 > Change-Id: Ie41e5093b2573f14cf6ac41b0b5804eba75be37e > --- > gdb/gdbthread.h | 16 ++++ > gdb/infrun.c | 63 +++++++++++++- > gdb/remote.c | 182 ++++++++++++++++++++++++++++++++++++++++- > gdb/target-debug.h | 2 + > gdb/target-delegates.c | 28 +++++++ > gdb/target.c | 9 ++ > gdb/target.h | 8 ++ > gdb/target/target.c | 11 +++ > gdb/target/target.h | 16 ++++ > gdb/thread.c | 15 ++++ > gdbserver/gdbthread.h | 3 + > gdbserver/server.cc | 130 +++++++++++++++++++++++++++++ > gdbserver/target.cc | 6 ++ > gdbserver/target.h | 6 ++ > 14 files changed, 493 insertions(+), 2 deletions(-) > > diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h > index c0f27a8a66e..79dedb23d4d 100644 > --- a/gdb/gdbthread.h > +++ b/gdb/gdbthread.h > @@ -28,6 +28,7 @@ struct symtab; > #include "ui-out.h" > #include "btrace.h" > #include "target/waitstatus.h" > +#include "target/target.h" > #include "cli/cli-utils.h" > #include "gdbsupport/refcounted-object.h" > #include "gdbsupport/common-gdbthread.h" > @@ -470,6 +471,17 @@ class thread_info : public refcounted_object, > m_thread_fsm =3D std::move (fsm); > } > =20 > + /* Record the thread options last set for this thread. */ > + > + void set_thread_options (gdb_thread_options thread_options); > + > + /* Get the thread options last set for this thread. */ > + > + gdb_thread_options thread_options () const > + { > + return m_thread_options; > + } > + > int current_line =3D 0; > struct symtab *current_symtab =3D NULL; > =20 > @@ -577,6 +589,10 @@ class thread_info : public refcounted_object, > left to do for the thread's execution command after the target > stops. Several execution commands use it. */ > std::unique_ptr m_thread_fsm; > + > + /* The thread options as last set with a call to > + target_set_thread_options. */ I think: s/target_set_thread_options/set_thread_options/ here? Otherwise LGTM. Reviewed-By: Andrew Burgess Thanks, Andrew > + gdb_thread_options m_thread_options; > }; > =20 > using thread_info_resumed_with_pending_wait_status_node > diff --git a/gdb/infrun.c b/gdb/infrun.c > index d1e6233591c..e5f8dc8d8ab 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -1584,7 +1584,6 @@ step_over_info_valid_p (void) > /* Return true if THREAD is doing a displaced step. */ > =20 > static bool > -ATTRIBUTE_UNUSED > displaced_step_in_progress_thread (thread_info *thread) > { > gdb_assert (thread !=3D nullptr); > @@ -1886,6 +1885,28 @@ displaced_step_prepare (thread_info *thread) > return status; > } > =20 > +/* Maybe disable thread-{cloned,created,exited} event reporting after > + a step-over (either in-line or displaced) finishes. */ > + > +static void > +update_thread_events_after_step_over (thread_info *event_thread) > +{ > + if (target_supports_set_thread_options (0)) > + { > + /* We can control per-thread options. Disable events for the > +=09 event thread. */ > + event_thread->set_thread_options (0); > + } > + else > + { > + /* We can only control the target-wide target_thread_events > +=09 setting. Disable it, but only if other threads don't need it > +=09 enabled. */ > + if (!displaced_step_in_progress_any_thread ()) > +=09target_thread_events (false); > + } > +} > + > /* If we displaced stepped an instruction successfully, adjust registers= and > memory to yield the same effect the instruction would have had if we = had > executed it at its original address, and return > @@ -1930,6 +1951,8 @@ displaced_step_finish (thread_info *event_thread, > if (!displaced->in_progress ()) > return DISPLACED_STEP_FINISH_STATUS_OK; > =20 > + update_thread_events_after_step_over (event_thread); > + > gdb_assert (event_thread->inf->displaced_step_state.in_progress_count = > 0); > event_thread->inf->displaced_step_state.in_progress_count--; > =20 > @@ -2423,6 +2446,42 @@ do_target_resume (ptid_t resume_ptid, bool step, e= num gdb_signal sig) > else > target_pass_signals (signal_pass); > =20 > + /* Request that the target report thread-{created,cloned} events in > + the following situations: > + > + - If we are performing an in-line step-over-breakpoint, then we > + will remove a breakpoint from the target and only run the > + current thread. We don't want any new thread (spawned by the > + step) to start running, as it might miss the breakpoint. > + > + - If we are stepping over a breakpoint out of line (displaced > + stepping) then we won't remove a breakpoint from the target, > + but, if the step spawns a new clone thread, then we will need > + to fixup the $pc address in the clone child too, so we need it > + to start stopped. > + */ > + if (step_over_info_valid_p () > + || displaced_step_in_progress_thread (tp)) > + { > + gdb_thread_options options =3D GDB_THREAD_OPTION_CLONE; > + if (target_supports_set_thread_options (options)) > +=09tp->set_thread_options (options); > + else > +=09target_thread_events (true); > + } > + > + /* If we're resuming more than one thread simultaneously, then any > + thread other than the leader is being set to run free. Clear any > + previous thread option for those threads. */ > + if (resume_ptid !=3D inferior_ptid && target_supports_set_thread_optio= ns (0)) > + { > + process_stratum_target *resume_target =3D tp->inf->process_target = (); > + for (thread_info *thr_iter : all_non_exited_threads (resume_target= , > +=09=09=09=09=09=09=09 resume_ptid)) > +=09if (thr_iter !=3D tp) > +=09 thr_iter->set_thread_options (0); > + } > + > infrun_debug_printf ("resume_ptid=3D%s, step=3D%d, sig=3D%s", > =09=09 resume_ptid.to_string ().c_str (), > =09=09 step, gdb_signal_to_symbol_string (sig)); > @@ -6144,6 +6203,8 @@ finish_step_over (struct execution_control_state *e= cs) > =09 back an event. */ > gdb_assert (ecs->event_thread->control.trap_expected); > =20 > + update_thread_events_after_step_over (ecs->event_thread); > + > clear_step_over_info (); > } > =20 > diff --git a/gdb/remote.c b/gdb/remote.c > index ed9c2a0946a..62f25b03928 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -248,6 +248,9 @@ enum { > /* Support for the QThreadEvents packet. */ > PACKET_QThreadEvents, > =20 > + /* Support for the QThreadOptions packet. */ > + PACKET_QThreadOptions, > + > /* Support for multi-process extensions. */ > PACKET_multiprocess_feature, > =20 > @@ -560,6 +563,10 @@ class remote_state > this can go away. */ > int wait_forever_enabled_p =3D 1; > =20 > + /* The set of thread options the target reported it supports, via > + qSupported. */ > + gdb_thread_options supported_thread_options =3D 0; > + > private: > /* Mapping of remote protocol data for each gdbarch. Usually there > is only one entry here, though we may see more with stubs that > @@ -720,6 +727,8 @@ class remote_target : public process_stratum_target > void detach (inferior *, int) override; > void disconnect (const char *, int) override; > =20 > + void commit_requested_thread_options (); > + > void commit_resumed () override; > void resume (ptid_t, int, enum gdb_signal) override; > ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) ov= erride; > @@ -846,6 +855,8 @@ class remote_target : public process_stratum_target > =20 > void thread_events (int) override; > =20 > + bool supports_set_thread_options (gdb_thread_options) override; > + > int can_do_single_step () override; > =20 > void terminal_inferior () override; > @@ -1144,6 +1155,9 @@ class remote_target : public process_stratum_target > =20 > void remote_packet_size (const protocol_feature *feature, > =09=09=09 packet_support support, const char *value); > + void remote_supported_thread_options (const protocol_feature *feature, > +=09=09=09=09=09enum packet_support support, > +=09=09=09=09=09const char *value); > =20 > void remote_serial_quit_handler (); > =20 > @@ -5471,7 +5485,8 @@ remote_supported_packet (remote_target *remote, > =20 > void > remote_target::remote_packet_size (const protocol_feature *feature, > -=09=09=09=09 enum packet_support support, const char *value) > +=09=09=09=09 enum packet_support support, > +=09=09=09=09 const char *value) > { > struct remote_state *rs =3D get_remote_state (); > =20 > @@ -5508,6 +5523,49 @@ remote_packet_size (remote_target *remote, const p= rotocol_feature *feature, > remote->remote_packet_size (feature, support, value); > } > =20 > +void > +remote_target::remote_supported_thread_options (const protocol_feature *= feature, > +=09=09=09=09=09=09enum packet_support support, > +=09=09=09=09=09=09const char *value) > +{ > + struct remote_state *rs =3D get_remote_state (); > + > + m_features.m_protocol_packets[feature->packet].support =3D support; > + > + if (support !=3D PACKET_ENABLE) > + return; > + > + if (value =3D=3D nullptr || *value =3D=3D '\0') > + { > + warning (_("Remote target reported \"%s\" without supported option= s."), > +=09 feature->name); > + return; > + } > + > + ULONGEST options =3D 0; > + const char *p =3D unpack_varlen_hex (value, &options); > + > + if (*p !=3D '\0') > + { > + warning (_("Remote target reported \"%s\" with " > +=09=09 "bad thread options: \"%s\"."), > +=09 feature->name, value); > + return; > + } > + > + /* Record the set of supported options. */ > + rs->supported_thread_options =3D (gdb_thread_option) options; > +} > + > +static void > +remote_supported_thread_options (remote_target *remote, > +=09=09=09=09 const protocol_feature *feature, > +=09=09=09=09 enum packet_support support, > +=09=09=09=09 const char *value) > +{ > + remote->remote_supported_thread_options (feature, support, value); > +} > + > static const struct protocol_feature remote_protocol_features[] =3D { > { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 }, > { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet, > @@ -5610,6 +5668,8 @@ static const struct protocol_feature remote_protoco= l_features[] =3D { > PACKET_Qbtrace_conf_pt_size }, > { "vContSupported", PACKET_DISABLE, remote_supported_packet, PACKET_vC= ontSupported }, > { "QThreadEvents", PACKET_DISABLE, remote_supported_packet, PACKET_QTh= readEvents }, > + { "QThreadOptions", PACKET_DISABLE, remote_supported_thread_options, > + PACKET_QThreadOptions }, > { "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_res= umed }, > { "memory-tagging", PACKET_DISABLE, remote_supported_packet, > PACKET_memory_tagging_feature }, > @@ -5712,6 +5772,10 @@ remote_target::remote_query_supported () > =09 !=3D AUTO_BOOLEAN_FALSE) > =09remote_query_supported_append (&q, "QThreadEvents+"); > =20 > + if (m_features.packet_set_cmd_state (PACKET_QThreadOptions) > +=09 !=3D AUTO_BOOLEAN_FALSE) > +=09remote_query_supported_append (&q, "QThreadOptions+"); > + > if (m_features.packet_set_cmd_state (PACKET_no_resumed) > =09 !=3D AUTO_BOOLEAN_FALSE) > =09remote_query_supported_append (&q, "no-resumed+"); > @@ -6784,6 +6848,8 @@ remote_target::resume (ptid_t scope_ptid, int step,= enum gdb_signal siggnal) > return; > } > =20 > + commit_requested_thread_options (); > + > /* In all-stop, we can't mark REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN > (explained in remote-notif.c:handle_notification) so > remote_notif_process is not called. We need find a place where > @@ -6946,6 +7012,8 @@ remote_target::commit_resumed () > if (!target_is_non_stop_p () || ::execution_direction =3D=3D EXEC_REVE= RSE) > return; > =20 > + commit_requested_thread_options (); > + > /* Try to send wildcard actions ("vCont;c" or "vCont;c:pPID.-1") > instead of resuming all threads of each process individually. > However, if any thread of a process must remain halted, we can't > @@ -14706,6 +14774,115 @@ remote_target::thread_events (int enable) > } > } > =20 > +/* Implementation of the supports_set_thread_options target > + method. */ > + > +bool > +remote_target::supports_set_thread_options (gdb_thread_options options) > +{ > + remote_state *rs =3D get_remote_state (); > + return (m_features.packet_support (PACKET_QThreadOptions) =3D=3D PACKE= T_ENABLE > +=09 && (rs->supported_thread_options & options) =3D=3D options); > +} > + > +/* For coalescing reasons, actually sending the options to the target > + happens at resume time, via this function. See target_resume for > + all-stop, and target_commit_resumed for non-stop. */ > + > +void > +remote_target::commit_requested_thread_options () > +{ > + struct remote_state *rs =3D get_remote_state (); > + > + if (m_features.packet_support (PACKET_QThreadOptions) !=3D PACKET_ENAB= LE) > + return; > + > + char *p =3D rs->buf.data (); > + char *endp =3D p + get_remote_packet_size (); > + > + /* Clear options for all threads by default. Note that unlike > + vCont, the rightmost options that match a thread apply, so we > + don't have to worry about whether we can use wildcard ptids. */ > + strcpy (p, "QThreadOptions;0"); > + p +=3D strlen (p); > + > + /* Send the QThreadOptions packet stored in P. */ > + auto flush =3D [&] () > + { > + *p++ =3D '\0'; > + > + putpkt (rs->buf); > + getpkt (&rs->buf, 0); > + > + switch (m_features.packet_ok (rs->buf, PACKET_QThreadOptions)) > +=09{ > +=09case PACKET_OK: > +=09 if (strcmp (rs->buf.data (), "OK") !=3D 0) > +=09 error (_("Remote refused setting thread options: %s"), rs->buf.da= ta ()); > +=09 break; > +=09case PACKET_ERROR: > +=09 error (_("Remote failure reply: %s"), rs->buf.data ()); > +=09case PACKET_UNKNOWN: > +=09 gdb_assert_not_reached ("PACKET_UNKNOWN"); > +=09 break; > +=09} > + }; > + > + /* Prepare P for another QThreadOptions packet. */ > + auto restart =3D [&] () > + { > + p =3D rs->buf.data (); > + strcpy (p, "QThreadOptions"); > + p +=3D strlen (p); > + }; > + > + /* Now set non-zero options for threads that need them. We don't > + bother with the case of all threads of a process wanting the same > + non-zero options as that's not an expected scenario. */ > + for (thread_info *tp : all_non_exited_threads (this)) > + { > + gdb_thread_options options =3D tp->thread_options (); > + > + if (options =3D=3D 0) > +=09continue; > + > + /* It might be possible to we have more threads with options > +=09 than can fit a single QThreadOptions packet. So build each > +=09 options/thread pair in this separate buffer to make sure it > +=09 fits. */ > + constexpr size_t max_options_size =3D 100; > + char obuf[max_options_size]; > + char *obuf_p =3D obuf; > + char *obuf_endp =3D obuf + max_options_size; > + > + *obuf_p++ =3D ';'; > + obuf_p +=3D xsnprintf (obuf_p, obuf_endp - obuf_p, "%s", > +=09=09=09 phex_nz (options, sizeof (options))); > + if (tp->ptid !=3D magic_null_ptid) > +=09{ > +=09 *obuf_p++ =3D ':'; > +=09 obuf_p =3D write_ptid (obuf_p, obuf_endp, tp->ptid); > +=09} > + > + size_t osize =3D obuf_p - obuf; > + if (osize > endp - p) > +=09{ > +=09 /* This new options/thread pair doesn't fit the packet > +=09 buffer. Send what we have already. */ > +=09 flush (); > +=09 restart (); > + > +=09 /* Should now fit. */ > +=09 gdb_assert (osize <=3D endp - p); > +=09} > + > + memcpy (p, obuf, osize); > + p +=3D osize; > + } > + > + flush (); > +} > + > static void > show_remote_cmd (const char *args, int from_tty) > { > @@ -15446,6 +15623,9 @@ Show the maximum size of the address (in bits) in= a memory packet."), NULL, > add_packet_config_cmd (PACKET_QThreadEvents, "QThreadEvents", "thread-= events", > =09=09=09 0); > =20 > + add_packet_config_cmd (PACKET_QThreadOptions, "QThreadOptions", > +=09=09=09 "thread-options", 0); > + > add_packet_config_cmd (PACKET_no_resumed, "N stop reply", > =09=09=09 "no-resumed-stop-reply", 0); > =20 > diff --git a/gdb/target-debug.h b/gdb/target-debug.h > index acb01d47e7c..72fb31f4b59 100644 > --- a/gdb/target-debug.h > +++ b/gdb/target-debug.h > @@ -176,6 +176,8 @@ > target_debug_do_print (X.get ()) > #define target_debug_print_target_waitkind(X) \ > target_debug_do_print (pulongest (X)) > +#define target_debug_print_gdb_thread_options(X) \ > + target_debug_do_print (to_string (X).c_str ()) > =20 > static void > target_debug_print_struct_target_waitstatus_p (struct target_waitstatus = *status) > diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c > index 7a4ef05b4e1..63338d82834 100644 > --- a/gdb/target-delegates.c > +++ b/gdb/target-delegates.c > @@ -106,6 +106,7 @@ struct dummy_target : public target_ops > int async_wait_fd () override; > bool has_pending_events () override; > void thread_events (int arg0) override; > + bool supports_set_thread_options (gdb_thread_options arg0) override; > bool supports_non_stop () override; > bool always_non_stop_p () override; > int find_memory_regions (find_memory_region_ftype arg0, void *arg1) ov= erride; > @@ -281,6 +282,7 @@ struct debug_target : public target_ops > int async_wait_fd () override; > bool has_pending_events () override; > void thread_events (int arg0) override; > + bool supports_set_thread_options (gdb_thread_options arg0) override; > bool supports_non_stop () override; > bool always_non_stop_p () override; > int find_memory_regions (find_memory_region_ftype arg0, void *arg1) ov= erride; > @@ -2272,6 +2274,32 @@ debug_target::thread_events (int arg0) > gdb_puts (")\n", gdb_stdlog); > } > =20 > +bool > +target_ops::supports_set_thread_options (gdb_thread_options arg0) > +{ > + return this->beneath ()->supports_set_thread_options (arg0); > +} > + > +bool > +dummy_target::supports_set_thread_options (gdb_thread_options arg0) > +{ > + return false; > +} > + > +bool > +debug_target::supports_set_thread_options (gdb_thread_options arg0) > +{ > + bool result; > + gdb_printf (gdb_stdlog, "-> %s->supports_set_thread_options (...)\n", = this->beneath ()->shortname ()); > + result =3D this->beneath ()->supports_set_thread_options (arg0); > + gdb_printf (gdb_stdlog, "<- %s->supports_set_thread_options (", this->= beneath ()->shortname ()); > + target_debug_print_gdb_thread_options (arg0); > + gdb_puts (") =3D ", gdb_stdlog); > + target_debug_print_bool (result); > + gdb_puts ("\n", gdb_stdlog); > + return result; > +} > + > bool > target_ops::supports_non_stop () > { > diff --git a/gdb/target.c b/gdb/target.c > index 9835222e5da..d1d3b6913fc 100644 > --- a/gdb/target.c > +++ b/gdb/target.c > @@ -4358,6 +4358,15 @@ target_thread_events (int enable) > current_inferior ()->top_target ()->thread_events (enable); > } > =20 > +/* See target.h. */ > + > +bool > +target_supports_set_thread_options (gdb_thread_options options) > +{ > + inferior *inf =3D current_inferior (); > + return inf->top_target ()->supports_set_thread_options (options); > +} > + > /* Controls if targets can report that they can/are async. This is > just for maintainers to use when debugging gdb. */ > bool target_async_permitted =3D true; > diff --git a/gdb/target.h b/gdb/target.h > index 58e24a5c28e..1657fe2fba7 100644 > --- a/gdb/target.h > +++ b/gdb/target.h > @@ -736,6 +736,10 @@ struct target_ops > TARGET_DEFAULT_RETURN (false); > virtual void thread_events (int) > TARGET_DEFAULT_IGNORE (); > + /* Returns true if the target supports setting thread options > + OPTIONS, false otherwise. */ > + virtual bool supports_set_thread_options (gdb_thread_options options= ) > + TARGET_DEFAULT_RETURN (false); > /* This method must be implemented in some situations. See the > comment on 'can_run'. */ > virtual bool supports_non_stop () > @@ -1895,6 +1899,10 @@ extern void target_async (bool enable); > /* Enables/disables thread create and exit events. */ > extern void target_thread_events (int enable); > =20 > +/* Returns true if the target supports setting thread options > + OPTIONS. */ > +extern bool target_supports_set_thread_options (gdb_thread_options optio= ns); > + > /* Whether support for controlling the target backends always in > non-stop mode is enabled. */ > extern enum auto_boolean target_non_stop_enabled; > diff --git a/gdb/target/target.c b/gdb/target/target.c > index 8089918f1d0..3af7d73df5a 100644 > --- a/gdb/target/target.c > +++ b/gdb/target/target.c > @@ -188,3 +188,14 @@ target_read_string (CORE_ADDR memaddr, int len, int = *bytes_read) > =20 > return gdb::unique_xmalloc_ptr ((char *) buffer.release ()); > } > + > +/* See target/target.h. */ > + > +std::string > +to_string (gdb_thread_options options) > +{ > + static constexpr gdb_thread_options::string_mapping mapping[] =3D { > + MAP_ENUM_FLAG (GDB_THREAD_OPTION_CLONE), > + }; > + return options.to_string (mapping); > +} > diff --git a/gdb/target/target.h b/gdb/target/target.h > index d1a18ee2212..2691f92e4ef 100644 > --- a/gdb/target/target.h > +++ b/gdb/target/target.h > @@ -22,9 +22,25 @@ > =20 > #include "target/waitstatus.h" > #include "target/wait.h" > +#include "gdbsupport/enum-flags.h" > =20 > /* This header is a stopgap until more code is shared. */ > =20 > +/* Available thread options. Keep this in sync with to_string, in > + target.c. */ > + > +enum gdb_thread_option : unsigned > +{ > + /* Tell the target to report TARGET_WAITKIND_THREAD_CLONED events > + for the thread. */ > + GDB_THREAD_OPTION_CLONE =3D 1 << 0, > +}; > + > +DEF_ENUM_FLAGS_TYPE (enum gdb_thread_option, gdb_thread_options); > + > +/* Convert gdb_thread_option to a string. */ > +extern std::string to_string (gdb_thread_options options); > + > /* Read LEN bytes of target memory at address MEMADDR, placing the > results in GDB's memory at MYADDR. Return zero for success, > nonzero if any error occurs. This function must be provided by > diff --git a/gdb/thread.c b/gdb/thread.c > index 5b472150a62..8958ce1000b 100644 > --- a/gdb/thread.c > +++ b/gdb/thread.c > @@ -399,6 +399,21 @@ thread_info::clear_pending_waitstatus () > =20 > /* See gdbthread.h. */ > =20 > +void > +thread_info::set_thread_options (gdb_thread_options thread_options) > +{ > + if (m_thread_options =3D=3D thread_options) > + return; > + > + m_thread_options =3D thread_options; > + > + infrun_debug_printf ("[options for %s are now %s]", > +=09=09 this->ptid.to_string ().c_str (), > +=09=09 to_string (thread_options).c_str ()); > +} > + > +/* See gdbthread.h. */ > + > int > thread_is_in_step_over_chain (struct thread_info *tp) > { > diff --git a/gdbserver/gdbthread.h b/gdbserver/gdbthread.h > index 493e1dbf6cb..a4dff0fe1a2 100644 > --- a/gdbserver/gdbthread.h > +++ b/gdbserver/gdbthread.h > @@ -80,6 +80,9 @@ struct thread_info > =20 > /* Branch trace target information for this thread. */ > struct btrace_target_info *btrace =3D nullptr; > + > + /* Thread options GDB requested with QThreadOptions. */ > + gdb_thread_options thread_options =3D 0; > }; > =20 > extern std::list all_threads; > diff --git a/gdbserver/server.cc b/gdbserver/server.cc > index 29f2d2a386c..7f7efd1fcc0 100644 > --- a/gdbserver/server.cc > +++ b/gdbserver/server.cc > @@ -36,6 +36,7 @@ > #include "dll.h" > #include "hostio.h" > #include > +#include > #include "gdbsupport/common-inferior.h" > #include "gdbsupport/job-control.h" > #include "gdbsupport/environ.h" > @@ -616,6 +617,17 @@ parse_store_memtags_request (char *request, CORE_ADD= R *addr, size_t *len, > return true; > } > =20 > +/* Parse thread options starting at *P and return them. On exit, > + advance *P past the options. */ > + > +static gdb_thread_options > +parse_gdb_thread_options (const char **p) > +{ > + ULONGEST options =3D 0; > + *p =3D unpack_varlen_hex (*p, &options); > + return (gdb_thread_option) options; > +} > + > /* Handle all of the extended 'Q' packets. */ > =20 > static void > @@ -897,6 +909,114 @@ handle_general_set (char *own_buf) > return; > } > =20 > + if (startswith (own_buf, "QThreadOptions;")) > + { > + const char *p =3D own_buf + strlen ("QThreadOptions"); > + > + gdb_thread_options supported_options =3D target_supported_thread_o= ptions (); > + if (supported_options =3D=3D 0) > +=09{ > +=09 /* Something went wrong -- we don't support any option, but > +=09 GDB sent the packet anyway. */ > +=09 write_enn (own_buf); > +=09 return; > +=09} > + > + /* We could store the options directly in thread->thread_options > +=09 without this map, but that would mean that a QThreadOptions > +=09 packet with a wildcard like "QThreadOptions;0;3:TID" would > +=09 result in the debug logs showing: > + > +=09 [options for TID are now 0x0] > +=09 [options for TID are now 0x3] > + > +=09 It's nicer if we only print the final options for each TID, > +=09 and if we only print about it if the options changed compared > +=09 to the options that were previously set on the thread. */ > + std::unordered_map set_options; > + > + while (*p !=3D '\0') > +=09{ > +=09 if (p[0] !=3D ';') > +=09 { > +=09 write_enn (own_buf); > +=09 return; > +=09 } > +=09 p++; > + > +=09 /* Read the options. */ > + > +=09 gdb_thread_options options =3D parse_gdb_thread_options (&p); > + > +=09 if ((options & ~supported_options) !=3D 0) > +=09 { > +=09 /* GDB asked for an unknown or unsupported option, so > +=09=09 error out. */ > +=09 std::string err > +=09=09=3D string_printf ("E.Unknown thread options requested: %s\n", > +=09=09=09=09 to_string (options).c_str ()); > +=09 strcpy (own_buf, err.c_str ()); > +=09 return; > +=09 } > + > +=09 ptid_t ptid; > + > +=09 if (p[0] =3D=3D ';' || p[0] =3D=3D '\0') > +=09 ptid =3D minus_one_ptid; > +=09 else if (p[0] =3D=3D ':') > +=09 { > +=09 const char *q; > + > +=09 ptid =3D read_ptid (p + 1, &q); > + > +=09 if (p =3D=3D q) > +=09=09{ > +=09=09 write_enn (own_buf); > +=09=09 return; > +=09=09} > +=09 p =3D q; > +=09 if (p[0] !=3D ';' && p[0] !=3D '\0') > +=09=09{ > +=09=09 write_enn (own_buf); > +=09=09 return; > +=09=09} > +=09 } > +=09 else > +=09 { > +=09 write_enn (own_buf); > +=09 return; > +=09 } > + > +=09 /* Convert PID.-1 =3D> PID.0 for ptid.matches. */ > +=09 if (ptid.lwp () =3D=3D -1) > +=09 ptid =3D ptid_t (ptid.pid ()); > + > +=09 for_each_thread ([&] (thread_info *thread) > +=09 { > +=09 if (ptid_of (thread).matches (ptid)) > +=09=09set_options[thread] =3D options; > +=09 }); > +=09} > + > + for (const auto &iter : set_options) > +=09{ > +=09 thread_info *thread =3D iter.first; > +=09 gdb_thread_options options =3D iter.second; > + > +=09 if (thread->thread_options !=3D options) > +=09 { > +=09 threads_debug_printf ("[options for %s are now %s]\n", > +=09=09=09=09 target_pid_to_str (ptid_of (thread)).c_str (), > +=09=09=09=09 to_string (options).c_str ()); > + > +=09 thread->thread_options =3D options; > +=09 } > +=09} > + > + write_ok (own_buf); > + return; > + } > + > if (startswith (own_buf, "QStartupWithShell:")) > { > const char *value =3D own_buf + strlen ("QStartupWithShell:"); > @@ -2348,6 +2468,8 @@ handle_query (char *own_buf, int packet_len, int *n= ew_packet_len_p) > =09=09cs.vCont_supported =3D 1; > =09 else if (feature =3D=3D "QThreadEvents+") > =09=09; > +=09 else if (feature =3D=3D "QThreadOptions+") > +=09=09; > =09 else if (feature =3D=3D "no-resumed+") > =09=09{ > =09=09 /* GDB supports and wants TARGET_WAITKIND_NO_RESUMED > @@ -2474,6 +2596,14 @@ handle_query (char *own_buf, int packet_len, int *= new_packet_len_p) > =20 > strcat (own_buf, ";vContSupported+"); > =20 > + gdb_thread_options supported_options =3D target_supported_thread_o= ptions (); > + if (supported_options !=3D 0) > +=09{ > +=09 char *end_buf =3D own_buf + strlen (own_buf); > +=09 sprintf (end_buf, ";QThreadOptions=3D%s", > +=09=09 phex_nz (supported_options, sizeof (supported_options))); > +=09} > + > strcat (own_buf, ";QThreadEvents+"); > =20 > strcat (own_buf, ";no-resumed+"); > diff --git a/gdbserver/target.cc b/gdbserver/target.cc > index f8e592d20c3..1c740bbf583 100644 > --- a/gdbserver/target.cc > +++ b/gdbserver/target.cc > @@ -532,6 +532,12 @@ process_stratum_target::supports_vfork_events () > return false; > } > =20 > +gdb_thread_options > +process_stratum_target::supported_thread_options () > +{ > + return 0; > +} > + > bool > process_stratum_target::supports_exec_events () > { > diff --git a/gdbserver/target.h b/gdbserver/target.h > index d993e361b76..fe68716c868 100644 > --- a/gdbserver/target.h > +++ b/gdbserver/target.h > @@ -276,6 +276,9 @@ class process_stratum_target > /* Returns true if vfork events are supported. */ > virtual bool supports_vfork_events (); > =20 > + /* Returns the set of supported thread options. */ > + virtual gdb_thread_options supported_thread_options (); > + > /* Returns true if exec events are supported. */ > virtual bool supports_exec_events (); > =20 > @@ -531,6 +534,9 @@ int kill_inferior (process_info *proc); > #define target_supports_vfork_events() \ > the_target->supports_vfork_events () > =20 > +#define target_supported_thread_options(options) \ > + the_target->supported_thread_options (options) > + > #define target_supports_exec_events() \ > the_target->supports_exec_events () > =20 > > base-commit: 2562954ede66f32bff7d985e752b8052c2ae5775 > prerequisite-patch-id: bbc9918ac5f79de07a29f34ec072794d270f942d > prerequisite-patch-id: c0bc5b4f99193bb50cb31f551673de1808dcda35 > prerequisite-patch-id: ab0838f2bc02931d7e830abe23833e7a8224442c > prerequisite-patch-id: 3a896bfe4b7c66a2e3a6aa668c5ae8395e5d8a52 > prerequisite-patch-id: 254a23b7d7cec889924daaf288304494c93fe1aa > prerequisite-patch-id: b1fe92da846e52cce1e9f13498cf668c5cdd6ee4 > --=20 > 2.36.0