Re: [PATCH 08/31] Thread options & clone events (core + remote)

[Andrew Burgess <aburgess@redhat.com>]

Pedro Alves <pedro@palves.net> writes:

> On 2023-01-31 12:25 p.m., Lancelot SIX wrote:
>> Hi,
>> 
>>> 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)
>>>      }
>>>  }
>>>  
>>> +/* Implementation of the supports_set_thread_options target
>>> +   method.  */
>>> +
>>> +bool
>>> +remote_target::supports_set_thread_options (gdb_thread_options options)
>>> +{
>>> +  remote_state *rs = get_remote_state ();
>>> +  return (packet_support (PACKET_QThreadOptions) == PACKET_ENABLE
>>> +	  && (rs->supported_thread_options & options) == 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 = get_remote_state ();
>>> +
>>> +  if (packet_support (PACKET_QThreadOptions) != PACKET_ENABLE)
>>> +    return;
>>> +
>>> +  char *p = rs->buf.data ();
>>> +  char *endp = 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 += 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 = tp->thread_options ();
>>> +
>>> +      if (options == 0)
>>> +	continue;
>>> +
>>> +      *p++ = ';';
>>> +      p += xsnprintf (p, endp - p, "%s", phex_nz (options, sizeof (options)));
>> 
>> 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 = vsnprintf (str, size, format, args);
>   gdb_assert (ret < size);                           <<<< here
>   va_end (args);
>
>
>> Even the `*p++'= ';'` above and similar `*p++ =` below are subject to
>> overflow if the number of options to encode grow too high.
>> 
>> See man vsnprintf(3) which is used by xsnprintf:
>> 
>>     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.
>> 
>> 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 would check the returned value of xsnprintf before adding it to p (the
>> same might apply to remote_target::write_ptid, and other increments to p).
>> 
>> 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 incrementally
> if needed.  This is the same thing we do for vCont actions (in vcont_builder::push_action).
>
> I've tested the flush/restart path with a local hack to force the path all the time:
>
>         size_t osize = 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 <pedro@palves.net>
> 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=<supported options>" 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=19675
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830
> 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 = std::move (fsm);
>    }
>  
> +  /* 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 = 0;
>    struct symtab *current_symtab = NULL;
>  
> @@ -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<struct thread_fsm> 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 <aburgess@redhat.com>

Thanks,
Andrew

> +  gdb_thread_options m_thread_options;
>  };
>  
>  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.  */
>  
>  static bool
> -ATTRIBUTE_UNUSED
>  displaced_step_in_progress_thread (thread_info *thread)
>  {
>    gdb_assert (thread != nullptr);
> @@ -1886,6 +1885,28 @@ displaced_step_prepare (thread_info *thread)
>    return status;
>  }
>  
> +/* 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
> +	 event thread.  */
> +      event_thread->set_thread_options (0);
> +    }
> +  else
> +    {
> +      /* We can only control the target-wide target_thread_events
> +	 setting.  Disable it, but only if other threads don't need it
> +	 enabled.  */
> +      if (!displaced_step_in_progress_any_thread ())
> +	target_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;
>  
> +  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--;
>  
> @@ -2423,6 +2446,42 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig)
>    else
>      target_pass_signals (signal_pass);
>  
> +  /* 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 = GDB_THREAD_OPTION_CLONE;
> +      if (target_supports_set_thread_options (options))
> +	tp->set_thread_options (options);
> +      else
> +	target_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 != inferior_ptid && target_supports_set_thread_options (0))
> +    {
> +      process_stratum_target *resume_target = tp->inf->process_target ();
> +      for (thread_info *thr_iter : all_non_exited_threads (resume_target,
> +							   resume_ptid))
> +	if (thr_iter != tp)
> +	  thr_iter->set_thread_options (0);
> +    }
> +
>    infrun_debug_printf ("resume_ptid=%s, step=%d, sig=%s",
>  		       resume_ptid.to_string ().c_str (),
>  		       step, gdb_signal_to_symbol_string (sig));
> @@ -6144,6 +6203,8 @@ finish_step_over (struct execution_control_state *ecs)
>  	 back an event.  */
>        gdb_assert (ecs->event_thread->control.trap_expected);
>  
> +      update_thread_events_after_step_over (ecs->event_thread);
> +
>        clear_step_over_info ();
>      }
>  
> 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,
>  
> +  /* Support for the QThreadOptions packet.  */
> +  PACKET_QThreadOptions,
> +
>    /* Support for multi-process extensions.  */
>    PACKET_multiprocess_feature,
>  
> @@ -560,6 +563,10 @@ class remote_state
>       this can go away.  */
>    int wait_forever_enabled_p = 1;
>  
> +  /* The set of thread options the target reported it supports, via
> +     qSupported.  */
> +  gdb_thread_options supported_thread_options = 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;
>  
> +  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) override;
> @@ -846,6 +855,8 @@ class remote_target : public process_stratum_target
>  
>    void thread_events (int) override;
>  
> +  bool supports_set_thread_options (gdb_thread_options) override;
> +
>    int can_do_single_step () override;
>  
>    void terminal_inferior () override;
> @@ -1144,6 +1155,9 @@ class remote_target : public process_stratum_target
>  
>    void remote_packet_size (const protocol_feature *feature,
>  			   packet_support support, const char *value);
> +  void remote_supported_thread_options (const protocol_feature *feature,
> +					enum packet_support support,
> +					const char *value);
>  
>    void remote_serial_quit_handler ();
>  
> @@ -5471,7 +5485,8 @@ remote_supported_packet (remote_target *remote,
>  
>  void
>  remote_target::remote_packet_size (const protocol_feature *feature,
> -				   enum packet_support support, const char *value)
> +				   enum packet_support support,
> +				   const char *value)
>  {
>    struct remote_state *rs = get_remote_state ();
>  
> @@ -5508,6 +5523,49 @@ remote_packet_size (remote_target *remote, const protocol_feature *feature,
>    remote->remote_packet_size (feature, support, value);
>  }
>  
> +void
> +remote_target::remote_supported_thread_options (const protocol_feature *feature,
> +						enum packet_support support,
> +						const char *value)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +
> +  m_features.m_protocol_packets[feature->packet].support = support;
> +
> +  if (support != PACKET_ENABLE)
> +    return;
> +
> +  if (value == nullptr || *value == '\0')
> +    {
> +      warning (_("Remote target reported \"%s\" without supported options."),
> +	       feature->name);
> +      return;
> +    }
> +
> +  ULONGEST options = 0;
> +  const char *p = unpack_varlen_hex (value, &options);
> +
> +  if (*p != '\0')
> +    {
> +      warning (_("Remote target reported \"%s\" with "
> +		 "bad thread options: \"%s\"."),
> +	       feature->name, value);
> +      return;
> +    }
> +
> +  /* Record the set of supported options.  */
> +  rs->supported_thread_options = (gdb_thread_option) options;
> +}
> +
> +static void
> +remote_supported_thread_options (remote_target *remote,
> +				 const protocol_feature *feature,
> +				 enum packet_support support,
> +				 const char *value)
> +{
> +  remote->remote_supported_thread_options (feature, support, value);
> +}
> +
>  static const struct protocol_feature remote_protocol_features[] = {
>    { "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_protocol_features[] = {
>      PACKET_Qbtrace_conf_pt_size },
>    { "vContSupported", PACKET_DISABLE, remote_supported_packet, PACKET_vContSupported },
>    { "QThreadEvents", PACKET_DISABLE, remote_supported_packet, PACKET_QThreadEvents },
> +  { "QThreadOptions", PACKET_DISABLE, remote_supported_thread_options,
> +    PACKET_QThreadOptions },
>    { "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed },
>    { "memory-tagging", PACKET_DISABLE, remote_supported_packet,
>      PACKET_memory_tagging_feature },
> @@ -5712,6 +5772,10 @@ remote_target::remote_query_supported ()
>  	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "QThreadEvents+");
>  
> +      if (m_features.packet_set_cmd_state (PACKET_QThreadOptions)
> +	  != AUTO_BOOLEAN_FALSE)
> +	remote_query_supported_append (&q, "QThreadOptions+");
> +
>        if (m_features.packet_set_cmd_state (PACKET_no_resumed)
>  	  != AUTO_BOOLEAN_FALSE)
>  	remote_query_supported_append (&q, "no-resumed+");
> @@ -6784,6 +6848,8 @@ remote_target::resume (ptid_t scope_ptid, int step, enum gdb_signal siggnal)
>        return;
>      }
>  
> +  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 == EXEC_REVERSE)
>      return;
>  
> +  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)
>      }
>  }
>  
> +/* Implementation of the supports_set_thread_options target
> +   method.  */
> +
> +bool
> +remote_target::supports_set_thread_options (gdb_thread_options options)
> +{
> +  remote_state *rs = get_remote_state ();
> +  return (m_features.packet_support (PACKET_QThreadOptions) == PACKET_ENABLE
> +	  && (rs->supported_thread_options & options) == 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 = get_remote_state ();
> +
> +  if (m_features.packet_support (PACKET_QThreadOptions) != PACKET_ENABLE)
> +    return;
> +
> +  char *p = rs->buf.data ();
> +  char *endp = 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 += strlen (p);
> +
> +  /* Send the QThreadOptions packet stored in P.  */
> +  auto flush = [&] ()
> +    {
> +      *p++ = '\0';
> +
> +      putpkt (rs->buf);
> +      getpkt (&rs->buf, 0);
> +
> +      switch (m_features.packet_ok (rs->buf, PACKET_QThreadOptions))
> +	{
> +	case PACKET_OK:
> +	  if (strcmp (rs->buf.data (), "OK") != 0)
> +	    error (_("Remote refused setting thread options: %s"), rs->buf.data ());
> +	  break;
> +	case PACKET_ERROR:
> +	  error (_("Remote failure reply: %s"), rs->buf.data ());
> +	case PACKET_UNKNOWN:
> +	  gdb_assert_not_reached ("PACKET_UNKNOWN");
> +	  break;
> +	}
> +    };
> +
> +  /* Prepare P for another QThreadOptions packet.  */
> +  auto restart = [&] ()
> +    {
> +      p = rs->buf.data ();
> +      strcpy (p, "QThreadOptions");
> +      p += 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 = tp->thread_options ();
> +
> +      if (options == 0)
> +	continue;
> +
> +      /* It might be possible to we have more threads with options
> +	 than can fit a single QThreadOptions packet.  So build each
> +	 options/thread pair in this separate buffer to make sure it
> +	 fits.  */
> +      constexpr size_t max_options_size = 100;
> +      char obuf[max_options_size];
> +      char *obuf_p = obuf;
> +      char *obuf_endp = obuf + max_options_size;
> +
> +      *obuf_p++ = ';';
> +      obuf_p += xsnprintf (obuf_p, obuf_endp - obuf_p, "%s",
> +			   phex_nz (options, sizeof (options)));
> +      if (tp->ptid != magic_null_ptid)
> +	{
> +	  *obuf_p++ = ':';
> +	  obuf_p = write_ptid (obuf_p, obuf_endp, tp->ptid);
> +	}
> +
> +      size_t osize = obuf_p - obuf;
> +      if (osize > endp - p)
> +	{
> +	  /* This new options/thread pair doesn't fit the packet
> +	     buffer.  Send what we have already.  */
> +	  flush ();
> +	  restart ();
> +
> +	  /* Should now fit.  */
> +	  gdb_assert (osize <= endp - p);
> +	}
> +
> +      memcpy (p, obuf, osize);
> +      p += 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",
>  			 0);
>  
> +  add_packet_config_cmd (PACKET_QThreadOptions, "QThreadOptions",
> +			 "thread-options", 0);
> +
>    add_packet_config_cmd (PACKET_no_resumed, "N stop reply",
>  			 "no-resumed-stop-reply", 0);
>  
> 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 ())
>  
>  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) override;
> @@ -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) override;
> @@ -2272,6 +2274,32 @@ debug_target::thread_events (int arg0)
>    gdb_puts (")\n", gdb_stdlog);
>  }
>  
> +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 = 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 (") = ", 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);
>  }
>  
> +/* See target.h.  */
> +
> +bool
> +target_supports_set_thread_options (gdb_thread_options options)
> +{
> +  inferior *inf = 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 = 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);
>  
> +/* Returns true if the target supports setting thread options
> +   OPTIONS.  */
> +extern bool target_supports_set_thread_options (gdb_thread_options options);
> +
>  /* 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)
>  
>    return gdb::unique_xmalloc_ptr<char> ((char *) buffer.release ());
>  }
> +
> +/* See target/target.h.  */
> +
> +std::string
> +to_string (gdb_thread_options options)
> +{
> +  static constexpr gdb_thread_options::string_mapping mapping[] = {
> +    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 @@
>  
>  #include "target/waitstatus.h"
>  #include "target/wait.h"
> +#include "gdbsupport/enum-flags.h"
>  
>  /* This header is a stopgap until more code is shared.  */
>  
> +/* 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 = 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 ()
>  
>  /* See gdbthread.h.  */
>  
> +void
> +thread_info::set_thread_options (gdb_thread_options thread_options)
> +{
> +  if (m_thread_options == thread_options)
> +    return;
> +
> +  m_thread_options = thread_options;
> +
> +  infrun_debug_printf ("[options for %s are now %s]",
> +		       this->ptid.to_string ().c_str (),
> +		       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
>  
>    /* Branch trace target information for this thread.  */
>    struct btrace_target_info *btrace = nullptr;
> +
> +  /* Thread options GDB requested with QThreadOptions.  */
> +  gdb_thread_options thread_options = 0;
>  };
>  
>  extern std::list<thread_info *> 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 <vector>
> +#include <unordered_map>
>  #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_ADDR *addr, size_t *len,
>    return true;
>  }
>  
> +/* 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 = 0;
> +  *p = unpack_varlen_hex (*p, &options);
> +  return (gdb_thread_option) options;
> +}
> +
>  /* Handle all of the extended 'Q' packets.  */
>  
>  static void
> @@ -897,6 +909,114 @@ handle_general_set (char *own_buf)
>        return;
>      }
>  
> +  if (startswith (own_buf, "QThreadOptions;"))
> +    {
> +      const char *p = own_buf + strlen ("QThreadOptions");
> +
> +      gdb_thread_options supported_options = target_supported_thread_options ();
> +      if (supported_options == 0)
> +	{
> +	  /* Something went wrong -- we don't support any option, but
> +	     GDB sent the packet anyway.  */
> +	  write_enn (own_buf);
> +	  return;
> +	}
> +
> +      /* We could store the options directly in thread->thread_options
> +	 without this map, but that would mean that a QThreadOptions
> +	 packet with a wildcard like "QThreadOptions;0;3:TID" would
> +	 result in the debug logs showing:
> +
> +	   [options for TID are now 0x0]
> +	   [options for TID are now 0x3]
> +
> +	 It's nicer if we only print the final options for each TID,
> +	 and if we only print about it if the options changed compared
> +	 to the options that were previously set on the thread.  */
> +      std::unordered_map<thread_info *, gdb_thread_options> set_options;
> +
> +      while (*p != '\0')
> +	{
> +	  if (p[0] != ';')
> +	    {
> +	      write_enn (own_buf);
> +	      return;
> +	    }
> +	  p++;
> +
> +	  /* Read the options.  */
> +
> +	  gdb_thread_options options = parse_gdb_thread_options (&p);
> +
> +	  if ((options & ~supported_options) != 0)
> +	    {
> +	      /* GDB asked for an unknown or unsupported option, so
> +		 error out.  */
> +	      std::string err
> +		= string_printf ("E.Unknown thread options requested: %s\n",
> +				 to_string (options).c_str ());
> +	      strcpy (own_buf, err.c_str ());
> +	      return;
> +	    }
> +
> +	  ptid_t ptid;
> +
> +	  if (p[0] == ';' || p[0] == '\0')
> +	    ptid = minus_one_ptid;
> +	  else if (p[0] == ':')
> +	    {
> +	      const char *q;
> +
> +	      ptid = read_ptid (p + 1, &q);
> +
> +	      if (p == q)
> +		{
> +		  write_enn (own_buf);
> +		  return;
> +		}
> +	      p = q;
> +	      if (p[0] != ';' && p[0] != '\0')
> +		{
> +		  write_enn (own_buf);
> +		  return;
> +		}
> +	    }
> +	  else
> +	    {
> +	      write_enn (own_buf);
> +	      return;
> +	    }
> +
> +	  /* Convert PID.-1 => PID.0 for ptid.matches.  */
> +	  if (ptid.lwp () == -1)
> +	    ptid = ptid_t (ptid.pid ());
> +
> +	  for_each_thread ([&] (thread_info *thread)
> +	    {
> +	      if (ptid_of (thread).matches (ptid))
> +		set_options[thread] = options;
> +	    });
> +	}
> +
> +      for (const auto &iter : set_options)
> +	{
> +	  thread_info *thread = iter.first;
> +	  gdb_thread_options options = iter.second;
> +
> +	  if (thread->thread_options != options)
> +	    {
> +	      threads_debug_printf ("[options for %s are now %s]\n",
> +				    target_pid_to_str (ptid_of (thread)).c_str (),
> +				    to_string (options).c_str ());
> +
> +	      thread->thread_options = options;
> +	    }
> +	}
> +
> +      write_ok (own_buf);
> +      return;
> +    }
> +
>    if (startswith (own_buf, "QStartupWithShell:"))
>      {
>        const char *value = own_buf + strlen ("QStartupWithShell:");
> @@ -2348,6 +2468,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
>  		cs.vCont_supported = 1;
>  	      else if (feature == "QThreadEvents+")
>  		;
> +	      else if (feature == "QThreadOptions+")
> +		;
>  	      else if (feature == "no-resumed+")
>  		{
>  		  /* 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)
>  
>        strcat (own_buf, ";vContSupported+");
>  
> +      gdb_thread_options supported_options = target_supported_thread_options ();
> +      if (supported_options != 0)
> +	{
> +	  char *end_buf = own_buf + strlen (own_buf);
> +	  sprintf (end_buf, ";QThreadOptions=%s",
> +		   phex_nz (supported_options, sizeof (supported_options)));
> +	}
> +
>        strcat (own_buf, ";QThreadEvents+");
>  
>        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;
>  }
>  
> +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 ();
>  
> +  /* 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 ();
>  
> @@ -531,6 +534,9 @@ int kill_inferior (process_info *proc);
>  #define target_supports_vfork_events() \
>    the_target->supports_vfork_events ()
>  
> +#define target_supported_thread_options(options) \
> +  the_target->supported_thread_options (options)
> +
>  #define target_supports_exec_events() \
>    the_target->supports_exec_events ()
>  
>
> 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
> -- 
> 2.36.0