[08/31] Thread options & clone events (core + remote)
Commit Message
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 | 143 ++++++++++++++++++++++++++++++++++++++++-
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, 454 insertions(+), 2 deletions(-)
Comments
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.
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?
Best,
Lancelot.
> + if (tp->ptid != magic_null_ptid)
> + {
> + *p++ = ':';
> + p = write_ptid (p, endp, tp->ptid);
> + }
> + }
> +
> + *p++ = '\0';
> +
> + putpkt (rs->buf);
> + getpkt (&rs->buf, 0);
> +
> + switch (packet_ok (rs->buf,
> + &remote_protocol_packets[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;
> + }
> +}
> +
> static void
> show_remote_cmd (const char *args, int from_tty)
> {
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. */
+ 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
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
On 2023-06-06 14:29, Andrew Burgess wrote:
>> @@ -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?
Yes. I fixed this.
>
> Otherwise LGTM.
>
> Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Thanks!
@@ -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. */
+ gdb_thread_options m_thread_options;
};
using thread_info_resumed_with_pending_wait_status_node
@@ -1583,7 +1583,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);
@@ -1885,6 +1884,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
@@ -1929,6 +1950,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--;
@@ -2422,6 +2445,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));
@@ -6090,6 +6149,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 ();
}
@@ -385,6 +385,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
@@ -421,6 +425,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;
@@ -547,6 +553,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;
@@ -837,6 +845,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 ();
@@ -2169,6 +2180,9 @@ enum {
/* Support for the QThreadEvents packet. */
PACKET_QThreadEvents,
+ /* Support for the QThreadOptions packet. */
+ PACKET_QThreadOptions,
+
/* Support for multi-process extensions. */
PACKET_multiprocess_feature,
@@ -5290,7 +5304,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 ();
@@ -5327,6 +5342,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 ();
+
+ remote_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,
@@ -5429,6 +5487,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 },
@@ -5523,6 +5583,9 @@ remote_target::remote_query_supported ()
if (packet_set_cmd_state (PACKET_QThreadEvents) != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "QThreadEvents+");
+ if (packet_set_cmd_state (PACKET_QThreadOptions) != AUTO_BOOLEAN_FALSE)
+ remote_query_supported_append (&q, "QThreadOptions+");
+
if (packet_set_cmd_state (PACKET_no_resumed) != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "no-resumed+");
@@ -6599,6 +6662,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
@@ -6761,6 +6826,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
@@ -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)));
+ if (tp->ptid != magic_null_ptid)
+ {
+ *p++ = ':';
+ p = write_ptid (p, endp, tp->ptid);
+ }
+ }
+
+ *p++ = '\0';
+
+ putpkt (rs->buf);
+ getpkt (&rs->buf, 0);
+
+ switch (packet_ok (rs->buf,
+ &remote_protocol_packets[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;
+ }
+}
+
static void
show_remote_cmd (const char *args, int from_tty)
{
@@ -15313,6 +15451,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_QThreadEvents],
"QThreadEvents", "thread-events", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_QThreadOptions],
+ "QThreadOptions", "thread-options", 0);
+
add_packet_config_cmd (&remote_protocol_packets[PACKET_no_resumed],
"N stop reply", "no-resumed-stop-reply", 0);
@@ -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)
@@ -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 ()
{
@@ -4389,6 +4389,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;
@@ -731,6 +731,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 ()
@@ -1894,6 +1898,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;
@@ -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);
+}
@@ -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
@@ -398,6 +398,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)
{
@@ -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;
@@ -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"
@@ -611,6 +612,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
@@ -892,6 +904,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:");
@@ -2364,6 +2484,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
@@ -2490,6 +2612,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+");
@@ -530,6 +530,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 ()
{
@@ -277,6 +277,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 ();
@@ -532,6 +535,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 ()