* [PATCH v2 0/3] Apply fixme notes for multi-target support
@ 2022-03-29 13:11 Christina Schimpe
2022-03-29 13:11 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Christina Schimpe @ 2022-03-29 13:11 UTC (permalink / raw)
To: gdb-patches
Hi all,
This is the V2 for the series "Apply fixme notes for multi-target support" based
on the feedback of Tom and Andrew. I am sorry that it took so long to get to.
V1 of this series can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-January/185091.html.
Changes for patch #1:
* Remove obsolete 'remote_state *' parameters.
* Extract packet number using the 'context' attached to the command.
* Enhance commit message.
* Adapt the logging for the 'show remote PACKET-NAME' and added logging for the
'set remote PACKET-NAME' commands for the new command behavior. I chose a very
similar logging to the one Andrew suggested in the review for V1 of this series.
* Adapt tests for the new logging.
* Add NEWS entry.
* Update the documentation.
* Add comments.
* Move smaller functions inside the new struct remote_features.
Changes for patch #2:
* Adapt the behavior of the 'show remote memory-read/write-packet-size' commands
such that they behave similar to the previous patch e.g.
"The show command always displays the current remote target's
configuration. If no remote target is selected the default
configuration for future connections is shown".
* Adapt the commit message accordingly.
* Adapt the logging for the 'set remote memory-read/write-packet-size' and added
logging for the 'set remote memory-read/write-packet-size' commands.
* Adapt tests for the new logging.
* Add NEWS entry.
* Update the documentation.
* Add comments.
* Cosmetic changes.
I did not make any changes to patch #3 of this series
(https://sourceware.org/pipermail/gdb-patches/2022-January/185090.html).
Reasoning for this is described in
https://sourceware.org/pipermail/gdb-patches/2022-January/185560.html.
Please let me know if you have comments on the patch or if it is acceptable as is.
Thanks,
Christina
Christina Schimpe (3):
gdb: Make global feature array a per-remote target array
gdb: Add per-remote target variables for memory read and write config
gdb: Remove workaround for the vCont packet
gdb/NEWS | 25 +
gdb/doc/gdb.texinfo | 24 +-
gdb/remote.c | 1469 +++++++++--------
gdb/testsuite/gdb.base/cond-eval-mode.exp | 9 +-
gdb/testsuite/gdb.base/dprintf.exp | 5 +-
gdb/testsuite/gdb.base/find-unmapped.exp | 5 +-
.../gdb.base/hbreak-in-shr-unsupported.exp | 4 +-
gdb/testsuite/gdb.base/remote.exp | 45 +-
.../gdb.multi/multi-target-info-inferiors.exp | 6 +-
gdb/testsuite/gdb.multi/multi-target.exp.tcl | 8 +-
.../connect-without-multi-process.exp | 4 +-
.../gdb.server/exit-multiple-threads.exp | 9 +-
gdb/testsuite/gdb.server/ext-restart.exp | 5 +-
gdb/testsuite/gdb.server/ext-wrapper.exp | 5 +-
gdb/testsuite/gdb.server/server-exec-info.exp | 5 +-
gdb/testsuite/gdb.server/server-kill.exp | 4 +-
.../gdb.server/stop-reply-no-thread-multi.exp | 8 +-
.../gdb.server/stop-reply-no-thread.exp | 9 +-
.../process-dies-while-handling-bp.exp | 9 +-
gdb/testsuite/gdb.trace/change-loc.exp | 4 +-
gdb/testsuite/gdb.trace/qtro.exp | 6 +-
21 files changed, 976 insertions(+), 692 deletions(-)
--
2.25.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] gdb: Make global feature array a per-remote target array
2022-03-29 13:11 [PATCH v2 0/3] Apply fixme notes for multi-target support Christina Schimpe
@ 2022-03-29 13:11 ` Christina Schimpe
2022-03-29 13:45 ` Eli Zaretskii
` (2 more replies)
2022-03-29 13:11 ` [PATCH v2 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
2022-03-29 13:11 ` [PATCH v2 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
2 siblings, 3 replies; 20+ messages in thread
From: Christina Schimpe @ 2022-03-29 13:11 UTC (permalink / raw)
To: gdb-patches
This patch applies the appropriate FIXME notes described in commit 5b6d1e4
"Multi-target support".
"You'll notice that remote.c includes some FIXME notes. These refer to
the fact that the global arrays that hold data for the remote packets
supported are still globals. For example, if we connect to two
different servers/stubs, then each might support different remote
protocol features. They might even be different architectures, like
e.g., one ARM baremetal stub, and a x86 gdbserver, to debug a
host/controller scenario as a single program. That isn't going to
work correctly today, because of said globals. I'm leaving fixing
that for another pass, since it does not appear to be trivial, and I'd
rather land the base work first. It's already useful to be able to
debug multiple instances of the same server (e.g., a distributed
cluster, where you have full control over the servers installed), so I
think as is it's already reasonable incremental progress."
Using this patch it is possible to configure per-remote targets'
feature packets.
Given the following setup for two gdbservers:
~~~~
gdbserver --multi :1234
gdbserver --disable-packet=vCont --multi :2345
~~~~
Before this patch configuring of range-stepping was not possible for one
of two connected remote targets with different support for the vCont
packet. As one of the targets supports vCont, it should be possible to
configure "set range-stepping". However, the output of GDB looks like:
(gdb) target extended-remote :1234
Remote debugging using :1234
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) target extended-remote :2345
Remote debugging using :2345
(gdb) set range-stepping on
warning: Range stepping is not supported by the current target
(gdb) inferior 1
[Switching to inferior 1 [<null>] (<noexec>)]
(gdb) set range-stepping on
warning: Range stepping is not supported by the current target
~~~~
Two warnings are shown. The warning for inferior 1 should not appear
as it is connected to a target supporting the vCont package.
~~~~
(gdb) target extended-remote :1234
Remote debugging using :1234
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) target extended-remote :2345
Remote debugging using :2345
(gdb) set range-stepping on
warning: Range stepping is not supported by the current target
(gdb) inferior 1
[Switching to inferior 1 [<null>] (<noexec>)]
(gdb) set range-stepping on
(gdb)
~~~~
Now only one warning is shown for inferior 2, which is connected to
a target not supporting vCont.
The per-remote target feature array is realized by a new class
remote_features, which stores the per-remote target array and
provides functions to determine supported features of the target.
A remote_target object now has a new member of that class.
Each time a new remote_target object is initialized, a new per-remote
target array is constructed based on the global remote_protocol_packets
array. The global array is initialized in the function _initialize_remote
and can be configured using the command line. However, the command line
configuration before this patch affected also existing remote connections
(due to the global feature array used by all remote targets). This
behavior is now different. Now the currently selected target's feature
array will be configured and future connections' feature packets. All
other existing remote targets' features are not affected.
The show command always displays the current remote target's
configuration. If no remote target is selected the default
configuration for future connections is shown.
If we have for instance the following setup with inferior 2 being
selected:
~~~~
(gdb) info inferiors
Num Description Connection Executable
1 <null> 1 (extended-remote :1234)
* 2 <null> 2 (extended-remote :2345)
~~~~
Before this patch, if we run 'set remote multiprocess-feature-packet', the
following configuration was set:
The feature array of all remote targets (in this setup the two connected
targets) and all future remote connections are affected.
After this patch, it will be configured as follows:
The feature array of target with port :2345 which is currently selected
and all future remote connections are affected. The show command 'show
remote multiprocess-feature-packet' will display the configuration of
target with port :2345.
It is therefore required to adapt the test
"gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp" to configure the
multiprocess-feature-packet before the connections are created.
To inform the gdb user about the new behaviour of the 'show remote
PACKET-NAME' commands and the new configuration impact for remote
targets using the 'set remote PACKET-NAME' commands the commands'
outputs are adapted. Due to this change it is required to adapt each
test using the set/show remote 'PACKET-NAME' commands.
---
gdb/NEWS | 13 +
gdb/doc/gdb.texinfo | 10 +-
gdb/remote.c | 1057 ++++++++++-------
gdb/testsuite/gdb.base/cond-eval-mode.exp | 9 +-
gdb/testsuite/gdb.base/dprintf.exp | 5 +-
gdb/testsuite/gdb.base/find-unmapped.exp | 5 +-
.../gdb.base/hbreak-in-shr-unsupported.exp | 4 +-
.../gdb.multi/multi-target-info-inferiors.exp | 6 +-
gdb/testsuite/gdb.multi/multi-target.exp.tcl | 8 +-
.../connect-without-multi-process.exp | 4 +-
.../gdb.server/exit-multiple-threads.exp | 9 +-
gdb/testsuite/gdb.server/ext-restart.exp | 5 +-
gdb/testsuite/gdb.server/ext-wrapper.exp | 5 +-
gdb/testsuite/gdb.server/server-exec-info.exp | 5 +-
gdb/testsuite/gdb.server/server-kill.exp | 4 +-
.../gdb.server/stop-reply-no-thread-multi.exp | 8 +-
.../gdb.server/stop-reply-no-thread.exp | 9 +-
.../process-dies-while-handling-bp.exp | 9 +-
gdb/testsuite/gdb.trace/change-loc.exp | 4 +-
gdb/testsuite/gdb.trace/qtro.exp | 6 +-
20 files changed, 695 insertions(+), 490 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index e10062752d..7740162d3e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,19 @@
*** Changes since GDB 12
+* Multi-target feature configuration
+
+ GDB now supports the individual configuration of remote target's feature
+ sets. Based on the current selection of a target, the commands 'set remote
+ <name>-packet (on|off|auto)' and 'show remote <name>-packet' can be used to
+ configure a target's feature packet and to display its configuration,
+ respectively.
+
+ The configuration of the packet itself applies to the currently selected
+ target (if available) and future remote connections. The show commands print
+ the configuration of the currently selected target. If no remote target is
+ selected, the default configuration for future connections is shown.
+
* GDB now supports hardware watchpoints on FreeBSD/Aarch64.
* Remove support for building against Python 2, it is now only possible to
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 729f9d79a9..2c5e4d1b1b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23364,8 +23364,14 @@ in @value{GDBN}. You may want to report the problem to the
@value{GDBN} developers.
For each packet @var{name}, the command to enable or disable the
-packet is @code{set remote @var{name}-packet}. The available settings
-are:
+packet is @code{set remote @var{name}-packet}. If you configure a packet, the
+configuration will apply for all future remote targets. In case there is a
+target selected, also the configuration of the current target is changed. All
+other existing remote targets' features are not affected.
+The command to print the current configuration of a packet is
+@code{show remote @var{name}-packet}. It displays the current remote target's
+configuration. If no remote target is selected, the default configuration for
+future connections is shown. The available settings are:
@multitable @columnfractions 0.28 0.32 0.25
@item Command Name
diff --git a/gdb/remote.c b/gdb/remote.c
index aa6a67a96e..5d9d328968 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -71,12 +71,12 @@
#include "gdbsupport/agent.h"
#include "btrace.h"
#include "record-btrace.h"
-#include <algorithm>
#include "gdbsupport/scoped_restore.h"
#include "gdbsupport/environ.h"
#include "gdbsupport/byte-vector.h"
#include "gdbsupport/search.h"
#include <algorithm>
+#include <iterator>
#include <unordered_map>
#include "async-event.h"
#include "gdbsupport/selftest.h"
@@ -117,6 +117,36 @@ enum packet_support
PACKET_DISABLE
};
+/* Convert the packet support auto_boolean to a name used for gdb printing. */
+
+static const char *
+get_packet_support_name (auto_boolean support)
+{
+ switch (support)
+ {
+ case AUTO_BOOLEAN_TRUE:
+ return "on";
+ case AUTO_BOOLEAN_FALSE:
+ return "off";
+ case AUTO_BOOLEAN_AUTO:
+ return "auto";
+ default:
+ return "internal-error";
+ }
+}
+
+/* Convert the target type (future remote target or currently connected target)
+ to a name used for gdb printing. */
+
+static const char *
+get_target_type_name (bool target_connected)
+{
+ if (target_connected)
+ return "on the current remote target";
+ else
+ return "on newly created remote targets";
+}
+
/* Analyze a packet's return value and update the packet config
accordingly. */
@@ -127,6 +157,155 @@ enum packet_result
PACKET_UNKNOWN
};
+/* Enumeration of packets for a remote target. */
+
+enum {
+ PACKET_vCont = 0,
+ PACKET_X,
+ PACKET_qSymbol,
+ PACKET_P,
+ PACKET_p,
+ PACKET_Z0,
+ PACKET_Z1,
+ PACKET_Z2,
+ PACKET_Z3,
+ PACKET_Z4,
+ PACKET_vFile_setfs,
+ PACKET_vFile_open,
+ PACKET_vFile_pread,
+ PACKET_vFile_pwrite,
+ PACKET_vFile_close,
+ PACKET_vFile_unlink,
+ PACKET_vFile_readlink,
+ PACKET_vFile_fstat,
+ PACKET_qXfer_auxv,
+ PACKET_qXfer_features,
+ PACKET_qXfer_exec_file,
+ PACKET_qXfer_libraries,
+ PACKET_qXfer_libraries_svr4,
+ PACKET_qXfer_memory_map,
+ PACKET_qXfer_osdata,
+ PACKET_qXfer_threads,
+ PACKET_qXfer_statictrace_read,
+ PACKET_qXfer_traceframe_info,
+ PACKET_qXfer_uib,
+ PACKET_qGetTIBAddr,
+ PACKET_qGetTLSAddr,
+ PACKET_qSupported,
+ PACKET_qTStatus,
+ PACKET_QPassSignals,
+ PACKET_QCatchSyscalls,
+ PACKET_QProgramSignals,
+ PACKET_QSetWorkingDir,
+ PACKET_QStartupWithShell,
+ PACKET_QEnvironmentHexEncoded,
+ PACKET_QEnvironmentReset,
+ PACKET_QEnvironmentUnset,
+ PACKET_qCRC,
+ PACKET_qSearch_memory,
+ PACKET_vAttach,
+ PACKET_vRun,
+ PACKET_QStartNoAckMode,
+ PACKET_vKill,
+ PACKET_qXfer_siginfo_read,
+ PACKET_qXfer_siginfo_write,
+ PACKET_qAttached,
+
+ /* Support for conditional tracepoints. */
+ PACKET_ConditionalTracepoints,
+
+ /* Support for target-side breakpoint conditions. */
+ PACKET_ConditionalBreakpoints,
+
+ /* Support for target-side breakpoint commands. */
+ PACKET_BreakpointCommands,
+
+ /* Support for fast tracepoints. */
+ PACKET_FastTracepoints,
+
+ /* Support for static tracepoints. */
+ PACKET_StaticTracepoints,
+
+ /* Support for installing tracepoints while a trace experiment is
+ running. */
+ PACKET_InstallInTrace,
+
+ PACKET_bc,
+ PACKET_bs,
+ PACKET_TracepointSource,
+ PACKET_QAllow,
+ PACKET_qXfer_fdpic,
+ PACKET_QDisableRandomization,
+ PACKET_QAgent,
+ PACKET_QTBuffer_size,
+ PACKET_Qbtrace_off,
+ PACKET_Qbtrace_bts,
+ PACKET_Qbtrace_pt,
+ PACKET_qXfer_btrace,
+
+ /* Support for the QNonStop packet. */
+ PACKET_QNonStop,
+
+ /* Support for the QThreadEvents packet. */
+ PACKET_QThreadEvents,
+
+ /* Support for multi-process extensions. */
+ PACKET_multiprocess_feature,
+
+ /* Support for enabling and disabling tracepoints while a trace
+ experiment is running. */
+ PACKET_EnableDisableTracepoints_feature,
+
+ /* Support for collecting strings using the tracenz bytecode. */
+ PACKET_tracenz_feature,
+
+ /* Support for continuing to run a trace experiment while GDB is
+ disconnected. */
+ PACKET_DisconnectedTracing_feature,
+
+ /* Support for qXfer:libraries-svr4:read with a non-empty annex. */
+ PACKET_augmented_libraries_svr4_read_feature,
+
+ /* Support for the qXfer:btrace-conf:read packet. */
+ PACKET_qXfer_btrace_conf,
+
+ /* Support for the Qbtrace-conf:bts:size packet. */
+ PACKET_Qbtrace_conf_bts_size,
+
+ /* Support for swbreak+ feature. */
+ PACKET_swbreak_feature,
+
+ /* Support for hwbreak+ feature. */
+ PACKET_hwbreak_feature,
+
+ /* Support for fork events. */
+ PACKET_fork_event_feature,
+
+ /* Support for vfork events. */
+ PACKET_vfork_event_feature,
+
+ /* Support for the Qbtrace-conf:pt:size packet. */
+ PACKET_Qbtrace_conf_pt_size,
+
+ /* Support for exec events. */
+ PACKET_exec_event_feature,
+
+ /* Support for query supported vCont actions. */
+ PACKET_vContSupported,
+
+ /* Support remote CTRL-C. */
+ PACKET_vCtrlC,
+
+ /* Support TARGET_WAITKIND_NO_RESUMED. */
+ PACKET_no_resumed,
+
+ /* Support for memory tagging, allocation tag fetch/store
+ packets and the tag violation stop replies. */
+ PACKET_memory_tagging_feature,
+
+ PACKET_MAX
+};
+
struct threads_listing_context;
/* Stub vCont actions support.
@@ -395,6 +574,90 @@ static const target_info remote_target_info = {
remote_doc
};
+/* Description and configuration of a remote packet. */
+
+struct packet_config
+ {
+ /* Name of the packet used for gdb output. */
+ const char *name;
+
+ /* Title of the packet, used by the set/show remote name-packet
+ commands to identify the individual packages and gdb output. */
+ const char *title;
+
+ /* If auto, GDB auto-detects support for this packet or feature,
+ either through qSupported, or by trying the packet and looking
+ at the response. If true, GDB assumes the target supports this
+ packet. If false, the packet is disabled. Configs that don't
+ have an associated command always have this set to auto. */
+ enum auto_boolean detect;
+
+ /* The "show remote foo-packet" command created for this packet. */
+ cmd_list_element *show_cmd;
+
+ /* The "set remote foo-packet" command created for this packet. */
+ cmd_list_element *set_cmd;
+
+ /* Does the target support this packet? */
+ enum packet_support support;
+ };
+
+/* This global array contains the default configuration for every new
+ per-remote target array. */
+static packet_config remote_protocol_packets[PACKET_MAX];
+
+/* Description of a remote target's features. It stores the configuration
+ and provides functions to determine supported features of the target. */
+
+struct remote_features
+{
+ remote_features ()
+ {
+ std::copy (std::begin (remote_protocol_packets),
+ std::end (remote_protocol_packets),
+ std::begin (m_protocol_packets));
+ }
+ ~remote_features () = default;
+
+ DISABLE_COPY_AND_ASSIGN (remote_features);
+
+ /* Returns whether a given packet defined by its enum value is supported. */
+ enum packet_support packet_support (int) const;
+
+ /* Returns the packet's corresponding "set remote foo-packet" command
+ state. See struct packet_config for more details. */
+ enum auto_boolean packet_set_cmd_state (int packet) const
+ { return m_protocol_packets[packet].detect; }
+
+ /* Returns true if the multi-process extensions are in effect. */
+ int remote_multi_process_p () const
+ { return packet_support (PACKET_multiprocess_feature) == PACKET_ENABLE; }
+
+ /* Returns true if fork events are supported. */
+ int remote_fork_event_p () const
+ { return packet_support (PACKET_fork_event_feature) == PACKET_ENABLE; }
+
+ /* Returns true if vfork events are supported. */
+ int remote_vfork_event_p () const
+ { return packet_support (PACKET_vfork_event_feature) == PACKET_ENABLE; }
+
+ /* Returns true if exec events are supported. */
+ int remote_exec_event_p () const
+ { return packet_support (PACKET_exec_event_feature) == PACKET_ENABLE; }
+
+ /* Returns true if memory tagging is supported, false otherwise. */
+ bool remote_memory_tagging_p () const
+ { return packet_support (PACKET_memory_tagging_feature) == PACKET_ENABLE; }
+
+ /* Reset all packets back to "unknown support". Called when opening a
+ new connection to a remote target. */
+ void reset_all_packet_configs_support ();
+
+ /* The per-remote target array which stores a remote's packet
+ configurations. */
+ packet_config m_protocol_packets[PACKET_MAX];
+};
+
class remote_target : public process_stratum_target
{
public:
@@ -585,8 +848,16 @@ class remote_target : public process_stratum_target
bool supports_string_tracing () override;
+ int remote_supports_cond_tracepoints ();
+
bool supports_evaluation_of_breakpoint_conditions () override;
+ int remote_supports_fast_tracepoints ();
+
+ int remote_supports_static_tracepoints ();
+
+ int remote_supports_install_in_trace ();
+
bool can_run_breakpoint_commands () override;
void trace_init () override;
@@ -946,6 +1217,8 @@ class remote_target : public process_stratum_target
bool vcont_r_supported ();
+ remote_features m_features;
+
private:
bool start_remote_1 (int from_tty, int extended_p);
@@ -1061,7 +1334,8 @@ static CORE_ADDR remote_address_masked (CORE_ADDR);
static int stub_unpack_int (const char *buff, int fieldlength);
-struct packet_config;
+static void show_packet_config_cmd (ui_file *file, packet_config *config,
+ bool target_connected);
static void show_remote_protocol_packet_cmd (struct ui_file *file,
int from_tty,
@@ -1884,34 +2158,51 @@ remote_target::get_memory_read_packet_size ()
return size;
}
-\f
+static enum packet_support packet_config_support
+ (const packet_config *config);
-struct packet_config
- {
- const char *name;
- const char *title;
- /* If auto, GDB auto-detects support for this packet or feature,
- either through qSupported, or by trying the packet and looking
- at the response. If true, GDB assumes the target supports this
- packet. If false, the packet is disabled. Configs that don't
- have an associated command always have this set to auto. */
- enum auto_boolean detect;
+static void
+set_remote_protocol_packet_cmd (const char *args, int from_tty,
+ cmd_list_element *c)
+{
+ remote_target *remote = get_current_remote_target ();
+ gdb_assert (c->var.has_value ());
- /* The "show remote foo-packet" command created for this packet. */
- cmd_list_element *show_cmd;
+ auto *default_config = static_cast<packet_config *> (c->context ());
+ const int packet_idx = std::distance (remote_protocol_packets,
+ default_config);
- /* Does the target support this packet? */
- enum packet_support support;
- };
+ if (packet_idx >= 0 && packet_idx < PACKET_MAX)
+ {
+ const char *name = remote_protocol_packets[packet_idx].name;
+ const auto_boolean value = c->var->get<auto_boolean> ();
+ const char *support = get_packet_support_name (value);
+ if (remote != nullptr)
+ {
+ remote->m_features.m_protocol_packets[packet_idx].detect = value;
+ printf_filtered (_("Use of the '%s' packet for the current and "
+ "future remote targets is set to \"%s\".\n"),
+ name, support);
+ }
+ else
+ printf_filtered (_("Use of the '%s' packet for future remote "
+ "targets is set to \"%s\".\n"), name, support);
+
+ remote_protocol_packets[packet_idx].detect = value;
+ return;
+ }
-static enum packet_support packet_config_support (struct packet_config *config);
-static enum packet_support packet_support (int packet);
+ internal_error (__FILE__, __LINE__, _("Could not find config for %s"),
+ c->name);
+}
static void
-show_packet_config_cmd (ui_file *file, struct packet_config *config)
+show_packet_config_cmd (ui_file *file, packet_config *config,
+ bool target_connected)
{
const char *support = "internal-error";
+ const char *target_type = get_target_type_name (target_connected);
switch (packet_config_support (config))
{
@@ -1928,16 +2219,21 @@ show_packet_config_cmd (ui_file *file, struct packet_config *config)
switch (config->detect)
{
case AUTO_BOOLEAN_AUTO:
- fprintf_filtered (file,
- _("Support for the `%s' packet "
- "is auto-detected, currently %s.\n"),
- config->name, support);
+ if (target_connected)
+ fprintf_filtered (file,
+ _("Support for the '%s' packet %s is \"auto\" "
+ "(currently %s).\n"),
+ config->name, target_type, support);
+ else
+ fprintf_filtered (file,
+ _("Support for the '%s' packet %s is \"auto\".\n"),
+ config->name, target_type);
break;
case AUTO_BOOLEAN_TRUE:
case AUTO_BOOLEAN_FALSE:
fprintf_filtered (file,
- _("Support for the `%s' packet is currently %s.\n"),
- config->name, support);
+ _("Support for the '%s' packet %s is \"%s\".\n"),
+ config->name, target_type, support);
break;
}
}
@@ -1960,10 +2256,13 @@ add_packet_config_cmd (struct packet_config *config, const char *name,
= add_setshow_auto_boolean_cmd (cmd_name.release (), class_obscure,
&config->detect, set_doc.get (),
show_doc.get (), NULL, /* help_doc */
- NULL,
+ set_remote_protocol_packet_cmd,
show_remote_protocol_packet_cmd,
&remote_set_cmdlist, &remote_show_cmdlist);
config->show_cmd = cmds.show;
+ config->set_cmd = cmds.set;
+ cmds.show->set_context(config);
+ cmds.set->set_context(config);
/* set/show remote NAME-packet {auto,on,off} -- legacy. */
if (legacy)
@@ -2067,173 +2366,12 @@ packet_ok (const gdb::char_vector &buf, struct packet_config *config)
return packet_ok (buf.data (), config);
}
-enum {
- PACKET_vCont = 0,
- PACKET_X,
- PACKET_qSymbol,
- PACKET_P,
- PACKET_p,
- PACKET_Z0,
- PACKET_Z1,
- PACKET_Z2,
- PACKET_Z3,
- PACKET_Z4,
- PACKET_vFile_setfs,
- PACKET_vFile_open,
- PACKET_vFile_pread,
- PACKET_vFile_pwrite,
- PACKET_vFile_close,
- PACKET_vFile_unlink,
- PACKET_vFile_readlink,
- PACKET_vFile_fstat,
- PACKET_qXfer_auxv,
- PACKET_qXfer_features,
- PACKET_qXfer_exec_file,
- PACKET_qXfer_libraries,
- PACKET_qXfer_libraries_svr4,
- PACKET_qXfer_memory_map,
- PACKET_qXfer_osdata,
- PACKET_qXfer_threads,
- PACKET_qXfer_statictrace_read,
- PACKET_qXfer_traceframe_info,
- PACKET_qXfer_uib,
- PACKET_qGetTIBAddr,
- PACKET_qGetTLSAddr,
- PACKET_qSupported,
- PACKET_qTStatus,
- PACKET_QPassSignals,
- PACKET_QCatchSyscalls,
- PACKET_QProgramSignals,
- PACKET_QSetWorkingDir,
- PACKET_QStartupWithShell,
- PACKET_QEnvironmentHexEncoded,
- PACKET_QEnvironmentReset,
- PACKET_QEnvironmentUnset,
- PACKET_qCRC,
- PACKET_qSearch_memory,
- PACKET_vAttach,
- PACKET_vRun,
- PACKET_QStartNoAckMode,
- PACKET_vKill,
- PACKET_qXfer_siginfo_read,
- PACKET_qXfer_siginfo_write,
- PACKET_qAttached,
-
- /* Support for conditional tracepoints. */
- PACKET_ConditionalTracepoints,
-
- /* Support for target-side breakpoint conditions. */
- PACKET_ConditionalBreakpoints,
-
- /* Support for target-side breakpoint commands. */
- PACKET_BreakpointCommands,
-
- /* Support for fast tracepoints. */
- PACKET_FastTracepoints,
-
- /* Support for static tracepoints. */
- PACKET_StaticTracepoints,
-
- /* Support for installing tracepoints while a trace experiment is
- running. */
- PACKET_InstallInTrace,
-
- PACKET_bc,
- PACKET_bs,
- PACKET_TracepointSource,
- PACKET_QAllow,
- PACKET_qXfer_fdpic,
- PACKET_QDisableRandomization,
- PACKET_QAgent,
- PACKET_QTBuffer_size,
- PACKET_Qbtrace_off,
- PACKET_Qbtrace_bts,
- PACKET_Qbtrace_pt,
- PACKET_qXfer_btrace,
-
- /* Support for the QNonStop packet. */
- PACKET_QNonStop,
-
- /* Support for the QThreadEvents packet. */
- PACKET_QThreadEvents,
-
- /* Support for multi-process extensions. */
- PACKET_multiprocess_feature,
-
- /* Support for enabling and disabling tracepoints while a trace
- experiment is running. */
- PACKET_EnableDisableTracepoints_feature,
-
- /* Support for collecting strings using the tracenz bytecode. */
- PACKET_tracenz_feature,
-
- /* Support for continuing to run a trace experiment while GDB is
- disconnected. */
- PACKET_DisconnectedTracing_feature,
-
- /* Support for qXfer:libraries-svr4:read with a non-empty annex. */
- PACKET_augmented_libraries_svr4_read_feature,
-
- /* Support for the qXfer:btrace-conf:read packet. */
- PACKET_qXfer_btrace_conf,
-
- /* Support for the Qbtrace-conf:bts:size packet. */
- PACKET_Qbtrace_conf_bts_size,
-
- /* Support for swbreak+ feature. */
- PACKET_swbreak_feature,
-
- /* Support for hwbreak+ feature. */
- PACKET_hwbreak_feature,
-
- /* Support for fork events. */
- PACKET_fork_event_feature,
-
- /* Support for vfork events. */
- PACKET_vfork_event_feature,
-
- /* Support for the Qbtrace-conf:pt:size packet. */
- PACKET_Qbtrace_conf_pt_size,
-
- /* Support for exec events. */
- PACKET_exec_event_feature,
-
- /* Support for query supported vCont actions. */
- PACKET_vContSupported,
-
- /* Support remote CTRL-C. */
- PACKET_vCtrlC,
-
- /* Support TARGET_WAITKIND_NO_RESUMED. */
- PACKET_no_resumed,
-
- /* Support for memory tagging, allocation tag fetch/store
- packets and the tag violation stop replies. */
- PACKET_memory_tagging_feature,
-
- PACKET_MAX
-};
-
-/* FIXME: needs to be per-remote-target. Ignoring this for now,
- assuming all remote targets are the same server (thus all support
- the same packets). */
-static struct packet_config remote_protocol_packets[PACKET_MAX];
-
-/* Returns the packet's corresponding "set remote foo-packet" command
- state. See struct packet_config for more details. */
-
-static enum auto_boolean
-packet_set_cmd_state (int packet)
-{
- return remote_protocol_packets[packet].detect;
-}
-
/* Returns whether a given packet or feature is supported. This takes
into account the state of the corresponding "set remote foo-packet"
command, which may be used to bypass auto-detection. */
static enum packet_support
-packet_config_support (struct packet_config *config)
+packet_config_support (const packet_config *config)
{
switch (config->detect)
{
@@ -2248,13 +2386,10 @@ packet_config_support (struct packet_config *config)
}
}
-/* Same as packet_config_support, but takes the packet's enum value as
- argument. */
-
-static enum packet_support
-packet_support (int packet)
+enum packet_support
+remote_features::packet_support (int packet) const
{
- struct packet_config *config = &remote_protocol_packets[packet];
+ const packet_config *config = &m_protocol_packets[packet];
return packet_config_support (config);
}
@@ -2264,19 +2399,24 @@ show_remote_protocol_packet_cmd (struct ui_file *file, int from_tty,
struct cmd_list_element *c,
const char *value)
{
- struct packet_config *packet;
+ remote_target *remote = get_current_remote_target ();
gdb_assert (c->var.has_value ());
- for (packet = remote_protocol_packets;
- packet < &remote_protocol_packets[PACKET_MAX];
- packet++)
+ auto *default_config = static_cast<packet_config *> (c->context ());
+ const int packet_idx = std::distance (remote_protocol_packets,
+ default_config);
+
+ if (packet_idx >= 0 && packet_idx < PACKET_MAX)
{
- if (c == packet->show_cmd)
- {
- show_packet_config_cmd (file, packet);
- return;
- }
+ if (remote != nullptr)
+ show_packet_config_cmd
+ (file, &remote->m_features.m_protocol_packets[packet_idx], true);
+ else
+ show_packet_config_cmd
+ (file, &remote_protocol_packets[packet_idx], false);
+ return;
}
+
internal_error (__FILE__, __LINE__, _("Could not find config for %s"),
c->name);
}
@@ -2302,10 +2442,25 @@ static void
set_remote_protocol_Z_packet_cmd (const char *args, int from_tty,
struct cmd_list_element *c)
{
+ remote_target *remote = get_current_remote_target ();
int i;
for (i = 0; i < NR_Z_PACKET_TYPES; i++)
- remote_protocol_packets[PACKET_Z0 + i].detect = remote_Z_packet_detect;
+ {
+ if (remote != nullptr)
+ remote->m_features.m_protocol_packets[PACKET_Z0 + i].detect
+ = remote_Z_packet_detect;
+ remote_protocol_packets[PACKET_Z0 + i].detect = remote_Z_packet_detect;
+ }
+
+ const char *support = get_packet_support_name (remote_Z_packet_detect);
+
+ if (remote != nullptr)
+ printf_filtered (_("Use of Z packets for the current and future "
+ "remote targets is set to \"%s\".\n"), support);
+ else
+ printf_filtered (_("Use of Z packets for future remote targets is set to "
+ "\"%s\".\n"), support);
}
static void
@@ -2313,63 +2468,27 @@ show_remote_protocol_Z_packet_cmd (struct ui_file *file, int from_tty,
struct cmd_list_element *c,
const char *value)
{
+ remote_target *remote = get_current_remote_target ();
int i;
for (i = 0; i < NR_Z_PACKET_TYPES; i++)
{
- show_packet_config_cmd (file, &remote_protocol_packets[PACKET_Z0 + i]);
+ if (remote != nullptr)
+ show_packet_config_cmd
+ (file, &remote->m_features.m_protocol_packets[PACKET_Z0 + i], true);
+ else
+ show_packet_config_cmd
+ (file, &remote_protocol_packets[PACKET_Z0 + i], false);
}
}
-/* Returns true if the multi-process extensions are in effect. */
-
-static int
-remote_multi_process_p (struct remote_state *rs)
-{
- return packet_support (PACKET_multiprocess_feature) == PACKET_ENABLE;
-}
-
-/* Returns true if fork events are supported. */
-
-static int
-remote_fork_event_p (struct remote_state *rs)
-{
- return packet_support (PACKET_fork_event_feature) == PACKET_ENABLE;
-}
-
-/* Returns true if vfork events are supported. */
-
-static int
-remote_vfork_event_p (struct remote_state *rs)
-{
- return packet_support (PACKET_vfork_event_feature) == PACKET_ENABLE;
-}
-
-/* Returns true if exec events are supported. */
-
-static int
-remote_exec_event_p (struct remote_state *rs)
-{
- return packet_support (PACKET_exec_event_feature) == PACKET_ENABLE;
-}
-
-/* Returns true if memory tagging is supported, false otherwise. */
-
-static bool
-remote_memory_tagging_p ()
-{
- return packet_support (PACKET_memory_tagging_feature) == PACKET_ENABLE;
-}
-
/* Insert fork catchpoint target routine. If fork events are enabled
then return success, nothing more to do. */
int
remote_target::insert_fork_catchpoint (int pid)
{
- struct remote_state *rs = get_remote_state ();
-
- return !remote_fork_event_p (rs);
+ return !m_features.remote_fork_event_p ();
}
/* Remove fork catchpoint target routine. Nothing to do, just
@@ -2387,9 +2506,7 @@ remote_target::remove_fork_catchpoint (int pid)
int
remote_target::insert_vfork_catchpoint (int pid)
{
- struct remote_state *rs = get_remote_state ();
-
- return !remote_vfork_event_p (rs);
+ return !m_features.remote_vfork_event_p ();
}
/* Remove vfork catchpoint target routine. Nothing to do, just
@@ -2407,9 +2524,7 @@ remote_target::remove_vfork_catchpoint (int pid)
int
remote_target::insert_exec_catchpoint (int pid)
{
- struct remote_state *rs = get_remote_state ();
-
- return !remote_exec_event_p (rs);
+ return !m_features.remote_exec_event_p ();
}
/* Remove exec catchpoint target routine. Nothing to do, just
@@ -2438,10 +2553,10 @@ remote_target::remote_query_attached (int pid)
struct remote_state *rs = get_remote_state ();
size_t size = get_remote_packet_size ();
- if (packet_support (PACKET_qAttached) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_qAttached) == PACKET_DISABLE)
return 0;
- if (remote_multi_process_p (rs))
+ if (m_features.remote_multi_process_p ())
xsnprintf (rs->buf.data (), size, "qAttached:%x", pid);
else
xsnprintf (rs->buf.data (), size, "qAttached");
@@ -2450,7 +2565,7 @@ remote_target::remote_query_attached (int pid)
getpkt (&rs->buf, 0);
switch (packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_qAttached]))
+ &m_features.m_protocol_packets[PACKET_qAttached]))
{
case PACKET_OK:
if (strcmp (rs->buf.data (), "1") == 0)
@@ -2650,8 +2765,7 @@ remote_target::remote_notice_new_inferior (ptid_t currthread, bool executing)
thread, so notifications are emitted in a sensible order. */
if (find_inferior_pid (this, currthread.pid ()) == NULL)
{
- struct remote_state *rs = get_remote_state ();
- bool fake_pid_p = !remote_multi_process_p (rs);
+ bool fake_pid_p = !m_features.remote_multi_process_p ();
inf = remote_add_inferior (fake_pid_p,
currthread.pid (), -1, 1);
@@ -2714,7 +2828,7 @@ record_currthread (struct remote_state *rs, ptid_t currthread)
void
remote_target::pass_signals (gdb::array_view<const unsigned char> pass_signals)
{
- if (packet_support (PACKET_QPassSignals) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QPassSignals) != PACKET_DISABLE)
{
char *pass_packet, *p;
int count = 0;
@@ -2748,7 +2862,8 @@ remote_target::pass_signals (gdb::array_view<const unsigned char> pass_signals)
{
putpkt (pass_packet);
getpkt (&rs->buf, 0);
- packet_ok (rs->buf, &remote_protocol_packets[PACKET_QPassSignals]);
+ packet_ok (rs->buf,
+ &m_features.m_protocol_packets[PACKET_QPassSignals]);
xfree (rs->last_pass_packet);
rs->last_pass_packet = pass_packet;
}
@@ -2768,7 +2883,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
enum packet_result result;
int n_sysno = 0;
- if (packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QCatchSyscalls) == PACKET_DISABLE)
{
/* Not supported. */
return 1;
@@ -2821,7 +2936,8 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
putpkt (catch_packet);
getpkt (&rs->buf, 0);
- result = packet_ok (rs->buf, &remote_protocol_packets[PACKET_QCatchSyscalls]);
+ result = packet_ok (rs->buf,
+ &m_features.m_protocol_packets[PACKET_QCatchSyscalls]);
if (result == PACKET_OK)
return 0;
else
@@ -2834,7 +2950,7 @@ remote_target::set_syscall_catchpoint (int pid, bool needed, int any_count,
void
remote_target::program_signals (gdb::array_view<const unsigned char> signals)
{
- if (packet_support (PACKET_QProgramSignals) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QProgramSignals) != PACKET_DISABLE)
{
char *packet, *p;
int count = 0;
@@ -2869,7 +2985,8 @@ remote_target::program_signals (gdb::array_view<const unsigned char> signals)
{
putpkt (packet);
getpkt (&rs->buf, 0);
- packet_ok (rs->buf, &remote_protocol_packets[PACKET_QProgramSignals]);
+ packet_ok (rs->buf,
+ &m_features.m_protocol_packets[PACKET_QProgramSignals]);
xfree (rs->last_program_signals_packet);
rs->last_program_signals_packet = packet;
}
@@ -2935,12 +3052,12 @@ remote_target::set_continue_thread (ptid_t ptid)
void
remote_target::set_general_process ()
{
- struct remote_state *rs = get_remote_state ();
-
/* If the remote can't handle multiple processes, don't bother. */
- if (!remote_multi_process_p (rs))
+ if (!m_features.remote_multi_process_p ())
return;
+ remote_state *rs = get_remote_state ();
+
/* We only need to change the remote current thread if it's pointing
at some other process. */
if (rs->general_thread.pid () != inferior_ptid.pid ())
@@ -3090,9 +3207,8 @@ char *
remote_target::write_ptid (char *buf, const char *endbuf, ptid_t ptid)
{
int pid, tid;
- struct remote_state *rs = get_remote_state ();
- if (remote_multi_process_p (rs))
+ if (m_features.remote_multi_process_p ())
{
pid = ptid.pid ();
if (pid < 0)
@@ -3853,7 +3969,7 @@ int
remote_target::remote_get_threads_with_qxfer (threads_listing_context *context)
{
#if defined(HAVE_LIBEXPAT)
- if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
{
gdb::optional<gdb::char_vector> xml
= target_read_stralloc (this, TARGET_OBJECT_THREADS, NULL);
@@ -4046,7 +4162,7 @@ remote_target::extra_thread_info (thread_info *tp)
if (!extra.empty ())
return extra.c_str ();
- if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
{
/* If we're using qXfer:threads:read, then the extra info is
included in the XML. So if we didn't have anything cached,
@@ -4472,7 +4588,6 @@ remote_target::get_current_thread (const char *wait_status)
thread_info *
remote_target::add_current_inferior_and_thread (const char *wait_status)
{
- struct remote_state *rs = get_remote_state ();
bool fake_pid_p = false;
switch_to_no_thread ();
@@ -4483,7 +4598,7 @@ remote_target::add_current_inferior_and_thread (const char *wait_status)
if (curr_ptid != null_ptid)
{
- if (!remote_multi_process_p (rs))
+ if (!m_features.remote_multi_process_p ())
fake_pid_p = true;
}
else
@@ -4763,7 +4878,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
remote_query_supported ();
/* If the stub wants to get a QAllow, compose one and send it. */
- if (packet_support (PACKET_QAllow) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QAllow) != PACKET_DISABLE)
set_permissions ();
/* gdbserver < 7.7 (before its fix from 2013-12-11) did reply to any
@@ -4779,7 +4894,10 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
putpkt (v_mustreplyempty);
getpkt (&rs->buf, 0);
if (strcmp (rs->buf.data (), "OK") == 0)
- remote_protocol_packets[PACKET_vFile_setfs].support = PACKET_DISABLE;
+ {
+ m_features.m_protocol_packets[PACKET_vFile_setfs].support
+ = PACKET_DISABLE;
+ }
else if (strcmp (rs->buf.data (), "") != 0)
error (_("Remote replied unexpectedly to '%s': %s"), v_mustreplyempty,
rs->buf.data ());
@@ -4798,7 +4916,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
If FALSE, then don't activate noack mode, regardless of what the
stub claimed should be the default with qSupported. */
- noack_config = &remote_protocol_packets[PACKET_QStartNoAckMode];
+ noack_config = &m_features.m_protocol_packets[PACKET_QStartNoAckMode];
if (packet_config_support (noack_config) != PACKET_DISABLE)
{
putpkt ("QStartNoAckMode");
@@ -4833,7 +4951,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
if (target_is_non_stop_p ())
{
- if (packet_support (PACKET_QNonStop) != PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_QNonStop) != PACKET_ENABLE)
error (_("Non-stop mode requested, but remote "
"does not support non-stop"));
@@ -4850,7 +4968,7 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
stopped. */
this->update_thread_list ();
}
- else if (packet_support (PACKET_QNonStop) == PACKET_ENABLE)
+ else if (m_features.packet_support (PACKET_QNonStop) == PACKET_ENABLE)
{
/* Don't assume that the stub can operate in all-stop mode.
Request it explicitly. */
@@ -5079,16 +5197,13 @@ extended_remote_target::open (const char *name, int from_tty)
open_1 (name, from_tty, 1 /*extended_p */);
}
-/* Reset all packets back to "unknown support". Called when opening a
- new connection to a remote target. */
-
-static void
-reset_all_packet_configs_support (void)
+void
+remote_features::reset_all_packet_configs_support ()
{
int i;
for (i = 0; i < PACKET_MAX; i++)
- remote_protocol_packets[i].support = PACKET_SUPPORT_UNKNOWN;
+ m_protocol_packets[i].support = PACKET_SUPPORT_UNKNOWN;
}
/* Initialize all packet configs. */
@@ -5121,7 +5236,7 @@ remote_target::remote_check_symbols ()
if (!target_has_execution ())
return;
- if (packet_support (PACKET_qSymbol) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_qSymbol) == PACKET_DISABLE)
return;
/* Make sure the remote is pointing at the right process. Note
@@ -5137,7 +5252,7 @@ remote_target::remote_check_symbols ()
putpkt ("qSymbol::");
getpkt (&reply, 0);
- packet_ok (reply, &remote_protocol_packets[PACKET_qSymbol]);
+ packet_ok (reply, &m_features.m_protocol_packets[PACKET_qSymbol]);
while (startswith (reply.data (), "qSymbol:"))
{
@@ -5261,7 +5376,7 @@ remote_supported_packet (remote_target *remote,
return;
}
- remote_protocol_packets[feature->packet].support = support;
+ remote->m_features.m_protocol_packets[feature->packet].support = support;
}
void
@@ -5469,47 +5584,57 @@ remote_target::remote_query_supported ()
containing no features. */
rs->buf[0] = 0;
- if (packet_support (PACKET_qSupported) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_qSupported) != PACKET_DISABLE)
{
std::string q;
- if (packet_set_cmd_state (PACKET_multiprocess_feature) != AUTO_BOOLEAN_FALSE)
+ if (m_features.packet_set_cmd_state (PACKET_multiprocess_feature)
+ != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "multiprocess+");
- if (packet_set_cmd_state (PACKET_swbreak_feature) != AUTO_BOOLEAN_FALSE)
+ if (m_features.packet_set_cmd_state (PACKET_swbreak_feature)
+ != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "swbreak+");
- if (packet_set_cmd_state (PACKET_hwbreak_feature) != AUTO_BOOLEAN_FALSE)
+
+ if (m_features.packet_set_cmd_state (PACKET_hwbreak_feature)
+ != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "hwbreak+");
remote_query_supported_append (&q, "qRelocInsn+");
- if (packet_set_cmd_state (PACKET_fork_event_feature)
+ if (m_features.packet_set_cmd_state (PACKET_fork_event_feature)
!= AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "fork-events+");
- if (packet_set_cmd_state (PACKET_vfork_event_feature)
+
+ if (m_features.packet_set_cmd_state (PACKET_vfork_event_feature)
!= AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "vfork-events+");
- if (packet_set_cmd_state (PACKET_exec_event_feature)
+
+ if (m_features.packet_set_cmd_state (PACKET_exec_event_feature)
!= AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "exec-events+");
- if (packet_set_cmd_state (PACKET_vContSupported) != AUTO_BOOLEAN_FALSE)
+ if (m_features.packet_set_cmd_state (PACKET_vContSupported)
+ != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "vContSupported+");
- if (packet_set_cmd_state (PACKET_QThreadEvents) != AUTO_BOOLEAN_FALSE)
+ if (m_features.packet_set_cmd_state (PACKET_QThreadEvents)
+ != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "QThreadEvents+");
- if (packet_set_cmd_state (PACKET_no_resumed) != AUTO_BOOLEAN_FALSE)
+ if (m_features.packet_set_cmd_state (PACKET_no_resumed)
+ != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "no-resumed+");
- if (packet_set_cmd_state (PACKET_memory_tagging_feature)
+ if (m_features.packet_set_cmd_state (PACKET_memory_tagging_feature)
!= AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "memory-tagging+");
/* Keep this one last to work around a gdbserver <= 7.10 bug in
the qSupported:xmlRegisters=i386 handling. */
if (remote_support_xml != NULL
- && packet_support (PACKET_qXfer_features) != PACKET_DISABLE)
+ && (m_features.packet_support (PACKET_qXfer_features)
+ != PACKET_DISABLE))
remote_query_supported_append (&q, remote_support_xml);
q = "qSupported:" + q;
@@ -5519,7 +5644,7 @@ remote_target::remote_query_supported ()
/* If an error occured, warn, but do not return - just reset the
buffer to empty and go on to disable features. */
- if (packet_ok (rs->buf, &remote_protocol_packets[PACKET_qSupported])
+ if (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_qSupported])
== PACKET_ERROR)
{
warning (_("Remote failure reply: %s"), rs->buf.data ());
@@ -5797,7 +5922,7 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
/* Reset the target state; these things will be queried either by
remote_query_supported or as they are needed. */
- reset_all_packet_configs_support ();
+ remote->m_features.reset_all_packet_configs_support ();
rs->explicit_packet_size = 0;
rs->noack_mode = 0;
rs->extended = extended_p;
@@ -5910,7 +6035,7 @@ remote_target::remote_detach_pid (int pid)
GDBserver to select GDB's current process. */
set_general_process ();
- if (remote_multi_process_p (rs))
+ if (m_features.remote_multi_process_p ())
xsnprintf (rs->buf.data (), get_remote_packet_size (), "D;%x", pid);
else
strcpy (rs->buf.data (), "D");
@@ -6041,10 +6166,10 @@ remote_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
process_stratum_target::follow_fork (child_inf, child_ptid,
fork_kind, follow_child, detach_fork);
- struct remote_state *rs = get_remote_state ();
-
- if ((fork_kind == TARGET_WAITKIND_FORKED && remote_fork_event_p (rs))
- || (fork_kind == TARGET_WAITKIND_VFORKED && remote_vfork_event_p (rs)))
+ if ((fork_kind == TARGET_WAITKIND_FORKED
+ && m_features.remote_fork_event_p ())
+ || (fork_kind == TARGET_WAITKIND_VFORKED
+ && m_features.remote_vfork_event_p ()))
{
/* When following the parent and detaching the child, we detach
the child here. For the case of following the child and
@@ -6110,7 +6235,7 @@ extended_remote_target::attach (const char *args, int from_tty)
/* Remote PID can be freely equal to getpid, do not check it here the same
way as in other targets. */
- if (packet_support (PACKET_vAttach) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_vAttach) == PACKET_DISABLE)
error (_("This target does not support attaching to a process"));
target_announce_attach (from_tty, pid);
@@ -6120,7 +6245,7 @@ extended_remote_target::attach (const char *args, int from_tty)
getpkt (&rs->buf, 0);
switch (packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_vAttach]))
+ &m_features.m_protocol_packets[PACKET_vAttach]))
{
case PACKET_OK:
if (!target_is_non_stop_p ())
@@ -6261,7 +6386,7 @@ remote_target::remote_vcont_probe ()
buf[0] = 0;
}
- packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCont]);
+ packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vCont]);
rs->supports_vCont_probed = true;
}
@@ -6292,7 +6417,7 @@ remote_target::append_resumption (char *p, char *endp,
threads with a wildcard (though the protocol allows it,
so stubs shouldn't make an active effort to forbid
it). */
- && !(remote_multi_process_p (rs) && ptid.is_pid ()))
+ && !(m_features.remote_multi_process_p () && ptid.is_pid ()))
{
struct thread_info *tp;
@@ -6326,7 +6451,7 @@ remote_target::append_resumption (char *p, char *endp,
else
p += xsnprintf (p, endp - p, ";c");
- if (remote_multi_process_p (rs) && ptid.is_pid ())
+ if (m_features.remote_multi_process_p () && ptid.is_pid ())
{
ptid_t nptid;
@@ -6410,9 +6535,9 @@ remote_target::remote_resume_with_hc (ptid_t ptid, int step,
warning (_(" - Can't pass signal %d to target in reverse: ignored."),
siggnal);
- if (step && packet_support (PACKET_bs) == PACKET_DISABLE)
+ if (step && m_features.packet_support (PACKET_bs) == PACKET_DISABLE)
error (_("Remote reverse-step not supported."));
- if (!step && packet_support (PACKET_bc) == PACKET_DISABLE)
+ if (!step && m_features.packet_support (PACKET_bc) == PACKET_DISABLE)
error (_("Remote reverse-continue not supported."));
strcpy (buf, step ? "bs" : "bc");
@@ -6452,10 +6577,10 @@ remote_target::remote_resume_with_vcont (ptid_t ptid, int step,
if (::execution_direction == EXEC_REVERSE)
return 0;
- if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
+ if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
remote_vcont_probe ();
- if (packet_support (PACKET_vCont) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_vCont) == PACKET_DISABLE)
return 0;
p = rs->buf.data ();
@@ -6988,7 +7113,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
/* FIXME: This supports_vCont_probed check is a workaround until
packet_support is per-connection. */
- if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN
+ if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN
|| !rs->supports_vCont_probed)
remote_vcont_probe ();
@@ -6996,7 +7121,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
error (_("Remote server does not support stopping threads"));
if (ptid == minus_one_ptid
- || (!remote_multi_process_p (rs) && ptid.is_pid ()))
+ || (!m_features.remote_multi_process_p () && ptid.is_pid ()))
p += xsnprintf (p, endp - p, "vCont;t");
else
{
@@ -7070,7 +7195,7 @@ remote_target::remote_interrupt_ns ()
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCtrlC]))
+ switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vCtrlC]))
{
case PACKET_OK:
break;
@@ -7577,7 +7702,8 @@ Packet: '%s'\n"),
/* Make sure the stub doesn't forget to indicate support
with qSupported. */
- if (packet_support (PACKET_swbreak_feature) != PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_swbreak_feature)
+ != PACKET_ENABLE)
error (_("Unexpected swbreak stop reason"));
/* The value part is documented as "must be empty",
@@ -7591,7 +7717,8 @@ Packet: '%s'\n"),
/* Make sure the stub doesn't forget to indicate support
with qSupported. */
- if (packet_support (PACKET_hwbreak_feature) != PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_hwbreak_feature)
+ != PACKET_ENABLE)
error (_("Unexpected hwbreak stop reason"));
/* See above. */
@@ -8362,7 +8489,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, reg->regnum));
int i;
- if (packet_support (PACKET_p) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_p) == PACKET_DISABLE)
return 0;
if (reg->pnum == -1)
@@ -8377,7 +8504,7 @@ remote_target::fetch_register_using_p (struct regcache *regcache,
buf = rs->buf.data ();
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_p]))
+ switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_p]))
{
case PACKET_OK:
break;
@@ -8644,7 +8771,7 @@ remote_target::prepare_to_store (struct regcache *regcache)
int i;
/* Make sure the entire registers array is valid. */
- switch (packet_support (PACKET_P))
+ switch (m_features.packet_support (PACKET_P))
{
case PACKET_DISABLE:
case PACKET_SUPPORT_UNKNOWN:
@@ -8672,7 +8799,7 @@ remote_target::store_register_using_P (const struct regcache *regcache,
gdb_byte *regp = (gdb_byte *) alloca (register_size (gdbarch, reg->regnum));
char *p;
- if (packet_support (PACKET_P) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_P) == PACKET_DISABLE)
return 0;
if (reg->pnum == -1)
@@ -8685,7 +8812,7 @@ remote_target::store_register_using_P (const struct regcache *regcache,
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_P]))
+ switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_P]))
{
case PACKET_OK:
return 1;
@@ -8868,7 +8995,7 @@ remote_target::check_binary_download (CORE_ADDR addr)
{
struct remote_state *rs = get_remote_state ();
- switch (packet_support (PACKET_X))
+ switch (m_features.packet_support (PACKET_X))
{
case PACKET_DISABLE:
break;
@@ -8892,12 +9019,12 @@ remote_target::check_binary_download (CORE_ADDR addr)
if (rs->buf[0] == '\0')
{
remote_debug_printf ("binary downloading NOT supported by target");
- remote_protocol_packets[PACKET_X].support = PACKET_DISABLE;
+ m_features.m_protocol_packets[PACKET_X].support = PACKET_DISABLE;
}
else
{
remote_debug_printf ("binary downloading supported by target");
- remote_protocol_packets[PACKET_X].support = PACKET_ENABLE;
+ m_features.m_protocol_packets[PACKET_X].support = PACKET_ENABLE;
}
break;
}
@@ -9124,7 +9251,7 @@ remote_target::remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr,
/* Check whether the target supports binary download. */
check_binary_download (memaddr);
- switch (packet_support (PACKET_X))
+ switch (m_features.packet_support (PACKET_X))
{
case PACKET_ENABLE:
packet_format = "X";
@@ -10089,11 +10216,10 @@ remote_target::kill ()
{
int res = -1;
inferior *inf = find_inferior_pid (this, inferior_ptid.pid ());
- struct remote_state *rs = get_remote_state ();
gdb_assert (inf != nullptr);
- if (packet_support (PACKET_vKill) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_vKill) != PACKET_DISABLE)
{
/* If we're stopped while forking and we haven't followed yet,
kill the child task. We need to do this before killing the
@@ -10112,7 +10238,7 @@ remote_target::kill ()
/* If we are in 'target remote' mode and we are killing the only
inferior, then we will tell gdbserver to exit and unpush the
target. */
- if (res == -1 && !remote_multi_process_p (rs)
+ if (res == -1 && !m_features.remote_multi_process_p ()
&& number_of_live_inferiors (this) == 1)
{
remote_kill_k ();
@@ -10134,7 +10260,7 @@ remote_target::kill ()
int
remote_target::remote_vkill (int pid)
{
- if (packet_support (PACKET_vKill) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_vKill) == PACKET_DISABLE)
return -1;
remote_state *rs = get_remote_state ();
@@ -10145,7 +10271,7 @@ remote_target::remote_vkill (int pid)
getpkt (&rs->buf, 0);
switch (packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_vKill]))
+ &m_features.m_protocol_packets[PACKET_vKill]))
{
case PACKET_OK:
return 0;
@@ -10241,7 +10367,8 @@ remote_target::mourn_inferior ()
bool
extended_remote_target::supports_disable_randomization ()
{
- return packet_support (PACKET_QDisableRandomization) == PACKET_ENABLE;
+ return (m_features.packet_support (PACKET_QDisableRandomization)
+ == PACKET_ENABLE);
}
void
@@ -10269,7 +10396,7 @@ remote_target::extended_remote_run (const std::string &args)
/* If the user has disabled vRun support, or we have detected that
support is not available, do not try it. */
- if (packet_support (PACKET_vRun) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_vRun) == PACKET_DISABLE)
return -1;
strcpy (rs->buf.data (), "vRun;");
@@ -10300,7 +10427,7 @@ remote_target::extended_remote_run (const std::string &args)
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vRun]))
+ switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vRun]))
{
case PACKET_OK:
/* We have a wait response. All is well. */
@@ -10353,7 +10480,7 @@ remote_target::extended_remote_environment_support ()
{
remote_state *rs = get_remote_state ();
- if (packet_support (PACKET_QEnvironmentReset) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QEnvironmentReset) != PACKET_DISABLE)
{
putpkt ("QEnvironmentReset");
getpkt (&rs->buf, 0);
@@ -10363,12 +10490,16 @@ remote_target::extended_remote_environment_support ()
gdb_environ *e = ¤t_inferior ()->environment;
- if (packet_support (PACKET_QEnvironmentHexEncoded) != PACKET_DISABLE)
- for (const std::string &el : e->user_set_env ())
- send_environment_packet ("set", "QEnvironmentHexEncoded",
- el.c_str ());
+ if (m_features.packet_support (PACKET_QEnvironmentHexEncoded)
+ != PACKET_DISABLE)
+ {
+ for (const std::string &el : e->user_set_env ())
+ send_environment_packet ("set", "QEnvironmentHexEncoded",
+ el.c_str ());
+ }
+
- if (packet_support (PACKET_QEnvironmentUnset) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QEnvironmentUnset) != PACKET_DISABLE)
for (const std::string &el : e->user_unset_env ())
send_environment_packet ("unset", "QEnvironmentUnset", el.c_str ());
}
@@ -10379,7 +10510,7 @@ remote_target::extended_remote_environment_support ()
void
remote_target::extended_remote_set_inferior_cwd ()
{
- if (packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE)
{
const std::string &inferior_cwd = current_inferior ()->cwd ();
remote_state *rs = get_remote_state ();
@@ -10404,7 +10535,7 @@ remote_target::extended_remote_set_inferior_cwd ()
putpkt (rs->buf);
getpkt (&rs->buf, 0);
if (packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_QSetWorkingDir])
+ &m_features.m_protocol_packets[PACKET_QSetWorkingDir])
!= PACKET_OK)
error (_("\
Remote replied unexpectedly while setting the inferior's working\n\
@@ -10441,7 +10572,7 @@ extended_remote_target::create_inferior (const char *exec_file,
/* If startup-with-shell is on, we inform gdbserver to start the
remote inferior using a shell. */
- if (packet_support (PACKET_QStartupWithShell) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QStartupWithShell) != PACKET_DISABLE)
{
xsnprintf (rs->buf.data (), get_remote_packet_size (),
"QStartupWithShell:%d", startup_with_shell ? 1 : 0);
@@ -10547,7 +10678,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
fails, and the user has explicitly requested the Z support then
report an error, otherwise, mark it disabled and go on. */
- if (packet_support (PACKET_Z0) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_Z0) != PACKET_DISABLE)
{
CORE_ADDR addr = bp_tgt->reqstd_address;
struct remote_state *rs;
@@ -10578,7 +10709,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0]))
+ switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z0]))
{
case PACKET_ERROR:
return -1;
@@ -10606,7 +10737,7 @@ remote_target::remove_breakpoint (struct gdbarch *gdbarch,
CORE_ADDR addr = bp_tgt->placed_address;
struct remote_state *rs = get_remote_state ();
- if (packet_support (PACKET_Z0) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_Z0) != PACKET_DISABLE)
{
char *p = rs->buf.data ();
char *endbuf = p + get_remote_packet_size ();
@@ -10662,7 +10793,7 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
char *p;
enum Z_packet_type packet = watchpoint_to_Z_packet (type);
- if (packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
return 1;
/* Make sure the remote is pointing at the right process, if
@@ -10679,7 +10810,8 @@ remote_target::insert_watchpoint (CORE_ADDR addr, int len,
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0 + packet]))
+ switch (packet_ok (rs->buf,
+ &m_features.m_protocol_packets[PACKET_Z0 + packet]))
{
case PACKET_ERROR:
return -1;
@@ -10711,7 +10843,7 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
char *p;
enum Z_packet_type packet = watchpoint_to_Z_packet (type);
- if (packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_Z0 + packet) == PACKET_DISABLE)
return -1;
/* Make sure the remote is pointing at the right process, if
@@ -10727,7 +10859,8 @@ remote_target::remove_watchpoint (CORE_ADDR addr, int len,
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z0 + packet]))
+ switch (packet_ok (rs->buf,
+ &m_features.m_protocol_packets[PACKET_Z0 + packet]))
{
case PACKET_ERROR:
case PACKET_UNKNOWN:
@@ -10801,7 +10934,7 @@ remote_target::stopped_by_sw_breakpoint ()
bool
remote_target::supports_stopped_by_sw_breakpoint ()
{
- return (packet_support (PACKET_swbreak_feature) == PACKET_ENABLE);
+ return (m_features.packet_support (PACKET_swbreak_feature) == PACKET_ENABLE);
}
/* The to_stopped_by_hw_breakpoint method of target remote. */
@@ -10822,7 +10955,7 @@ remote_target::stopped_by_hw_breakpoint ()
bool
remote_target::supports_stopped_by_hw_breakpoint ()
{
- return (packet_support (PACKET_hwbreak_feature) == PACKET_ENABLE);
+ return (m_features.packet_support (PACKET_hwbreak_feature) == PACKET_ENABLE);
}
bool
@@ -10861,7 +10994,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
char *p, *endbuf;
char *message;
- if (packet_support (PACKET_Z1) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_Z1) == PACKET_DISABLE)
return -1;
/* Make sure the remote is pointing at the right process, if
@@ -10890,7 +11023,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z1]))
+ switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z1]))
{
case PACKET_ERROR:
if (rs->buf[1] == '.')
@@ -10919,7 +11052,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
char *p = rs->buf.data ();
char *endbuf = p + get_remote_packet_size ();
- if (packet_support (PACKET_Z1) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_Z1) == PACKET_DISABLE)
return -1;
/* Make sure the remote is pointing at the right process, if
@@ -10938,7 +11071,7 @@ remote_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
putpkt (rs->buf);
getpkt (&rs->buf, 0);
- switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_Z1]))
+ switch (packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_Z1]))
{
case PACKET_ERROR:
case PACKET_UNKNOWN:
@@ -10962,7 +11095,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
/* It doesn't make sense to use qCRC if the remote target is
connected but not running. */
if (target_has_execution ()
- && packet_support (PACKET_qCRC) != PACKET_DISABLE)
+ && m_features.packet_support (PACKET_qCRC) != PACKET_DISABLE)
{
enum packet_result result;
@@ -10981,7 +11114,7 @@ remote_target::verify_memory (const gdb_byte *data, CORE_ADDR lma, ULONGEST size
getpkt (&rs->buf, 0);
result = packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_qCRC]);
+ &m_features.m_protocol_packets[PACKET_qCRC]);
if (result == PACKET_ERROR)
return -1;
else if (result == PACKET_OK)
@@ -11241,12 +11374,12 @@ remote_target::xfer_partial (enum target_object object,
{
if (readbuf)
return remote_read_qxfer ("siginfo", annex, readbuf, offset, len,
- xfered_len, &remote_protocol_packets
+ xfered_len, &m_features.m_protocol_packets
[PACKET_qXfer_siginfo_read]);
else
return remote_write_qxfer ("siginfo", annex,
writebuf, offset, len, xfered_len,
- &remote_protocol_packets
+ &m_features.m_protocol_packets
[PACKET_qXfer_siginfo_write]);
}
@@ -11255,7 +11388,7 @@ remote_target::xfer_partial (enum target_object object,
if (readbuf)
return remote_read_qxfer ("statictrace", annex,
readbuf, offset, len, xfered_len,
- &remote_protocol_packets
+ &m_features.m_protocol_packets
[PACKET_qXfer_statictrace_read]);
else
return TARGET_XFER_E_IO;
@@ -11285,74 +11418,74 @@ remote_target::xfer_partial (enum target_object object,
case TARGET_OBJECT_AUXV:
gdb_assert (annex == NULL);
- return remote_read_qxfer ("auxv", annex, readbuf, offset, len,
- xfered_len,
- &remote_protocol_packets[PACKET_qXfer_auxv]);
+ return remote_read_qxfer
+ ("auxv", annex, readbuf, offset, len, xfered_len,
+ &m_features.m_protocol_packets[PACKET_qXfer_auxv]);
case TARGET_OBJECT_AVAILABLE_FEATURES:
return remote_read_qxfer
("features", annex, readbuf, offset, len, xfered_len,
- &remote_protocol_packets[PACKET_qXfer_features]);
+ &m_features.m_protocol_packets[PACKET_qXfer_features]);
case TARGET_OBJECT_LIBRARIES:
return remote_read_qxfer
("libraries", annex, readbuf, offset, len, xfered_len,
- &remote_protocol_packets[PACKET_qXfer_libraries]);
+ &m_features.m_protocol_packets[PACKET_qXfer_libraries]);
case TARGET_OBJECT_LIBRARIES_SVR4:
return remote_read_qxfer
("libraries-svr4", annex, readbuf, offset, len, xfered_len,
- &remote_protocol_packets[PACKET_qXfer_libraries_svr4]);
+ &m_features.m_protocol_packets[PACKET_qXfer_libraries_svr4]);
case TARGET_OBJECT_MEMORY_MAP:
gdb_assert (annex == NULL);
- return remote_read_qxfer ("memory-map", annex, readbuf, offset, len,
- xfered_len,
- &remote_protocol_packets[PACKET_qXfer_memory_map]);
+ return remote_read_qxfer
+ ("memory-map", annex, readbuf, offset, len, xfered_len,
+ &m_features.m_protocol_packets[PACKET_qXfer_memory_map]);
case TARGET_OBJECT_OSDATA:
/* Should only get here if we're connected. */
gdb_assert (rs->remote_desc);
return remote_read_qxfer
("osdata", annex, readbuf, offset, len, xfered_len,
- &remote_protocol_packets[PACKET_qXfer_osdata]);
+ &m_features.m_protocol_packets[PACKET_qXfer_osdata]);
case TARGET_OBJECT_THREADS:
gdb_assert (annex == NULL);
- return remote_read_qxfer ("threads", annex, readbuf, offset, len,
- xfered_len,
- &remote_protocol_packets[PACKET_qXfer_threads]);
+ return remote_read_qxfer
+ ("threads", annex, readbuf, offset, len, xfered_len,
+ &m_features.m_protocol_packets[PACKET_qXfer_threads]);
case TARGET_OBJECT_TRACEFRAME_INFO:
gdb_assert (annex == NULL);
return remote_read_qxfer
("traceframe-info", annex, readbuf, offset, len, xfered_len,
- &remote_protocol_packets[PACKET_qXfer_traceframe_info]);
+ &m_features.m_protocol_packets[PACKET_qXfer_traceframe_info]);
case TARGET_OBJECT_FDPIC:
- return remote_read_qxfer ("fdpic", annex, readbuf, offset, len,
- xfered_len,
- &remote_protocol_packets[PACKET_qXfer_fdpic]);
+ return remote_read_qxfer
+ ("fdpic", annex, readbuf, offset, len, xfered_len,
+ &m_features.m_protocol_packets[PACKET_qXfer_fdpic]);
case TARGET_OBJECT_OPENVMS_UIB:
- return remote_read_qxfer ("uib", annex, readbuf, offset, len,
- xfered_len,
- &remote_protocol_packets[PACKET_qXfer_uib]);
+ return remote_read_qxfer
+ ("uib", annex, readbuf, offset, len, xfered_len,
+ &m_features.m_protocol_packets[PACKET_qXfer_uib]);
case TARGET_OBJECT_BTRACE:
- return remote_read_qxfer ("btrace", annex, readbuf, offset, len,
- xfered_len,
- &remote_protocol_packets[PACKET_qXfer_btrace]);
+ return remote_read_qxfer
+ ("btrace", annex, readbuf, offset, len, xfered_len,
+ &m_features.m_protocol_packets[PACKET_qXfer_btrace]);
case TARGET_OBJECT_BTRACE_CONF:
- return remote_read_qxfer ("btrace-conf", annex, readbuf, offset,
- len, xfered_len,
- &remote_protocol_packets[PACKET_qXfer_btrace_conf]);
+ return remote_read_qxfer
+ ("btrace-conf", annex, readbuf, offset, len, xfered_len,
+ &m_features.m_protocol_packets[PACKET_qXfer_btrace_conf]);
case TARGET_OBJECT_EXEC_FILE:
- return remote_read_qxfer ("exec-file", annex, readbuf, offset,
- len, xfered_len,
- &remote_protocol_packets[PACKET_qXfer_exec_file]);
+ return remote_read_qxfer
+ ("exec-file", annex, readbuf, offset, len, xfered_len,
+ &m_features.m_protocol_packets[PACKET_qXfer_exec_file]);
default:
return TARGET_XFER_E_IO;
@@ -11419,7 +11552,7 @@ remote_target::search_memory (CORE_ADDR start_addr, ULONGEST search_space_len,
struct remote_state *rs = get_remote_state ();
int max_size = get_memory_write_packet_size ();
struct packet_config *packet =
- &remote_protocol_packets[PACKET_qSearch_memory];
+ &m_features.m_protocol_packets[PACKET_qSearch_memory];
/* Number of packet bytes used to encode the pattern;
this could be more than PATTERN_LEN due to escape characters. */
int escaped_pattern_len;
@@ -11827,8 +11960,6 @@ init_remote_threadtests (void)
std::string
remote_target::pid_to_str (ptid_t ptid)
{
- struct remote_state *rs = get_remote_state ();
-
if (ptid == null_ptid)
return normal_pid_to_str (ptid);
else if (ptid.is_pid ())
@@ -11844,7 +11975,7 @@ remote_target::pid_to_str (ptid_t ptid)
connecting with extended-remote and the stub already being
attached to a process, and reporting yes to qAttached, hence
no smart special casing here. */
- if (!remote_multi_process_p (rs))
+ if (!m_features.remote_multi_process_p ())
return "Remote target";
return normal_pid_to_str (ptid);
@@ -11853,7 +11984,7 @@ remote_target::pid_to_str (ptid_t ptid)
{
if (magic_null_ptid == ptid)
return "Thread <main>";
- else if (remote_multi_process_p (rs))
+ else if (m_features.remote_multi_process_p ())
if (ptid.lwp () == 0)
return normal_pid_to_str (ptid);
else
@@ -11871,7 +12002,7 @@ CORE_ADDR
remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
CORE_ADDR offset)
{
- if (packet_support (PACKET_qGetTLSAddr) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_qGetTLSAddr) != PACKET_DISABLE)
{
struct remote_state *rs = get_remote_state ();
char *p = rs->buf.data ();
@@ -11890,7 +12021,7 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
putpkt (rs->buf);
getpkt (&rs->buf, 0);
result = packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_qGetTLSAddr]);
+ &m_features.m_protocol_packets[PACKET_qGetTLSAddr]);
if (result == PACKET_OK)
{
ULONGEST addr;
@@ -11918,7 +12049,7 @@ remote_target::get_thread_local_address (ptid_t ptid, CORE_ADDR lm,
bool
remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
{
- if (packet_support (PACKET_qGetTIBAddr) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_qGetTIBAddr) != PACKET_DISABLE)
{
struct remote_state *rs = get_remote_state ();
char *p = rs->buf.data ();
@@ -11933,7 +12064,7 @@ remote_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
putpkt (rs->buf);
getpkt (&rs->buf, 0);
result = packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_qGetTIBAddr]);
+ &m_features.m_protocol_packets[PACKET_qGetTIBAddr]);
if (result == PACKET_OK)
{
ULONGEST val;
@@ -12186,7 +12317,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
int ret, bytes_read;
const char *attachment_tmp;
- if (packet_support (which_packet) == PACKET_DISABLE)
+ if (m_features.packet_support (which_packet) == PACKET_DISABLE)
{
*remote_errno = FILEIO_ENOSYS;
return -1;
@@ -12203,7 +12334,7 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
return -1;
}
- switch (packet_ok (rs->buf, &remote_protocol_packets[which_packet]))
+ switch (packet_ok (rs->buf, &m_features.m_protocol_packets[which_packet]))
{
case PACKET_ERROR:
*remote_errno = FILEIO_EINVAL;
@@ -12273,7 +12404,7 @@ remote_target::remote_hostio_set_filesystem (struct inferior *inf,
char arg[9];
int ret;
- if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
return 0;
if (rs->fs_pid != -1 && required_pid == rs->fs_pid)
@@ -12287,7 +12418,7 @@ remote_target::remote_hostio_set_filesystem (struct inferior *inf,
ret = remote_hostio_send_command (p - rs->buf.data (), PACKET_vFile_setfs,
remote_errno, NULL, NULL);
- if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
return 0;
if (ret == 0)
@@ -12659,7 +12790,7 @@ remote_target::filesystem_is_local ()
does not support vFile:open. */
if (gdb_sysroot == TARGET_SYSROOT_PREFIX)
{
- enum packet_support ps = packet_support (PACKET_vFile_open);
+ enum packet_support ps = m_features.packet_support (PACKET_vFile_open);
if (ps == PACKET_SUPPORT_UNKNOWN)
{
@@ -12675,7 +12806,7 @@ remote_target::filesystem_is_local ()
if (fd >= 0)
remote_hostio_close (fd, &remote_errno);
- ps = packet_support (PACKET_vFile_open);
+ ps = m_features.packet_support (PACKET_vFile_open);
}
if (ps == PACKET_DISABLE)
@@ -13028,8 +13159,8 @@ remote_delete_command (const char *args, int from_tty)
bool
remote_target::can_execute_reverse ()
{
- if (packet_support (PACKET_bs) == PACKET_ENABLE
- || packet_support (PACKET_bc) == PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_bs) == PACKET_ENABLE
+ || m_features.packet_support (PACKET_bc) == PACKET_ENABLE)
return true;
else
return false;
@@ -13051,58 +13182,58 @@ remote_target::supports_disable_randomization ()
bool
remote_target::supports_multi_process ()
{
- struct remote_state *rs = get_remote_state ();
-
- return remote_multi_process_p (rs);
+ return m_features.remote_multi_process_p ();
}
-static int
-remote_supports_cond_tracepoints ()
+int
+remote_target::remote_supports_cond_tracepoints ()
{
- return packet_support (PACKET_ConditionalTracepoints) == PACKET_ENABLE;
+ return (m_features.packet_support (PACKET_ConditionalTracepoints)
+ == PACKET_ENABLE);
}
bool
remote_target::supports_evaluation_of_breakpoint_conditions ()
{
- return packet_support (PACKET_ConditionalBreakpoints) == PACKET_ENABLE;
+ return (m_features.packet_support (PACKET_ConditionalBreakpoints)
+ == PACKET_ENABLE);
}
-static int
-remote_supports_fast_tracepoints ()
+int
+remote_target::remote_supports_fast_tracepoints ()
{
- return packet_support (PACKET_FastTracepoints) == PACKET_ENABLE;
+ return m_features.packet_support (PACKET_FastTracepoints) == PACKET_ENABLE;
}
-static int
-remote_supports_static_tracepoints ()
+int
+remote_target::remote_supports_static_tracepoints ()
{
- return packet_support (PACKET_StaticTracepoints) == PACKET_ENABLE;
+ return m_features.packet_support (PACKET_StaticTracepoints) == PACKET_ENABLE;
}
-static int
-remote_supports_install_in_trace ()
+int
+remote_target::remote_supports_install_in_trace ()
{
- return packet_support (PACKET_InstallInTrace) == PACKET_ENABLE;
+ return m_features.packet_support (PACKET_InstallInTrace) == PACKET_ENABLE;
}
bool
remote_target::supports_enable_disable_tracepoint ()
{
- return (packet_support (PACKET_EnableDisableTracepoints_feature)
+ return (m_features.packet_support (PACKET_EnableDisableTracepoints_feature)
== PACKET_ENABLE);
}
bool
remote_target::supports_string_tracing ()
{
- return packet_support (PACKET_tracenz_feature) == PACKET_ENABLE;
+ return m_features.packet_support (PACKET_tracenz_feature) == PACKET_ENABLE;
}
bool
remote_target::can_run_breakpoint_commands ()
{
- return packet_support (PACKET_BreakpointCommands) == PACKET_ENABLE;
+ return m_features.packet_support (PACKET_BreakpointCommands) == PACKET_ENABLE;
}
void
@@ -13347,7 +13478,7 @@ remote_target::download_tracepoint (struct bp_location *loc)
error (_("Error on target while setting tracepoints."));
}
- if (packet_support (PACKET_TracepointSource) == PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_TracepointSource) == PACKET_ENABLE)
{
if (b->location != NULL)
{
@@ -13503,7 +13634,8 @@ remote_target::trace_set_readonly_regions ()
sec_length = 1 + strlen (tmp1) + 1 + strlen (tmp2);
if (offset + sec_length + 1 > rs->buf.size ())
{
- if (packet_support (PACKET_qXfer_traceframe_info) != PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_qXfer_traceframe_info)
+ != PACKET_ENABLE)
warning (_("\
Too many sections for read-only sections definition packet."));
break;
@@ -13540,7 +13672,7 @@ remote_target::get_trace_status (struct trace_status *ts)
enum packet_result result;
struct remote_state *rs = get_remote_state ();
- if (packet_support (PACKET_qTStatus) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_qTStatus) == PACKET_DISABLE)
return -1;
/* FIXME we need to get register block size some other way. */
@@ -13563,7 +13695,7 @@ remote_target::get_trace_status (struct trace_status *ts)
throw;
}
- result = packet_ok (p, &remote_protocol_packets[PACKET_qTStatus]);
+ result = packet_ok (p, &m_features.m_protocol_packets[PACKET_qTStatus]);
/* If the remote target doesn't do tracing, flag it. */
if (result == PACKET_UNKNOWN)
@@ -13818,7 +13950,8 @@ remote_target::set_disconnected_tracing (int val)
{
struct remote_state *rs = get_remote_state ();
- if (packet_support (PACKET_DisconnectedTracing_feature) == PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_DisconnectedTracing_feature)
+ == PACKET_ENABLE)
{
char *reply;
@@ -13912,7 +14045,7 @@ remote_target::get_min_fast_tracepoint_insn_len ()
void
remote_target::set_trace_buffer_size (LONGEST val)
{
- if (packet_support (PACKET_QTBuffer_size) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QTBuffer_size) != PACKET_DISABLE)
{
struct remote_state *rs = get_remote_state ();
char *buf = rs->buf.data ();
@@ -13933,7 +14066,7 @@ remote_target::set_trace_buffer_size (LONGEST val)
putpkt (rs->buf);
remote_get_noisy_reply ();
result = packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_QTBuffer_size]);
+ &m_features.m_protocol_packets[PACKET_QTBuffer_size]);
if (result != PACKET_OK)
warning (_("Bogus reply from target: %s"), rs->buf.data ());
@@ -13989,7 +14122,7 @@ remote_target::set_trace_notes (const char *user, const char *notes,
bool
remote_target::use_agent (bool use)
{
- if (packet_support (PACKET_QAgent) != PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QAgent) != PACKET_DISABLE)
{
struct remote_state *rs = get_remote_state ();
@@ -14011,7 +14144,7 @@ remote_target::use_agent (bool use)
bool
remote_target::can_use_agent ()
{
- return (packet_support (PACKET_QAgent) != PACKET_DISABLE);
+ return (m_features.packet_support (PACKET_QAgent) != PACKET_DISABLE);
}
struct btrace_target_info
@@ -14044,7 +14177,7 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
buf = rs->buf.data ();
endbuf = buf + get_remote_packet_size ();
- packet = &remote_protocol_packets[PACKET_Qbtrace_conf_bts_size];
+ packet = &m_features.m_protocol_packets[PACKET_Qbtrace_conf_bts_size];
if (packet_config_support (packet) == PACKET_ENABLE
&& conf->bts.size != rs->btrace_config.bts.size)
{
@@ -14066,7 +14199,7 @@ remote_target::btrace_sync_conf (const btrace_config *conf)
rs->btrace_config.bts.size = conf->bts.size;
}
- packet = &remote_protocol_packets[PACKET_Qbtrace_conf_pt_size];
+ packet = &m_features.m_protocol_packets[PACKET_Qbtrace_conf_pt_size];
if (packet_config_support (packet) == PACKET_ENABLE
&& conf->pt.size != rs->btrace_config.pt.size)
{
@@ -14118,7 +14251,7 @@ remote_target::remote_btrace_maybe_reopen ()
/* Don't bother walking the entirety of the remote thread list when
we know the feature isn't supported by the remote. */
- if (packet_support (PACKET_qXfer_btrace_conf) != PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_qXfer_btrace_conf) != PACKET_ENABLE)
return;
for (thread_info *tp : all_non_exited_threads (this))
@@ -14175,11 +14308,11 @@ remote_target::enable_btrace (thread_info *tp,
switch (conf->format)
{
case BTRACE_FORMAT_BTS:
- packet = &remote_protocol_packets[PACKET_Qbtrace_bts];
+ packet = &m_features.m_protocol_packets[PACKET_Qbtrace_bts];
break;
case BTRACE_FORMAT_PT:
- packet = &remote_protocol_packets[PACKET_Qbtrace_pt];
+ packet = &m_features.m_protocol_packets[PACKET_Qbtrace_pt];
break;
}
@@ -14228,7 +14361,8 @@ remote_target::enable_btrace (thread_info *tp,
void
remote_target::disable_btrace (struct btrace_target_info *tinfo)
{
- struct packet_config *packet = &remote_protocol_packets[PACKET_Qbtrace_off];
+ packet_config *packet
+ = &m_features.m_protocol_packets[PACKET_Qbtrace_off];
struct remote_state *rs = get_remote_state ();
char *buf = rs->buf.data ();
char *endbuf = buf + get_remote_packet_size ();
@@ -14271,7 +14405,7 @@ remote_target::read_btrace (struct btrace_data *btrace,
struct btrace_target_info *tinfo,
enum btrace_read_type type)
{
- struct packet_config *packet = &remote_protocol_packets[PACKET_qXfer_btrace];
+ packet_config *packet = &m_features.m_protocol_packets[PACKET_qXfer_btrace];
const char *annex;
if (packet_config_support (packet) != PACKET_ENABLE)
@@ -14318,8 +14452,9 @@ remote_target::btrace_conf (const struct btrace_target_info *tinfo)
bool
remote_target::augmented_libraries_svr4_read ()
{
- return (packet_support (PACKET_augmented_libraries_svr4_read_feature)
- == PACKET_ENABLE);
+ return
+ (m_features.packet_support (PACKET_augmented_libraries_svr4_read_feature)
+ == PACKET_ENABLE);
}
/* Implementation of to_load. */
@@ -14340,7 +14475,7 @@ remote_target::pid_to_exec_file (int pid)
static gdb::optional<gdb::char_vector> filename;
char *annex = NULL;
- if (packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
return NULL;
inferior *inf = find_inferior_pid (this, pid);
@@ -14372,11 +14507,11 @@ remote_target::can_do_single_step ()
feature. If the stub doesn't support vContSupported feature,
we have conservatively to think target doesn't supports single
step. */
- if (packet_support (PACKET_vContSupported) == PACKET_ENABLE)
+ if (m_features.packet_support (PACKET_vContSupported) == PACKET_ENABLE)
{
struct remote_state *rs = get_remote_state ();
- if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
+ if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
remote_vcont_probe ();
return rs->supports_vCont.s && rs->supports_vCont.S;
@@ -14516,7 +14651,7 @@ remote_target::thread_events (int enable)
struct remote_state *rs = get_remote_state ();
size_t size = get_remote_packet_size ();
- if (packet_support (PACKET_QThreadEvents) == PACKET_DISABLE)
+ if (m_features.packet_support (PACKET_QThreadEvents) == PACKET_DISABLE)
return;
xsnprintf (rs->buf.data (), size, "QThreadEvents:%x", enable ? 1 : 0);
@@ -14524,7 +14659,7 @@ remote_target::thread_events (int enable)
getpkt (&rs->buf, 0);
switch (packet_ok (rs->buf,
- &remote_protocol_packets[PACKET_QThreadEvents]))
+ &m_features.m_protocol_packets[PACKET_QThreadEvents]))
{
case PACKET_OK:
if (strcmp (rs->buf.data (), "OK") != 0)
@@ -14661,10 +14796,10 @@ show_range_stepping (struct ui_file *file, int from_tty,
bool
remote_target::vcont_r_supported ()
{
- if (packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
+ if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
remote_vcont_probe ();
- return (packet_support (PACKET_vCont) == PACKET_ENABLE
+ return (m_features.packet_support (PACKET_vCont) == PACKET_ENABLE
&& get_remote_state ()->supports_vCont.r);
}
@@ -14707,7 +14842,7 @@ show_remote_timeout (struct ui_file *file, int from_tty,
bool
remote_target::supports_memory_tagging ()
{
- return remote_memory_tagging_p ();
+ return m_features.remote_memory_tagging_p ();
}
/* Create the qMemTags packet given ADDRESS, LEN and TYPE. */
@@ -14773,7 +14908,7 @@ remote_target::fetch_memtags (CORE_ADDR address, size_t len,
gdb::byte_vector &tags, int type)
{
/* Make sure the qMemTags packet is supported. */
- if (!remote_memory_tagging_p ())
+ if (!m_features.remote_memory_tagging_p ())
gdb_assert_not_reached ("remote fetch_memtags called with packet disabled");
struct remote_state *rs = get_remote_state ();
@@ -14793,7 +14928,7 @@ remote_target::store_memtags (CORE_ADDR address, size_t len,
const gdb::byte_vector &tags, int type)
{
/* Make sure the QMemTags packet is supported. */
- if (!remote_memory_tagging_p ())
+ if (!m_features.remote_memory_tagging_p ())
gdb_assert_not_reached ("remote store_memtags called with packet disabled");
struct remote_state *rs = get_remote_state ();
@@ -14828,7 +14963,7 @@ test_memory_tagging_functions ()
remote_target remote;
struct packet_config *config
- = &remote_protocol_packets[PACKET_memory_tagging_feature];
+ = &remote.m_features.m_protocol_packets[PACKET_memory_tagging_feature];
scoped_restore restore_memtag_support_
= make_scoped_restore (&config->support);
diff --git a/gdb/testsuite/gdb.base/cond-eval-mode.exp b/gdb/testsuite/gdb.base/cond-eval-mode.exp
index 2c3779536c..5db9d9e9b5 100644
--- a/gdb/testsuite/gdb.base/cond-eval-mode.exp
+++ b/gdb/testsuite/gdb.base/cond-eval-mode.exp
@@ -51,13 +51,18 @@ gdb_test_multiple $test_target $test_target {
# evaluation. Now make sure we can force-disable the
# ConditionalBreakpoints RSP feature.
if [gdb_is_target_remote] {
- gdb_test_no_output "set remote conditional-breakpoints-packet off"
+ gdb_test \
+ "set remote conditional-breakpoints-packet off" \
+ "Use of the 'ConditionalBreakpoints' packet for the current and future remote targets is set to \"off\"."
gdb_test $test_target "$warning" \
"set breakpoint condition-evaluation target, with support disabled"
# Confirm we can re-enable it.
- gdb_test_no_output "set remote conditional-breakpoints-packet on"
+ gdb_test \
+ "set remote conditional-breakpoints-packet on" \
+ "Use of the 'ConditionalBreakpoints' packet for the current and future remote targets is set to \"on\"."
+
gdb_test_no_output $test_target "restore $test_target"
}
diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
index 0b209c02a6..ed15604433 100644
--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -204,7 +204,10 @@ gdb_test "set dprintf-style foobar" "Undefined item: \"foobar\"." \
# as expected. dprintf relies on support for target-side breakpoint
# commands --- use it as proxy.
if [gdb_is_target_remote] {
- gdb_test_no_output "set remote breakpoint-commands-packet off"
+ gdb_test \
+ "set remote breakpoint-commands-packet off" \
+ "Use of the 'BreakpointCommands' packet for the current and future remote targets is set to \"off\"."
+
gdb_test "set dprintf-style agent" \
"warning: Target cannot run dprintf commands.*" \
"set dprintf-style agent, with feature disabled"
diff --git a/gdb/testsuite/gdb.base/find-unmapped.exp b/gdb/testsuite/gdb.base/find-unmapped.exp
index e9c70e5ada..b29f487462 100644
--- a/gdb/testsuite/gdb.base/find-unmapped.exp
+++ b/gdb/testsuite/gdb.base/find-unmapped.exp
@@ -78,7 +78,10 @@ if {[target_info gdb_protocol] == "remote"
|| [target_info gdb_protocol] == "extended-remote"} {
test_not_found 0
with_test_prefix "search-memory-packet off" {
- gdb_test_no_output "set remote search-memory-packet off"
+ gdb_test \
+ "set remote search-memory-packet off" \
+ "Use of the 'qSearch:memory' packet for the current and future remote targets is set to \"off\"."
+
test_not_found 0
}
} else {
diff --git a/gdb/testsuite/gdb.base/hbreak-in-shr-unsupported.exp b/gdb/testsuite/gdb.base/hbreak-in-shr-unsupported.exp
index 13c01de02e..ef9b32c95d 100644
--- a/gdb/testsuite/gdb.base/hbreak-in-shr-unsupported.exp
+++ b/gdb/testsuite/gdb.base/hbreak-in-shr-unsupported.exp
@@ -58,7 +58,9 @@ gdb_test_no_output "set breakpoint always-inserted on"
# Force-disable Z1 packets, in case the target actually supports
# these.
if {$is_target_remote} {
- gdb_test_no_output "set remote Z-packet off"
+ gdb_test \
+ "set remote Z-packet off" \
+ "Use of Z packets for the current and future remote targets is set to \"off\"."
}
# Probe for hw breakpoints support. With Z packets disabled, this
diff --git a/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp b/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
index 8b0c6c91a2..712dd0b7e5 100644
--- a/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
+++ b/gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp
@@ -30,15 +30,13 @@ set run_python_tests [expr ! [skip_python_tests]]
# indicates whether the multi-process feature of remote targets is
# turned off or on.
proc test_info_inferiors {multi_process} {
- setup "off"
+
+ setup "off" $multi_process
if { $::run_python_tests } {
gdb_test_no_output "source ${::remote_python_file}" "load python file"
}
- gdb_test_no_output \
- "set remote multiprocess-feature-packet $multi_process"
-
# Get the description for inferior INF for when the current
# inferior id is CURRENT.
proc inf_desc {inf current} {
diff --git a/gdb/testsuite/gdb.multi/multi-target.exp.tcl b/gdb/testsuite/gdb.multi/multi-target.exp.tcl
index 8e0c39679b..9a62129d8d 100644
--- a/gdb/testsuite/gdb.multi/multi-target.exp.tcl
+++ b/gdb/testsuite/gdb.multi/multi-target.exp.tcl
@@ -104,7 +104,7 @@ proc cleanup_gdbservers { } {
# Return true on success, false otherwise.
-proc setup {non-stop} {
+proc setup {non-stop {multi_process ""}} {
global gcorefile gcore_created
global binfile
@@ -123,6 +123,12 @@ proc setup {non-stop} {
gdb_test_no_output "set non-stop ${non-stop}"
+ if {${multi_process} ne ""} then {
+ gdb_test \
+ "set remote multiprocess-feature-packet $multi_process" \
+ "Use of the 'multiprocess-feature' packet for future remote targets is set to \"${multi_process}\"."
+ }
+
if ![runto all_started] then {
return 0
}
diff --git a/gdb/testsuite/gdb.server/connect-without-multi-process.exp b/gdb/testsuite/gdb.server/connect-without-multi-process.exp
index c4bbcac72f..d7156a81ea 100644
--- a/gdb/testsuite/gdb.server/connect-without-multi-process.exp
+++ b/gdb/testsuite/gdb.server/connect-without-multi-process.exp
@@ -47,7 +47,9 @@ proc do_test {multiprocess} {
# extended-remote board, therefore already connected.
gdb_test "disconnect" ".*"
- gdb_test_no_output "set remote multiprocess-feature $multiprocess"
+ gdb_test \
+ "set remote multiprocess-feature $multiprocess" \
+ "Use of the 'multiprocess-feature' packet for future remote targets is set to \"$multiprocess\"."
set res [gdbserver_spawn ""]
set gdbserver_protocol [lindex $res 0]
diff --git a/gdb/testsuite/gdb.server/exit-multiple-threads.exp b/gdb/testsuite/gdb.server/exit-multiple-threads.exp
index 23ac512ef7..0b1b053224 100644
--- a/gdb/testsuite/gdb.server/exit-multiple-threads.exp
+++ b/gdb/testsuite/gdb.server/exit-multiple-threads.exp
@@ -57,9 +57,14 @@ proc prepare_for_test { executable disable_multi_process } {
# Disable XML-based thread listing, and possible the multi-process
# extensions.
- gdb_test_no_output "set remote threads-packet off"
+ gdb_test \
+ "set remote threads-packet off" \
+ "Use of the 'qXfer:threads:read' packet for future remote targets is set to \"off\"."
+
if { $disable_multi_process } {
- gdb_test_no_output "set remote multiprocess-feature-packet off"
+ gdb_test \
+ "set remote multiprocess-feature-packet off" \
+ "Use of the 'multiprocess-feature' packet for future remote targets is set to \"off\"."
}
# Start gdbserver and connect.
diff --git a/gdb/testsuite/gdb.server/ext-restart.exp b/gdb/testsuite/gdb.server/ext-restart.exp
index ac03caa0aa..ae4edf5411 100644
--- a/gdb/testsuite/gdb.server/ext-restart.exp
+++ b/gdb/testsuite/gdb.server/ext-restart.exp
@@ -53,7 +53,10 @@ gdb_test "run" "Breakpoint.* main .*" "run to main"
with_test_prefix "restart" {
# Disable vRun packet and clear remote exec-file, so that GDB will
# use R packet to restart the process.
- gdb_test_no_output "set remote run-packet off"
+ gdb_test \
+ "set remote run-packet off" \
+ "Use of the 'vRun' packet for the current and future remote targets is set to \"off\"."
+
gdb_test_no_output "set remote exec-file"
set test "run to main"
diff --git a/gdb/testsuite/gdb.server/ext-wrapper.exp b/gdb/testsuite/gdb.server/ext-wrapper.exp
index 4c6cea4cd8..6331cc4331 100644
--- a/gdb/testsuite/gdb.server/ext-wrapper.exp
+++ b/gdb/testsuite/gdb.server/ext-wrapper.exp
@@ -57,7 +57,10 @@ gdb_test "print d" "\\$${decimal} = ${hex} \"1\".*"
with_test_prefix "restart" {
# Disable vRun packet and clear remote exec-file, so that GDB will
# use R packet to restart the process.
- gdb_test_no_output "set remote run-packet off"
+ gdb_test \
+ "set remote run-packet off" \
+ "Use of the 'vRun' packet for the current and future remote targets is set to \"off\"."
+
gdb_test_no_output "set remote exec-file"
set test "run to marker"
gdb_test_multiple "run" $test {
diff --git a/gdb/testsuite/gdb.server/server-exec-info.exp b/gdb/testsuite/gdb.server/server-exec-info.exp
index 8ab33af336..18e87dcf3d 100644
--- a/gdb/testsuite/gdb.server/server-exec-info.exp
+++ b/gdb/testsuite/gdb.server/server-exec-info.exp
@@ -31,7 +31,10 @@ if [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] {
# extended-remote board, therefore already connected.
gdb_test "disconnect" ".*"
-gdb_test_no_output "set remote pid-to-exec-file-packet off"
+gdb_test \
+ "set remote pid-to-exec-file-packet off" \
+ "Use of the 'qXfer:exec-file:read' packet for future remote targets is set to \"off\"."
+
gdb_test "file" ".*" "file" \
{Discard symbol table from `.*'\? \(y or n\) } "y"
gdbserver_run ""
diff --git a/gdb/testsuite/gdb.server/server-kill.exp b/gdb/testsuite/gdb.server/server-kill.exp
index 93daf48290..9eb0b0f68c 100644
--- a/gdb/testsuite/gdb.server/server-kill.exp
+++ b/gdb/testsuite/gdb.server/server-kill.exp
@@ -116,7 +116,9 @@ proc_with_prefix test_tstatus {} {
# Enable trace status packet which is disabled after the
# connection if the remote target doesn't support tracepoint at
# all. Otherwise, no RSP packet is sent out.
- gdb_test_no_output "set remote trace-status-packet on"
+ gdb_test \
+ "set remote trace-status-packet on" \
+ "Use of the 'qTStatus' packet for the current and future remote targets is set to \"on\"."
# Force GDB to talk with GDBserver, so that we can get the
# "connection closed" error.
diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp
index 3bc7cbed52..82e1ec6dcf 100644
--- a/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp
+++ b/gdb/testsuite/gdb.server/stop-reply-no-thread-multi.exp
@@ -71,8 +71,12 @@ proc run_test { target_non_stop disable_feature } {
set gdbserver_gdbport [lindex $res 1]
# Disable XML-based thread listing, and multi-process extensions.
- gdb_test_no_output "set remote threads-packet off"
- gdb_test_no_output "set remote multiprocess-feature-packet off"
+ gdb_test \
+ "set remote threads-packet off" \
+ "Use of the 'qXfer:threads:read' packet for future remote targets is set to \"off\"."
+ gdb_test \
+ "set remote multiprocess-feature-packet off" \
+ "Use of the 'multiprocess-feature' packet for future remote targets is set to \"off\"."
set res [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
if ![gdb_assert {$res == 0} "connect"] {
diff --git a/gdb/testsuite/gdb.server/stop-reply-no-thread.exp b/gdb/testsuite/gdb.server/stop-reply-no-thread.exp
index 488a585996..7032839ea8 100644
--- a/gdb/testsuite/gdb.server/stop-reply-no-thread.exp
+++ b/gdb/testsuite/gdb.server/stop-reply-no-thread.exp
@@ -59,8 +59,13 @@ proc run_test { disable_feature target_nonstop } {
set gdbserver_gdbport [lindex $res 1]
# Disable XML-based thread listing, and multi-process extensions.
- gdb_test_no_output "set remote threads-packet off"
- gdb_test_no_output "set remote multiprocess-feature-packet off"
+ gdb_test \
+ "set remote threads-packet off" \
+ "Use of the 'qXfer:threads:read' packet for future remote targets is set to \"off\"."
+
+ gdb_test \
+ "set remote multiprocess-feature-packet off" \
+ "Use of the 'multiprocess-feature' packet for future remote targets is set to \"off\"."
# Set target-nonstop mode.
gdb_test_no_output "maint set target-non-stop ${target_nonstop}"
diff --git a/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp b/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
index d630805941..1923abff64 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-handling-bp.exp
@@ -58,15 +58,18 @@ proc do_test { non_stop cond_bp_target } {
if {!$cond_bp_target} {
# Leaving breakpoint evaluation to GDB exposes failures
# similar to native debugging.
- gdb_test_no_output "set remote conditional-breakpoints-packet off"
+ gdb_test \
+ "set remote conditional-breakpoints-packet off" \
+ "Use of the 'ConditionalBreakpoints' packet for the current and future remote targets is set to \"off\"."
+
set should_kfail 1
} else {
set test "show remote conditional-breakpoints-packet"
gdb_test_multiple $test $test {
- -re "currently enabled\.\r\n$gdb_prompt $" {
+ -re "\(currently enabled\).*\r\n$gdb_prompt $" {
pass $test
}
- -re "currently disabled\.\r\n$gdb_prompt $" {
+ -re "\(currently disabled\).*\r\n$gdb_prompt $" {
unsupported "no support for target-side conditional breakpoints"
return
}
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index cbb4fa3607..305cd867c5 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -324,7 +324,9 @@ proc tracepoint_install_in_trace_disabled { trace_type } {
gdb_test_no_output "tstart"
# Force-disable the InstallInTrace RSP feature.
- gdb_test_no_output "set remote install-in-trace-packet off"
+ gdb_test \
+ "set remote install-in-trace-packet off" \
+ "Use of the 'InstallInTrace' packet for the current and future remote targets is set to \"off\"."
# Set a tracepoint while a trace experiment is ongoing.
gdb_test "${trace_type} set_tracepoint" \
diff --git a/gdb/testsuite/gdb.trace/qtro.exp b/gdb/testsuite/gdb.trace/qtro.exp
index 62f937f43a..62b1bfb86a 100644
--- a/gdb/testsuite/gdb.trace/qtro.exp
+++ b/gdb/testsuite/gdb.trace/qtro.exp
@@ -79,7 +79,7 @@ prepare_for_trace_disassembly
set traceframe_info_supported -1
set test "probe for traceframe-info support"
gdb_test_multiple "show remote traceframe-info-packet" $test {
- -re ".*Support for .* is auto-detected, currently (\[a-z\]*).*$gdb_prompt $" {
+ -re ".*Support for .* is \"auto\" (\[a-z\]*).*$gdb_prompt $" {
set status $expect_out(1,string)
if { $status == "enabled" } {
@@ -110,7 +110,9 @@ foreach tfinfo { auto off } {
clean_restart $testfile
runto_main
- gdb_test_no_output "set remote traceframe-info-packet $tfinfo"
+ gdb_test \
+ "set remote traceframe-info-packet $tfinfo" \
+ "Use of the 'qXfer:traceframe-info:read' packet for the current and future remote targets is set to *.*$tfinfo*.*"
prepare_for_trace_disassembly
--
2.25.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] gdb: Add per-remote target variables for memory read and write config
2022-03-29 13:11 [PATCH v2 0/3] Apply fixme notes for multi-target support Christina Schimpe
2022-03-29 13:11 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
@ 2022-03-29 13:11 ` Christina Schimpe
2022-03-29 13:48 ` Eli Zaretskii
2022-04-18 14:56 ` Tom Tromey
2022-03-29 13:11 ` [PATCH v2 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
2 siblings, 2 replies; 20+ messages in thread
From: Christina Schimpe @ 2022-03-29 13:11 UTC (permalink / raw)
To: gdb-patches
This patch adds per-remote target variables for the configuration of
memory read- and write packet size. It is a further change to commit
"gdb: Make global feature array a per-remote target array" to apply the
fixme notes described in commit 5b6d1e4 "Multi-target support".
The former global variables for that configuration are still available
to allow the command line configuration for all future remote
connections. Similar to the command line configuration of the per-
remote target feature array, the commands
- set remotewritesize (deprecated)
- set remote memory-read-packet-size
- set remote memory-write-packet-size
will configure the current target (if available) and future remote
connections. The show command will display the current remote target's
packet size configuration. If no remote target is selected the default
configuration for future connections will be shown.
It is required to adapt the test gdb.base/remote.exp which is failing
for --target_board=native-extended-gdbserver. With that board GDB
connects to gdbserver at gdb start time. Due to this patch two loggings
"The target may not be able to.." are shown if the command 'set remote
memory-write-packet-size fixed' is executed while a target is connected
for the current inferior. To fix this, the clean_restart command is
moved to a later time point of the test. It is sufficient to be
connected to the server when "runto_main" is executed. Now the
connection time is similar to a testrun with
--target_board=native-gdbserver.
To allow the user to distinguish between the packet-size configuration
for future connections and for the currently selected target, the
logging of the command 'set remote memory-write-packet-size fixed' is
adapted.
---
gdb/NEWS | 12 +++
gdb/doc/gdb.texinfo | 14 +++
gdb/remote.c | 143 +++++++++++++++++++++---------
gdb/testsuite/gdb.base/remote.exp | 45 ++++++----
4 files changed, 154 insertions(+), 60 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 7740162d3e..aa704d076d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -16,6 +16,18 @@
the configuration of the currently selected target. If no remote target is
selected, the default configuration for future connections is shown.
+ The individual packet sizes can be configured and shown using the commands
+ ** 'set remote memory-read-packet-size (number of bytes|fixed|limit)'
+ ** 'set remote memory-write-packet-size (number of bytes|fixed|limit)'
+ ** 'show remote memory-read-packet-size'
+ ** 'show remote memory-write-packet-size'.
+
+ The configuration of the packet itself, as well as the size of a memory-read
+ or memory-write packet, applies to the currently selected target (if
+ available) and future remote connections. The "show remote" commands print
+ the configuration of the currently selected target. If no remote target is
+ selected, the default configuration for future connections is shown.
+
* GDB now supports hardware watchpoints on FreeBSD/Aarch64.
* Remove support for building against Python 2, it is now only possible to
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2c5e4d1b1b..d9f70476d2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23612,6 +23612,20 @@ future connections is shown. The available settings are:
@end multitable
+@cindex Configuration of the remote packet size
+The number of bytes per memory-read or memory-write packet for a remote target
+can be configured using the commands @code{set remote memory-read-packet-size}
+and @code{set remote memory-write-packet-size}. If set to @samp{0} (zero) the
+default packet size will be used. The actual limit is further reduced depending
+on the target. Specify @samp{fixed} to disable the target-dependent restriction
+and @samp{limit} to enable it. Similar to the enabling and disabling of remote
+packets, the command applies to the currently selected target (if available) and
+all future remote connections.
+The configuration of the selected target can be displayed using the commands
+@code{show remote memory-read-packet-size} and
+@code{show remote memory-write-packet-size}. If no remote target is selected,
+the default configuration for future connections is shown.
+
@node Remote Stub
@section Implementing a Remote Stub
diff --git a/gdb/remote.c b/gdb/remote.c
index 5d9d328968..510af52c49 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -602,8 +602,31 @@ struct packet_config
enum packet_support support;
};
-/* This global array contains the default configuration for every new
- per-remote target array. */
+/* User configurable variables for the number of characters in a
+ memory read/write packet. MIN (rsa->remote_packet_size,
+ rsa->sizeof_g_packet) is the default. Some targets need smaller
+ values (fifo overruns, et.al.) and some users need larger values
+ (speed up transfers). The variables ``preferred_*'' (the user
+ request), ``current_*'' (what was actually set) and ``forced_*''
+ (Positive - a soft limit, negative - a hard limit). */
+
+struct memory_packet_config
+{
+ const char *name;
+ long size;
+ int fixed_p;
+};
+
+/* These global variables contain the default configuration for every new
+ remote_feature object. */
+static memory_packet_config memory_read_packet_config =
+{
+ "memory-read-packet-size",
+};
+static memory_packet_config memory_write_packet_config =
+{
+ "memory-write-packet-size",
+};
static packet_config remote_protocol_packets[PACKET_MAX];
/* Description of a remote target's features. It stores the configuration
@@ -613,6 +636,9 @@ struct remote_features
{
remote_features ()
{
+ m_memory_read_packet_config = memory_read_packet_config;
+ m_memory_write_packet_config = memory_write_packet_config;
+
std::copy (std::begin (remote_protocol_packets),
std::end (remote_protocol_packets),
std::begin (m_protocol_packets));
@@ -653,6 +679,11 @@ struct remote_features
new connection to a remote target. */
void reset_all_packet_configs_support ();
+ /* Configuration of a remote target's memory read packet. */
+ memory_packet_config m_memory_read_packet_config;
+ /* Configuration of a remote target's memory write packet. */
+ memory_packet_config m_memory_write_packet_config;
+
/* The per-remote target array which stores a remote's packet
configurations. */
packet_config m_protocol_packets[PACKET_MAX];
@@ -1906,21 +1937,6 @@ show_remotebreak (struct ui_file *file, int from_tty,
static unsigned int remote_address_size;
\f
-/* User configurable variables for the number of characters in a
- memory read/write packet. MIN (rsa->remote_packet_size,
- rsa->sizeof_g_packet) is the default. Some targets need smaller
- values (fifo overruns, et.al.) and some users need larger values
- (speed up transfers). The variables ``preferred_*'' (the user
- request), ``current_*'' (what was actually set) and ``forced_*''
- (Positive - a soft limit, negative - a hard limit). */
-
-struct memory_packet_config
-{
- const char *name;
- long size;
- int fixed_p;
-};
-
/* The default max memory-write-packet-size, when the setting is
"fixed". The 16k is historical. (It came from older GDB's using
alloca for buffers and the knowledge (folklore?) that some hosts
@@ -1986,7 +2002,8 @@ remote_target::get_memory_packet_size (struct memory_packet_config *config)
something really big then do a sanity check. */
static void
-set_memory_packet_size (const char *args, struct memory_packet_config *config)
+set_memory_packet_size (const char *args, struct memory_packet_config *config,
+ bool target_connected)
{
int fixed_p = config->fixed_p;
long size = config->size;
@@ -2020,9 +2037,16 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config)
? DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED
: size);
- if (! query (_("The target may not be able to correctly handle a %s\n"
- "of %ld bytes. Change the packet size? "),
- config->name, query_size))
+ if (target_connected
+ && !query (_("The target may not be able to correctly handle a %s\n"
+ "of %ld bytes. Change the packet size? "),
+ config->name, query_size))
+ error (_("Packet size not changed."));
+ else if (!target_connected
+ && !query (_("Future targets may not be able to correctly "
+ "handle a %s\nof %ld bytes. Change the packet "
+ "size for future remote targets? "),
+ config->name, query_size))
error (_("Packet size not changed."));
}
/* Update the config. */
@@ -2031,20 +2055,23 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config)
}
static void
-show_memory_packet_size (struct memory_packet_config *config)
+show_memory_packet_size (memory_packet_config *config, remote_target *remote)
{
+ const char* target_type = get_target_type_name (remote != nullptr);
+
if (config->size == 0)
- printf_filtered (_("The %s is 0 (default). "), config->name);
+ printf_filtered (_("The %s %s is 0 (default). "),
+ config->name, target_type);
else
- printf_filtered (_("The %s is %ld. "), config->name, config->size);
+ printf_filtered (_("The %s %s is %ld. "), config->name, target_type,
+ config->size);
+
if (config->fixed_p)
printf_filtered (_("Packets are fixed at %ld bytes.\n"),
get_fixed_memory_packet_size (config));
else
{
- remote_target *remote = get_current_remote_target ();
-
- if (remote != NULL)
+ if (remote != nullptr)
printf_filtered (_("Packets are limited to %ld bytes.\n"),
remote->get_memory_packet_size (config));
else
@@ -2053,22 +2080,46 @@ show_memory_packet_size (struct memory_packet_config *config)
}
}
-/* FIXME: needs to be per-remote-target. */
-static struct memory_packet_config memory_write_packet_config =
+/* Print the current configuration of memory-read or write-packet size.
+ If TARGET_CONNECTED is true the currently selected target's configuration
+ should be passed in CONFIG, if false the default configuration for future
+ remote targets. */
+
+static void
+print_set_memory_packet_size (const char *args,
+ const memory_packet_config* config,
+ bool target_connected)
{
- "memory-write-packet-size",
-};
+ if (target_connected)
+ printf_filtered (_("Use of the %s for the current and future remote "
+ "targets is set to \"%s\".\n"), config->name, args);
+ else
+ printf_filtered (_("Use of the %s for future remote targets is set to "
+ "\"%s\".\n"), config->name, args);
+}
static void
set_memory_write_packet_size (const char *args, int from_tty)
{
- set_memory_packet_size (args, &memory_write_packet_config);
+ remote_target *remote = get_current_remote_target ();
+ if (remote != nullptr)
+ set_memory_packet_size
+ (args, &remote->m_features.m_memory_write_packet_config, true);
+
+ memory_packet_config* config = &memory_write_packet_config;
+ set_memory_packet_size (args, config, false);
+ print_set_memory_packet_size (args, config, remote != nullptr);
}
static void
show_memory_write_packet_size (const char *args, int from_tty)
{
- show_memory_packet_size (&memory_write_packet_config);
+ remote_target *remote = get_current_remote_target ();
+ if (remote != nullptr)
+ show_memory_packet_size (&remote->m_features.m_memory_write_packet_config,
+ remote);
+ else
+ show_memory_packet_size (&memory_write_packet_config, nullptr);
}
/* Show the number of hardware watchpoints that can be used. */
@@ -2124,31 +2175,37 @@ show_remote_packet_max_chars (struct ui_file *file, int from_tty,
long
remote_target::get_memory_write_packet_size ()
{
- return get_memory_packet_size (&memory_write_packet_config);
+ return get_memory_packet_size (&m_features.m_memory_write_packet_config);
}
-/* FIXME: needs to be per-remote-target. */
-static struct memory_packet_config memory_read_packet_config =
-{
- "memory-read-packet-size",
-};
-
static void
set_memory_read_packet_size (const char *args, int from_tty)
{
- set_memory_packet_size (args, &memory_read_packet_config);
+ remote_target *remote = get_current_remote_target ();
+ if (remote != nullptr)
+ set_memory_packet_size
+ (args, &remote->m_features.m_memory_read_packet_config, true);
+
+ memory_packet_config* config = &memory_read_packet_config;
+ set_memory_packet_size (args, config, false);
+ print_set_memory_packet_size (args, config, remote != nullptr);
}
static void
show_memory_read_packet_size (const char *args, int from_tty)
{
- show_memory_packet_size (&memory_read_packet_config);
+ remote_target *remote = get_current_remote_target ();
+ if (remote != nullptr)
+ show_memory_packet_size (&remote->m_features.m_memory_read_packet_config,
+ remote);
+ else
+ show_memory_packet_size (&memory_read_packet_config, nullptr);
}
long
remote_target::get_memory_read_packet_size ()
{
- long size = get_memory_packet_size (&memory_read_packet_config);
+ long size = get_memory_packet_size (&m_features.m_memory_read_packet_config);
/* FIXME: cagney/1999-11-07: Functions like getpkt() need to get an
extra buffer size argument before the memory read size can be
diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp
index 1f0869433f..935f7e5aee 100644
--- a/gdb/testsuite/gdb.base/remote.exp
+++ b/gdb/testsuite/gdb.base/remote.exp
@@ -38,31 +38,37 @@ gdb_test "disconnect" ".*"
#
gdb_test "show remote memory-write-packet-size" \
- "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
+ "The memory-write-packet-size on newly created remote targets is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
"write-packet default"
gdb_test "set remote memory-write-packet-size" \
"Argument required .integer, `fixed' or `limited'.\." \
"set write-packet - NULL"
-gdb_test_no_output "set remote memory-write-packet-size 20"
+gdb_test "set remote memory-write-packet-size 20" \
+ "Use of the memory-write-packet-size for future remote targets is set to \"20\"."
+
gdb_test "show remote memory-write-packet-size" \
- "The memory-write-packet-size is 20. The actual limit will be further reduced dependent on the target\." \
+ "The memory-write-packet-size on newly created remote targets is 20. The actual limit will be further reduced dependent on the target\." \
"set write-packet - small"
-gdb_test_no_output "set remote memory-write-packet-size 1"
+gdb_test "set remote memory-write-packet-size 1" \
+ "Use of the memory-write-packet-size for future remote targets is set to \"1\"."
+
gdb_test "show remote memory-write-packet-size" \
- "The memory-write-packet-size is 1. The actual limit will be further reduced dependent on the target\." \
+ "The memory-write-packet-size on newly created remote targets is 1. The actual limit will be further reduced dependent on the target\." \
"set write-packet - very-small"
-gdb_test_no_output "set remote memory-write-packet-size 0"
+gdb_test "set remote memory-write-packet-size 0" \
+ "Use of the memory-write-packet-size for future remote targets is set to \"0\"."
+
gdb_test "show remote memory-write-packet-size" \
- "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
+ "The memory-write-packet-size on newly created remote targets is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
"write-packet default again"
set test "set remote memory-write-packet-size fixed"
gdb_test_multiple $test $test {
- -re "Change the packet size. .y or n. " {
+ -re "Change the packet size for future remote targets. .y or n. " {
gdb_test_multiple "y" $test {
-re "$gdb_prompt $" {
pass $test
@@ -70,13 +76,16 @@ gdb_test_multiple $test $test {
}
}
}
+
gdb_test "show remote memory-write-packet-size" \
- "The memory-write-packet-size is 0 \\(default\\). Packets are fixed at 16384 bytes\." \
+ "The memory-write-packet-size on newly created remote targets is 0 \\(default\\). Packets are fixed at 16384 bytes\." \
"write-packet default fixed"
-gdb_test_no_output "set remote memory-write-packet-size limit"
+gdb_test "set remote memory-write-packet-size limit" \
+ "Use of the memory-write-packet-size for future remote targets is set to \"limit\"."
+
gdb_test "show remote memory-write-packet-size" \
- "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
+ "The memory-write-packet-size on newly created remote targets is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
"write-packet default limit again"
#
@@ -88,8 +97,9 @@ proc gdb_load_timed {executable class writesize} {
set test "timed download `[file tail $executable]' - $class, $writesize"
if {$writesize != ""} then {
- gdb_test_no_output "set remote memory-write-packet-size $writesize" \
- "$test - set packet size"
+ gdb_test "set remote memory-write-packet-size $writesize" \
+ "Use of the memory-write-packet-size for future remote targets is set to \"$writesize\"." \
+ "$test - set packet size"
send_gdb "set remote memory-write-packet-size $class\n"
gdb_expect 5 {
@@ -129,8 +139,6 @@ proc gdb_load_timed {executable class writesize} {
pass $test
}
-clean_restart $binfile
-
# These download tests won't actually download anything on !is_remote
# target boards, but we run them anyway because it's simpler, and
# harmless.
@@ -155,6 +163,8 @@ gdb_load_timed $binfile "limit" 0
# Get the size of random_data table (defaults to 48K).
set sizeof_random_data [get_sizeof "random_data" 48*1024]
+clean_restart $binfile
+
#
# Part THREE: Check the upload behavour
#
@@ -178,10 +188,11 @@ if {$sizeof_random_data > 16380 } then {
# Read a chunk just larger than the packet size (reduce the packet
# size to make life easier)
-gdb_test_no_output "set remote memory-read-packet-size 16"
+gdb_test "set remote memory-read-packet-size 16" \
+ "Use of the memory-read-packet-size for the current and future remote targets is set to \"16\"."
gdb_test "show remote memory-read-packet-size" \
- "The memory-read-packet-size is 16. Packets are limited to 20 bytes."
+ "The memory-read-packet-size on the current remote target is 16. Packets are limited to 20 bytes."
gdb_test "x/17ub random_data" \
"<random_data>:\[ \t\]+60\[ \t\]+74\[ \t\]+216\[ \t\]+38\[ \t\]+149\[ \t\]+49\[ \t\]+207\[ \t\]+44.*<random_data\\+8>:\[ \t\]+124\[ \t\]+38\[ \t\]+93\[ \t\]+125\[ \t\]+232\[ \t\]+67\[ \t\]+228\[ \t\]+56.*<random_data\\+16>:\[ \t\]+161"
--
2.25.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] gdb: Remove workaround for the vCont packet
2022-03-29 13:11 [PATCH v2 0/3] Apply fixme notes for multi-target support Christina Schimpe
2022-03-29 13:11 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
2022-03-29 13:11 ` [PATCH v2 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
@ 2022-03-29 13:11 ` Christina Schimpe
2022-04-18 14:59 ` Tom Tromey
2 siblings, 1 reply; 20+ messages in thread
From: Christina Schimpe @ 2022-03-29 13:11 UTC (permalink / raw)
To: gdb-patches
The workaround for the vCont packet is no longer required due to the
former commit "gdb: Make global feature array a per-remote target array".
The vCont packet is now checked once when the connection is started and
the supported vCont actions are set to the target's remote state
attribute.
---
gdb/remote.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 510af52c49..5536ce7be2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -455,9 +455,6 @@ class remote_state
/* The status of the stub support for the various vCont actions. */
vCont_action_support supports_vCont;
- /* Whether vCont support was probed already. This is a workaround
- until packet_support is per-connection. */
- bool supports_vCont_probed;
/* True if the user has pressed Ctrl-C, but the target hasn't
responded to that. */
@@ -4934,6 +4931,10 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
which later probes to skip. */
remote_query_supported ();
+ /* Check vCont support and set the remote state's vCont_action_support
+ attribute. */
+ remote_vcont_probe ();
+
/* If the stub wants to get a QAllow, compose one and send it. */
if (m_features.packet_support (PACKET_QAllow) != PACKET_DISABLE)
set_permissions ();
@@ -6444,7 +6445,6 @@ remote_target::remote_vcont_probe ()
}
packet_ok (rs->buf, &m_features.m_protocol_packets[PACKET_vCont]);
- rs->supports_vCont_probed = true;
}
/* Helper function for building "vCont" resumptions. Write a
@@ -6634,9 +6634,6 @@ remote_target::remote_resume_with_vcont (ptid_t ptid, int step,
if (::execution_direction == EXEC_REVERSE)
return 0;
- if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
- remote_vcont_probe ();
-
if (m_features.packet_support (PACKET_vCont) == PACKET_DISABLE)
return 0;
@@ -7168,12 +7165,6 @@ remote_target::remote_stop_ns (ptid_t ptid)
}
}
- /* FIXME: This supports_vCont_probed check is a workaround until
- packet_support is per-connection. */
- if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN
- || !rs->supports_vCont_probed)
- remote_vcont_probe ();
-
if (!rs->supports_vCont.t)
error (_("Remote server does not support stopping threads"));
@@ -14568,9 +14559,6 @@ remote_target::can_do_single_step ()
{
struct remote_state *rs = get_remote_state ();
- if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
- remote_vcont_probe ();
-
return rs->supports_vCont.s && rs->supports_vCont.S;
}
else
@@ -14853,9 +14841,6 @@ show_range_stepping (struct ui_file *file, int from_tty,
bool
remote_target::vcont_r_supported ()
{
- if (m_features.packet_support (PACKET_vCont) == PACKET_SUPPORT_UNKNOWN)
- remote_vcont_probe ();
-
return (m_features.packet_support (PACKET_vCont) == PACKET_ENABLE
&& get_remote_state ()->supports_vCont.r);
}
--
2.25.1
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array
2022-03-29 13:11 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
@ 2022-03-29 13:45 ` Eli Zaretskii
2022-04-18 14:56 ` Tom Tromey
2022-04-18 19:01 ` Pedro Alves
2 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2022-03-29 13:45 UTC (permalink / raw)
To: Christina Schimpe; +Cc: gdb-patches
> Date: Tue, 29 Mar 2022 15:11:56 +0200
> From: Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org>
>
> gdb/NEWS | 13 +
> gdb/doc/gdb.texinfo | 10 +-
> gdb/remote.c | 1057 ++++++++++-------
> gdb/testsuite/gdb.base/cond-eval-mode.exp | 9 +-
> gdb/testsuite/gdb.base/dprintf.exp | 5 +-
> gdb/testsuite/gdb.base/find-unmapped.exp | 5 +-
> .../gdb.base/hbreak-in-shr-unsupported.exp | 4 +-
> .../gdb.multi/multi-target-info-inferiors.exp | 6 +-
> gdb/testsuite/gdb.multi/multi-target.exp.tcl | 8 +-
> .../connect-without-multi-process.exp | 4 +-
> .../gdb.server/exit-multiple-threads.exp | 9 +-
> gdb/testsuite/gdb.server/ext-restart.exp | 5 +-
> gdb/testsuite/gdb.server/ext-wrapper.exp | 5 +-
> gdb/testsuite/gdb.server/server-exec-info.exp | 5 +-
> gdb/testsuite/gdb.server/server-kill.exp | 4 +-
> .../gdb.server/stop-reply-no-thread-multi.exp | 8 +-
> .../gdb.server/stop-reply-no-thread.exp | 9 +-
> .../process-dies-while-handling-bp.exp | 9 +-
> gdb/testsuite/gdb.trace/change-loc.exp | 4 +-
> gdb/testsuite/gdb.trace/qtro.exp | 6 +-
> 20 files changed, 695 insertions(+), 490 deletions(-)
The documentation parts are OK, thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] gdb: Add per-remote target variables for memory read and write config
2022-03-29 13:11 ` [PATCH v2 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
@ 2022-03-29 13:48 ` Eli Zaretskii
2022-04-18 14:56 ` Tom Tromey
1 sibling, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2022-03-29 13:48 UTC (permalink / raw)
To: Christina Schimpe; +Cc: gdb-patches
> Date: Tue, 29 Mar 2022 15:11:57 +0200
> From: Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org>
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 7740162d3e..aa704d076d 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -16,6 +16,18 @@
> the configuration of the currently selected target. If no remote target is
> selected, the default configuration for future connections is shown.
>
> + The individual packet sizes can be configured and shown using the commands
> + ** 'set remote memory-read-packet-size (number of bytes|fixed|limit)'
> + ** 'set remote memory-write-packet-size (number of bytes|fixed|limit)'
> + ** 'show remote memory-read-packet-size'
> + ** 'show remote memory-write-packet-size'.
> +
> + The configuration of the packet itself, as well as the size of a memory-read
> + or memory-write packet, applies to the currently selected target (if
> + available) and future remote connections. The "show remote" commands print
> + the configuration of the currently selected target. If no remote target is
> + selected, the default configuration for future connections is shown.
> +
> * GDB now supports hardware watchpoints on FreeBSD/Aarch64.
>
> * Remove support for building against Python 2, it is now only possible to
This part of the patch is OK.
> +@cindex Configuration of the remote packet size
Index entries should all begin with a lower-case letter, so that their
sorting (when the Info manual is produced) is not affected by the
current locale.
The documentation parts of the patch are okay with the above not
fixed.
Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] gdb: Add per-remote target variables for memory read and write config
2022-03-29 13:11 ` [PATCH v2 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
2022-03-29 13:48 ` Eli Zaretskii
@ 2022-04-18 14:56 ` Tom Tromey
1 sibling, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2022-04-18 14:56 UTC (permalink / raw)
To: Christina Schimpe via Gdb-patches
>>>>> Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> writes:
> This patch adds per-remote target variables for the configuration of
> memory read- and write packet size. It is a further change to commit
> "gdb: Make global feature array a per-remote target array" to apply the
> fixme notes described in commit 5b6d1e4 "Multi-target support".
Thank you for the patch.
> + const char* target_type = get_target_type_name (remote != nullptr);
The "*" should be after the space.
> + const memory_packet_config* config,
Here too.
> + memory_packet_config* config = &memory_write_packet_config;
And here... there are a few of these.
This ok with these nits fixed, you don't have to re-send the patch.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array
2022-03-29 13:11 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
2022-03-29 13:45 ` Eli Zaretskii
@ 2022-04-18 14:56 ` Tom Tromey
2022-04-18 19:01 ` Pedro Alves
2 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2022-04-18 14:56 UTC (permalink / raw)
To: Christina Schimpe via Gdb-patches
>>>>> Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> writes:
> This patch applies the appropriate FIXME notes described in commit 5b6d1e4
> "Multi-target support".
Thank you for doing this. This looks good to me.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] gdb: Remove workaround for the vCont packet
2022-03-29 13:11 ` [PATCH v2 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
@ 2022-04-18 14:59 ` Tom Tromey
0 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2022-04-18 14:59 UTC (permalink / raw)
To: Christina Schimpe via Gdb-patches
>>>>> Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> writes:
> The workaround for the vCont packet is no longer required due to the
> former commit "gdb: Make global feature array a per-remote target array".
> The vCont packet is now checked once when the connection is started and
> the supported vCont actions are set to the target's remote state
> attribute.
Looks good. Thank you for doing all this.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array
2022-03-29 13:11 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
2022-03-29 13:45 ` Eli Zaretskii
2022-04-18 14:56 ` Tom Tromey
@ 2022-04-18 19:01 ` Pedro Alves
2022-04-20 11:30 ` "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array) Pedro Alves
` (2 more replies)
2 siblings, 3 replies; 20+ messages in thread
From: Pedro Alves @ 2022-04-18 19:01 UTC (permalink / raw)
To: Christina Schimpe, gdb-patches
On 2022-03-29 14:11, Christina Schimpe via Gdb-patches wrote:
> This patch applies the appropriate FIXME notes described in commit 5b6d1e4
> "Multi-target support".
>
> "You'll notice that remote.c includes some FIXME notes. These refer to
> the fact that the global arrays that hold data for the remote packets
> supported are still globals. For example, if we connect to two
> different servers/stubs, then each might support different remote
> protocol features. They might even be different architectures, like
> e.g., one ARM baremetal stub, and a x86 gdbserver, to debug a
> host/controller scenario as a single program. That isn't going to
> work correctly today, because of said globals. I'm leaving fixing
> that for another pass, since it does not appear to be trivial, and I'd
> rather land the base work first. It's already useful to be able to
> debug multiple instances of the same server (e.g., a distributed
> cluster, where you have full control over the servers installed), so I
> think as is it's already reasonable incremental progress."
>
> Using this patch it is possible to configure per-remote targets'
> feature packets.
Thanks for working on this. And so sorry for coming in late! I wish I had gotten
involved in this discussion sooner.
I'm not 100% sure about the solution here.
What is the reasoning for making "set remote foo" affect future
connections in addition to the current connection? I didn't see a discussion about
that, and it seems counterintuitive to me offhand. I would think that:
- if connected, the set command affects the current connection.
- if not connected, the set command affects future connections.
... would be the simplest solution. Thus, if the inferior you have selected is connected
and you want to configure future connections, you'd first drop the connection, or switch
to an inferior that is not connected.
It seems odd that set affects both current and future connections, while show only mentions the
state about the current connection, for instance.
Also, I think it would be better if both the set and the show commands used the
same wording. Currently you have, when not connected:
(gdb) set remote X-packet off
Use of the 'p' packet for future remote targets is set to "off".
(gdb) show remote X-packet
Support for the 'p' packet on newly created remote targets is "disabled".
Note the
"Use of" vs "Support for",
and the
"for future remote targets", vs "on newly created remote targets".
Also note that "disabled" is not accepted by the "set command", while printing
it in quotes suggests that it would. I mean, note:
Current master:
(gdb) show remote X-packet
Support for the `p' packet is currently disabled.
(no quotes around disabled)
vs your patches:
(gdb) show remote X-packet
Support for the 'p' packet on newly created remote targets is "disabled".
and of course:
(gdb) set remote X-packet disabled
"on", "off" or "auto" expected.
Also, BTW, err, why do we talk about the 'p' packet if the command is about
the X packet? That's busted. Seems like a preexisting issue in current master.
> +/* Convert the target type (future remote target or currently connected target)
> + to a name used for gdb printing. */
> +
> +static const char *
> +get_target_type_name (bool target_connected)
> +{
> + if (target_connected)
> + return "on the current remote target";
> + else
> + return "on newly created remote targets";
> +}
Note this is not i18n friendly.
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array)
2022-04-18 19:01 ` Pedro Alves
@ 2022-04-20 11:30 ` Pedro Alves
2022-04-20 11:31 ` Pedro Alves
2022-04-27 13:55 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Schimpe, Christina
2 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2022-04-20 11:30 UTC (permalink / raw)
To: Christina Schimpe, gdb-patches, Andrew Burgess
On 2022-04-18 20:01, Pedro Alves wrote:
>
> (gdb) show remote X-packet
> Support for the 'p' packet on newly created remote targets is "disabled".
>
> and of course:
>
> (gdb) set remote X-packet disabled
> "on", "off" or "auto" expected.
>
>
> Also, BTW, err, why do we talk about the 'p' packet if the command is about
> the X packet? That's busted. Seems like a preexisting issue in current master.
I bisected this, and found out it was a regression introduced by this commit:
8579fd136a614985bd27f20539c7bb7c5a51287d is the first bad commit
commit 8579fd136a614985bd27f20539c7bb7c5a51287d
Author: Andrew Burgess <aburgess@redhat.com>
Date: Mon Nov 8 14:58:46 2021 +0000
gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
This commit is present in the GDB 12 branch and not in GDB 11, so it's a regression there.
^ permalink raw reply [flat|nested] 20+ messages in thread
* "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array)
2022-04-18 19:01 ` Pedro Alves
2022-04-20 11:30 ` "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array) Pedro Alves
@ 2022-04-20 11:31 ` Pedro Alves
2022-04-21 10:25 ` Andrew Burgess
2022-04-27 13:55 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Schimpe, Christina
2 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2022-04-20 11:31 UTC (permalink / raw)
To: Christina Schimpe, gdb-patches, Andrew Burgess
On 2022-04-18 20:01, Pedro Alves wrote:
>
> (gdb) show remote X-packet
> Support for the 'p' packet on newly created remote targets is "disabled".
>
> and of course:
>
> (gdb) set remote X-packet disabled
> "on", "off" or "auto" expected.
>
>
> Also, BTW, err, why do we talk about the 'p' packet if the command is about
> the X packet? That's busted. Seems like a preexisting issue in current master.
I bisected this, and found out it was a regression introduced by this commit:
8579fd136a614985bd27f20539c7bb7c5a51287d is the first bad commit
commit 8579fd136a614985bd27f20539c7bb7c5a51287d
Author: Andrew Burgess <aburgess@redhat.com>
Date: Mon Nov 8 14:58:46 2021 +0000
gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
This commit is present in the GDB 12 branch and not in GDB 11, so it's a regression there.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array)
2022-04-20 11:31 ` Pedro Alves
@ 2022-04-21 10:25 ` Andrew Burgess
2022-04-21 10:31 ` Pedro Alves
0 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2022-04-21 10:25 UTC (permalink / raw)
To: Pedro Alves, Christina Schimpe, gdb-patches
Pedro Alves <pedro@palves.net> writes:
> On 2022-04-18 20:01, Pedro Alves wrote:
>>
>> (gdb) show remote X-packet
>> Support for the 'p' packet on newly created remote targets is "disabled".
>>
>> and of course:
>>
>> (gdb) set remote X-packet disabled
>> "on", "off" or "auto" expected.
>>
>>
>> Also, BTW, err, why do we talk about the 'p' packet if the command is about
>> the X packet? That's busted. Seems like a preexisting issue in current master.
>
> I bisected this, and found out it was a regression introduced by this commit:
>
> 8579fd136a614985bd27f20539c7bb7c5a51287d is the first bad commit
> commit 8579fd136a614985bd27f20539c7bb7c5a51287d
> Author: Andrew Burgess <aburgess@redhat.com>
> Date: Mon Nov 8 14:58:46 2021 +0000
>
> gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr
>
> The motivation is to reduce the number of places where unmanaged
> pointers are returned from allocation type routines. All of the
> callers are updated.
>
> There should be no user visible changes after this commit.
>
>
> This commit is present in the GDB 12 branch and not in GDB 11, so it's a regression there.
Sorry for the regression. I took a look at the code and I honestly have
no idea why I thought the code in the above commit would work :-/
Below is a patch that should fix this issue. I probably should write a
test to go along with it, but thought I'd share this for feedback first.
Let me know what you think,
Thanks,
Andrew
---
commit c4be5cb70bc9b67f7d465eb14a62a0692ed6ad94
Author: Andrew Burgess <aburgess@redhat.com>
Date: Thu Apr 21 11:16:18 2022 +0100
gdb: fix 'remote show FOO-packet' aliases
The following behaviour was observed in GDB:
(gdb) show remote X-packet
Support for the `p' packet is auto-detected, currently unknown.
Note the message mentions the 'p' packet. This is a regression since
this commit:
commit 8579fd136a614985bd27f20539c7bb7c5a51287d
Date: Mon Nov 8 14:58:46 2021 +0000
gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr
Before this commit the behaviour was:
(gdb) show remote X-packet
Support for the `X' packet is auto-detected, currently unknown.
The problem was caused by a failed attempt to ensure that some
allocated strings were deleted when GDB exits. The code in the above
commit attempted to make use of 'static' to solve this problem,
however, the solution was just wrong.
In this new commit I instead allocate a static vector into which all
the allocated strings are stored, this ensures the strings are
released when GDB exits (which makes output from tools like valgrind
cleaner), but each string within the vector can be unique, which fixes
the regression.
diff --git a/gdb/remote.c b/gdb/remote.c
index 75d6bf3919d..562cc586f2b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1968,15 +1968,17 @@ add_packet_config_cmd (struct packet_config *config, const char *name,
/* set/show remote NAME-packet {auto,on,off} -- legacy. */
if (legacy)
{
- /* It's not clear who should take ownership of this string, so, for
- now, make it static, and give copies to each of the add_alias_cmd
- calls below. */
- static gdb::unique_xmalloc_ptr<char> legacy_name
+ /* It's not clear who should take ownership of the LEGACY_NAME string
+ created below, so, for now, place the string into a static vector
+ which ensures the strings is released when GDB exits. */
+ static std::vector<gdb::unique_xmalloc_ptr<char>> legacy_names;
+ gdb::unique_xmalloc_ptr<char> legacy_name
= xstrprintf ("%s-packet", name);
add_alias_cmd (legacy_name.get (), cmds.set, class_obscure, 0,
&remote_set_cmdlist);
add_alias_cmd (legacy_name.get (), cmds.show, class_obscure, 0,
&remote_show_cmdlist);
+ legacy_names.emplace_back (std::move (legacy_name));
}
}
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array)
2022-04-21 10:25 ` Andrew Burgess
@ 2022-04-21 10:31 ` Pedro Alves
2022-04-21 11:01 ` Andrew Burgess
0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2022-04-21 10:31 UTC (permalink / raw)
To: Andrew Burgess, Christina Schimpe, gdb-patches
On 2022-04-21 11:25, Andrew Burgess wrote:
> Pedro Alves <pedro@palves.net> writes:
>>
>> This commit is present in the GDB 12 branch and not in GDB 11, so it's a regression there.
>
> Sorry for the regression. I took a look at the code and I honestly have
> no idea why I thought the code in the above commit would work :-/
>
No worries, happens all the time to me. :-)
> Below is a patch that should fix this issue. I probably should write a
> test to go along with it, but thought I'd share this for feedback first.
>
> Let me know what you think,
LGTM. Thanks for fixing it.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array)
2022-04-21 10:31 ` Pedro Alves
@ 2022-04-21 11:01 ` Andrew Burgess
2022-04-21 16:28 ` Andrew Burgess
0 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2022-04-21 11:01 UTC (permalink / raw)
To: Pedro Alves, Christina Schimpe, gdb-patches
Pedro Alves <pedro@palves.net> writes:
> On 2022-04-21 11:25, Andrew Burgess wrote:
>> Pedro Alves <pedro@palves.net> writes:
>>>
>>> This commit is present in the GDB 12 branch and not in GDB 11, so it's a regression there.
>>
>> Sorry for the regression. I took a look at the code and I honestly have
>> no idea why I thought the code in the above commit would work :-/
>>
>
> No worries, happens all the time to me. :-)
>
>> Below is a patch that should fix this issue. I probably should write a
>> test to go along with it, but thought I'd share this for feedback first.
>>
>> Let me know what you think,
>
> LGTM. Thanks for fixing it.
Thanks, I'll just let a test run complete, and push this later today.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array)
2022-04-21 11:01 ` Andrew Burgess
@ 2022-04-21 16:28 ` Andrew Burgess
2022-04-21 18:20 ` Pedro Alves
0 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2022-04-21 16:28 UTC (permalink / raw)
To: Pedro Alves, Christina Schimpe, gdb-patches
Andrew Burgess <aburgess@redhat.com> writes:
> Pedro Alves <pedro@palves.net> writes:
>
>> On 2022-04-21 11:25, Andrew Burgess wrote:
>>> Pedro Alves <pedro@palves.net> writes:
>>>>
>>>> This commit is present in the GDB 12 branch and not in GDB 11, so it's a regression there.
>>>
>>> Sorry for the regression. I took a look at the code and I honestly have
>>> no idea why I thought the code in the above commit would work :-/
>>>
>>
>> No worries, happens all the time to me. :-)
>>
>>> Below is a patch that should fix this issue. I probably should write a
>>> test to go along with it, but thought I'd share this for feedback first.
>>>
>>> Let me know what you think,
>>
>> LGTM. Thanks for fixing it.
>
> Thanks, I'll just let a test run complete, and push this later today.
I pushed the following patch to master and gdb-12-branch.
Thanks,
Andrew
---
commit 5f21c7aae2040f3463d1ff56a4b0bcdbba34832d
Author: Andrew Burgess <aburgess@redhat.com>
Date: Thu Apr 21 11:16:18 2022 +0100
gdb: fix 'remote show FOO-packet' aliases
The following behaviour was observed in GDB:
(gdb) show remote X-packet
Support for the `p' packet is auto-detected, currently unknown.
Note the message mentions the 'p' packet. This is a regression since
this commit:
commit 8579fd136a614985bd27f20539c7bb7c5a51287d
Date: Mon Nov 8 14:58:46 2021 +0000
gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr
Before this commit the behaviour was:
(gdb) show remote X-packet
Support for the `X' packet is auto-detected, currently unknown.
The problem was caused by a failed attempt to ensure that some
allocated strings were deleted when GDB exits. The code in the above
commit attempted to make use of 'static' to solve this problem,
however, the solution was just wrong.
In this new commit I instead allocate a static vector into which all
the allocated strings are stored, this ensures the strings are
released when GDB exits (which makes output from tools like valgrind
cleaner), but each string within the vector can be unique, which fixes
the regression.
diff --git a/gdb/remote.c b/gdb/remote.c
index 75d6bf3919d..562cc586f2b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1968,15 +1968,17 @@ add_packet_config_cmd (struct packet_config *config, const char *name,
/* set/show remote NAME-packet {auto,on,off} -- legacy. */
if (legacy)
{
- /* It's not clear who should take ownership of this string, so, for
- now, make it static, and give copies to each of the add_alias_cmd
- calls below. */
- static gdb::unique_xmalloc_ptr<char> legacy_name
+ /* It's not clear who should take ownership of the LEGACY_NAME string
+ created below, so, for now, place the string into a static vector
+ which ensures the strings is released when GDB exits. */
+ static std::vector<gdb::unique_xmalloc_ptr<char>> legacy_names;
+ gdb::unique_xmalloc_ptr<char> legacy_name
= xstrprintf ("%s-packet", name);
add_alias_cmd (legacy_name.get (), cmds.set, class_obscure, 0,
&remote_set_cmdlist);
add_alias_cmd (legacy_name.get (), cmds.show, class_obscure, 0,
&remote_show_cmdlist);
+ legacy_names.emplace_back (std::move (legacy_name));
}
}
diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp
index 1f0869433f2..579dcd40e5c 100644
--- a/gdb/testsuite/gdb.base/remote.exp
+++ b/gdb/testsuite/gdb.base/remote.exp
@@ -195,4 +195,9 @@ gdb_test_no_output "set remote hardware-breakpoint-limit -1"
gdb_test_no_output "set remote hardware-watchpoint-limit 2147483647"
gdb_test_no_output "set remote hardware-breakpoint-limit 2147483647"
+# Check the X/P/p alias commands display the correct packet names.
+foreach pkt {X P p} {
+ gdb_test "show remote ${pkt}-packet" "Support for the `${pkt}' packet is.*"
+}
+
gdb_exit
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array)
2022-04-21 16:28 ` Andrew Burgess
@ 2022-04-21 18:20 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2022-04-21 18:20 UTC (permalink / raw)
To: Andrew Burgess, Christina Schimpe, gdb-patches
On 2022-04-21 17:28, Andrew Burgess wrote:
> Andrew Burgess <aburgess@redhat.com> writes:
>
>> Pedro Alves <pedro@palves.net> writes:
>>
>>> On 2022-04-21 11:25, Andrew Burgess wrote:
>>>> Pedro Alves <pedro@palves.net> writes:
>>>>>
>>>>> This commit is present in the GDB 12 branch and not in GDB 11, so it's a regression there.
>>>>
>>>> Sorry for the regression. I took a look at the code and I honestly have
>>>> no idea why I thought the code in the above commit would work :-/
>>>>
>>>
>>> No worries, happens all the time to me. :-)
>>>
>>>> Below is a patch that should fix this issue. I probably should write a
>>>> test to go along with it, but thought I'd share this for feedback first.
>>>>
>>>> Let me know what you think,
>>>
>>> LGTM. Thanks for fixing it.
>>
>> Thanks, I'll just let a test run complete, and push this later today.
>
> I pushed the following patch to master and gdb-12-branch.
Perfect, thanks for adding the testcase.
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array
2022-04-18 19:01 ` Pedro Alves
2022-04-20 11:30 ` "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array) Pedro Alves
2022-04-20 11:31 ` Pedro Alves
@ 2022-04-27 13:55 ` Schimpe, Christina
2022-05-25 14:27 ` Pedro Alves
2 siblings, 1 reply; 20+ messages in thread
From: Schimpe, Christina @ 2022-04-27 13:55 UTC (permalink / raw)
To: Pedro Alves, Andrew Burgess, gdb-patches
Hi Pedro and Andrew,
Thanks a lot for identifying and fixing the regression and thank you for the review.
I added my comments for Pedro's review below. It would be great if you could briefly review
my new suggestion for the logging.
> I'm not 100% sure about the solution here.
>
> What is the reasoning for making "set remote foo" affect future connections
> in addition to the current connection? I didn't see a discussion about that,
> and it seems counterintuitive to me offhand. I would think that:
>
> - if connected, the set command affects the current connection.
>
> - if not connected, the set command affects future connections.
>
> ... would be the simplest solution. Thus, if the inferior you have selected is
> connected and you want to configure future connections, you'd first drop
> the connection, or switch to an inferior that is not connected.
>
I think the reason why I did not investigate this approach further is because these "set remote"
commands applied to future connections before and I did not want to change that.
My initial patch did not add any logging for the set remote commands and the user would not
have noticed that the configuration does not apply to future targets anymore (if connected).
But with the appropriate logging it should be clear and the user should be warned. So I don't
have any preferences anymore and would go ahead to adapt the patch to Pedro's suggestion,
if there are no further arguments against it.
> Also, I think it would be better if both the set and the show commands used
> the same wording. Currently you have, when not connected:
>
> (gdb) set remote X-packet off
> Use of the 'p' packet for future remote targets is set to "off".
> (gdb) show remote X-packet
> Support for the 'p' packet on newly created remote targets is "disabled".
>
> Note the
> "Use of" vs "Support for",
> and the
> "for future remote targets", vs "on newly created remote targets".
>
> Also note that "disabled" is not accepted by the "set command", while
> printing it in quotes suggests that it would. I mean, note:
>
> Current master:
>
> (gdb) show remote X-packet
> Support for the `p' packet is currently disabled.
>
> (no quotes around disabled)
>
> vs your patches:
>
> (gdb) show remote X-packet
> Support for the 'p' packet on newly created remote targets is "disabled".
>
> and of course:
>
> (gdb) set remote X-packet disabled
> "on", "off" or "auto" expected.
>
Yes, you are right.
So I now would suggest the following logging for the packet configuration commands
(based on Pedro's suggestion for the new packet configuration):
~~~
(gdb) set remote kill-packet
"on", "off" or "auto" expected.
(gdb) show remote kill-packet
Support for the 'vKill' packet on future remote targets is "auto", currently unknown.
(gdb) set remote kill-packet off
Support for the 'vKill' packet on future remote targets is set to "off".
(gdb) show remote kill-packet
Support for the 'vKill' packet on future remote targets is "off".
(gdb) target extended-remote :1234
Remote debugging using :1234
(gdb) set remote kill-packet on
Support for the 'vKill' packet on the current remote target is set to "on".
(gdb) show remote kill-packet
Support for the 'vKill' packet on the current remote target is "on".
~~~~
So the only difference between the logging for the show and the set commands, is that
for set we log "is set to" instead of "is".
And for the memory read and write configuration of patch (2/3):
~~~~
(gdb) set remote memory-read-packet-size
Argument required (integer, "fixed" or "limited').
(gdb) show remote memory-read-packet-size
The memory-read-packet-size on future remote targets is 0 (default). The actual limit will be further reduced dependent on the target.
(gdb) set remote memory-read-packet-size fixed
Future targets may not be able to correctly handle a memory-read-packet-size
of 16384 bytes. Change the packet size for future remote targets? (y or n) y
The memory-read-packet-size on future remote targets is set to "fixed".
(gdb) show remote memory-read-packet-size
The memory-read-packet-size on future remote targets is 0 (default). Packets are fixed at 16384 bytes.
(gdb) target extended-remote :1234
Remote debugging using :1234
(gdb) set remote memory-read-packet-size 16300
The memory-read-packet-size on the current remote targets is set to 16300.
(gdb) show remote memory-read-packet-size
The memory-read-packet-size on the current remote target is 16300. Packets are fixed at 16300 bytes.
~~~~
Note that the configuration options before were shown as
~~~~
(gdb) set remote memory-read-packet-size
Argument required (integer, `fixed' or `limited').
~~~~
I also noted a small issue in the configuration for the "limited" option:
~~~~
(gdb) set remote memory-read-packet-size
Argument required (integer, `fixed' or `limited').
(gdb) set remote memory-read-packet-size limited
Invalid memory-read-packet-size (bad syntax).
(gdb) set remote memory-read-packet-size limit
(gdb)
~~~~
So currently you need to specify "limit" although "limited" is suggested. I would adapt it to "limited" in the v3.
>
> > +/* Convert the target type (future remote target or currently connected
> target)
> > + to a name used for gdb printing. */
> > +
> > +static const char *
> > +get_target_type_name (bool target_connected) {
> > + if (target_connected)
> > + return "on the current remote target";
> > + else
> > + return "on newly created remote targets"; }
>
> Note this is not i18n friendly.
>
Ah yes, I will adapt it in the v3, thanks.
Best Regards,
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array
2022-04-27 13:55 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Schimpe, Christina
@ 2022-05-25 14:27 ` Pedro Alves
2022-06-01 10:45 ` Schimpe, Christina
0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2022-05-25 14:27 UTC (permalink / raw)
To: Schimpe, Christina, Andrew Burgess, gdb-patches
Hi Christina,
On 2022-04-27 14:55, Schimpe, Christina wrote:
> Hi Pedro and Andrew,
>
> Thanks a lot for identifying and fixing the regression and thank you for the review.
>
> I added my comments for Pedro's review below. It would be great if you could briefly review
> my new suggestion for the logging.
>
>
>> I'm not 100% sure about the solution here.
>>
>> What is the reasoning for making "set remote foo" affect future connections
>> in addition to the current connection? I didn't see a discussion about that,
>> and it seems counterintuitive to me offhand. I would think that:
>>
>> - if connected, the set command affects the current connection.
>>
>> - if not connected, the set command affects future connections.
>>
>> ... would be the simplest solution. Thus, if the inferior you have selected is
>> connected and you want to configure future connections, you'd first drop
>> the connection, or switch to an inferior that is not connected.
>>
>
> I think the reason why I did not investigate this approach further is because these "set remote"
> commands applied to future connections before and I did not want to change that.
> My initial patch did not add any logging for the set remote commands and the user would not
> have noticed that the configuration does not apply to future targets anymore (if connected).
> But with the appropriate logging it should be clear and the user should be warned. So I don't
> have any preferences anymore and would go ahead to adapt the patch to Pedro's suggestion,
> if there are no further arguments against it.
>
>> Also, I think it would be better if both the set and the show commands used
>> the same wording. Currently you have, when not connected:
>>
>> (gdb) set remote X-packet off
>> Use of the 'p' packet for future remote targets is set to "off".
>> (gdb) show remote X-packet
>> Support for the 'p' packet on newly created remote targets is "disabled".
>>
>> Note the
>> "Use of" vs "Support for",
>> and the
>> "for future remote targets", vs "on newly created remote targets".
>>
>> Also note that "disabled" is not accepted by the "set command", while
>> printing it in quotes suggests that it would. I mean, note:
>>
>> Current master:
>>
>> (gdb) show remote X-packet
>> Support for the `p' packet is currently disabled.
>>
>> (no quotes around disabled)
>>
>> vs your patches:
>>
>> (gdb) show remote X-packet
>> Support for the 'p' packet on newly created remote targets is "disabled".
>>
>> and of course:
>>
>> (gdb) set remote X-packet disabled
>> "on", "off" or "auto" expected.
>>
>
> Yes, you are right.
>
> So I now would suggest the following logging for the packet configuration commands
> (based on Pedro's suggestion for the new packet configuration):
>
> ~~~
> (gdb) set remote kill-packet
> "on", "off" or "auto" expected.
> (gdb) show remote kill-packet
> Support for the 'vKill' packet on future remote targets is "auto", currently unknown.
> (gdb) set remote kill-packet off
> Support for the 'vKill' packet on future remote targets is set to "off".
> (gdb) show remote kill-packet
> Support for the 'vKill' packet on future remote targets is "off".
> (gdb) target extended-remote :1234
> Remote debugging using :1234
> (gdb) set remote kill-packet on
> Support for the 'vKill' packet on the current remote target is set to "on".
> (gdb) show remote kill-packet
> Support for the 'vKill' packet on the current remote target is "on".
> ~~~~
> So the only difference between the logging for the show and the set commands, is that
> for set we log "is set to" instead of "is".
>
Seems fine.
>
> And for the memory read and write configuration of patch (2/3):
>
> ~~~~
> (gdb) set remote memory-read-packet-size
> Argument required (integer, "fixed" or "limited').
> (gdb) show remote memory-read-packet-size
> The memory-read-packet-size on future remote targets is 0 (default). The actual limit will be further reduced dependent on the target.
> (gdb) set remote memory-read-packet-size fixed
> Future targets may not be able to correctly handle a memory-read-packet-size
You say "future remote targets" in the other messages, so I guess here you'd
say "Future remote targets" instead of "Future targets" too.
> of 16384 bytes. Change the packet size for future remote targets? (y or n) y
> The memory-read-packet-size on future remote targets is set to "fixed".
> (gdb) show remote memory-read-packet-size
> The memory-read-packet-size on future remote targets is 0 (default). Packets are fixed at 16384 bytes.
> (gdb) target extended-remote :1234
> Remote debugging using :1234
> (gdb) set remote memory-read-packet-size 16300
> The memory-read-packet-size on the current remote targets is set to 16300.
"remote targets" -> "remote target" (singular).
> (gdb) show remote memory-read-packet-size
> The memory-read-packet-size on the current remote target is 16300. Packets are fixed at 16300 bytes.
> ~~~~
>
Seems all fine to me.
> Note that the configuration options before were shown as
> ~~~~
> (gdb) set remote memory-read-packet-size
> Argument required (integer, `fixed' or `limited').
> ~~~~
>
> I also noted a small issue in the configuration for the "limited" option:
> ~~~~
> (gdb) set remote memory-read-packet-size
> Argument required (integer, `fixed' or `limited').
> (gdb) set remote memory-read-packet-size limited
> Invalid memory-read-packet-size (bad syntax).
> (gdb) set remote memory-read-packet-size limit
> (gdb)
> ~~~~
> So currently you need to specify "limit" although "limited" is suggested. I would adapt it to "limited" in the v3.
Sounds like that may break someone's scripts? I'd go for tweaking the suggestion to
say "limit" instead.
I went to look what does the manual say, but I wasn't able to find where this comment
is documented in the manual... Odd.
The online help does say "limit", though:
(gdb) help set remote memory-read-packet-size
Set the maximum number of bytes per memory-read packet.
Specify the number of bytes in a packet or 0 (zero) for the
default packet size. The actual limit is further reduced
dependent on the target. Specify ``fixed'' to disable the
further restriction and ``limit'' to enable that restriction.
(gdb)
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array
2022-05-25 14:27 ` Pedro Alves
@ 2022-06-01 10:45 ` Schimpe, Christina
0 siblings, 0 replies; 20+ messages in thread
From: Schimpe, Christina @ 2022-06-01 10:45 UTC (permalink / raw)
To: Pedro Alves, Andrew Burgess, gdb-patches
Hi Pedro,
Thanks for the review.
I will go ahead now and work on a v3 of this series with the new suggestion for the
"set remote foo" packet configuration and the logging suggested in the email thread
below.
Best Regards,
Christina
> -----Original Message-----
> From: Pedro Alves <pedro@palves.net>
> Sent: Wednesday, May 25, 2022 4:27 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; Andrew Burgess
> <aburgess@redhat.com>; gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target
> array
>
> Hi Christina,
>
> On 2022-04-27 14:55, Schimpe, Christina wrote:
> > Hi Pedro and Andrew,
> >
> > Thanks a lot for identifying and fixing the regression and thank you for the
> review.
> >
> > I added my comments for Pedro's review below. It would be great if
> > you could briefly review my new suggestion for the logging.
> >
> >
> >> I'm not 100% sure about the solution here.
> >>
> >> What is the reasoning for making "set remote foo" affect future
> >> connections in addition to the current connection? I didn't see a
> >> discussion about that, and it seems counterintuitive to me offhand. I would
> think that:
> >>
> >> - if connected, the set command affects the current connection.
> >>
> >> - if not connected, the set command affects future connections.
> >>
> >> ... would be the simplest solution. Thus, if the inferior you have
> >> selected is connected and you want to configure future connections,
> >> you'd first drop the connection, or switch to an inferior that is not connected.
> >>
> >
> > I think the reason why I did not investigate this approach further is because
> these "set remote"
> > commands applied to future connections before and I did not want to change
> that.
> > My initial patch did not add any logging for the set remote commands
> > and the user would not have noticed that the configuration does not apply to
> future targets anymore (if connected).
> > But with the appropriate logging it should be clear and the user
> > should be warned. So I don't have any preferences anymore and would
> > go ahead to adapt the patch to Pedro's suggestion, if there are no further
> arguments against it.
> >
> >> Also, I think it would be better if both the set and the show
> >> commands used the same wording. Currently you have, when not
> connected:
> >>
> >> (gdb) set remote X-packet off
> >> Use of the 'p' packet for future remote targets is set to "off".
> >> (gdb) show remote X-packet
> >> Support for the 'p' packet on newly created remote targets is "disabled".
> >>
> >> Note the
> >> "Use of" vs "Support for",
> >> and the
> >> "for future remote targets", vs "on newly created remote targets".
> >>
> >> Also note that "disabled" is not accepted by the "set command", while
> >> printing it in quotes suggests that it would. I mean, note:
> >>
> >> Current master:
> >>
> >> (gdb) show remote X-packet
> >> Support for the `p' packet is currently disabled.
> >>
> >> (no quotes around disabled)
> >>
> >> vs your patches:
> >>
> >> (gdb) show remote X-packet
> >> Support for the 'p' packet on newly created remote targets is "disabled".
> >>
> >> and of course:
> >>
> >> (gdb) set remote X-packet disabled
> >> "on", "off" or "auto" expected.
> >>
> >
> > Yes, you are right.
> >
> > So I now would suggest the following logging for the packet
> > configuration commands (based on Pedro's suggestion for the new packet
> configuration):
> >
> > ~~~
> > (gdb) set remote kill-packet
> > "on", "off" or "auto" expected.
> > (gdb) show remote kill-packet
> > Support for the 'vKill' packet on future remote targets is "auto", currently
> unknown.
> > (gdb) set remote kill-packet off
> > Support for the 'vKill' packet on future remote targets is set to "off".
> > (gdb) show remote kill-packet
> > Support for the 'vKill' packet on future remote targets is "off".
> > (gdb) target extended-remote :1234
> > Remote debugging using :1234
> > (gdb) set remote kill-packet on
> > Support for the 'vKill' packet on the current remote target is set to "on".
> > (gdb) show remote kill-packet
> > Support for the 'vKill' packet on the current remote target is "on".
> > ~~~~
> > So the only difference between the logging for the show and the set
> > commands, is that for set we log "is set to" instead of "is".
> >
>
> Seems fine.
>
> >
> > And for the memory read and write configuration of patch (2/3):
> >
> > ~~~~
> > (gdb) set remote memory-read-packet-size Argument required (integer,
> > "fixed" or "limited').
> > (gdb) show remote memory-read-packet-size The memory-read-packet-size
> > on future remote targets is 0 (default). The actual limit will be further reduced
> dependent on the target.
> > (gdb) set remote memory-read-packet-size fixed Future targets may not
> > be able to correctly handle a memory-read-packet-size
>
> You say "future remote targets" in the other messages, so I guess here you'd say
> "Future remote targets" instead of "Future targets" too.
>
> > of 16384 bytes. Change the packet size for future remote targets? (y
> > or n) y The memory-read-packet-size on future remote targets is set to "fixed".
> > (gdb) show remote memory-read-packet-size The memory-read-packet-size
> > on future remote targets is 0 (default). Packets are fixed at 16384 bytes.
> > (gdb) target extended-remote :1234
> > Remote debugging using :1234
> > (gdb) set remote memory-read-packet-size 16300 The
> > memory-read-packet-size on the current remote targets is set to 16300.
>
> "remote targets" -> "remote target" (singular).
>
> > (gdb) show remote memory-read-packet-size The memory-read-packet-size
> > on the current remote target is 16300. Packets are fixed at 16300 bytes.
> > ~~~~
> >
>
> Seems all fine to me.
>
> > Note that the configuration options before were shown as ~~~~
> > (gdb) set remote memory-read-packet-size Argument required (integer,
> > `fixed' or `limited').
> > ~~~~
> >
> > I also noted a small issue in the configuration for the "limited" option:
> > ~~~~
> > (gdb) set remote memory-read-packet-size Argument required (integer,
> > `fixed' or `limited').
> > (gdb) set remote memory-read-packet-size limited Invalid
> > memory-read-packet-size (bad syntax).
> > (gdb) set remote memory-read-packet-size limit
> > (gdb)
> > ~~~~
> > So currently you need to specify "limit" although "limited" is suggested. I
> would adapt it to "limited" in the v3.
>
> Sounds like that may break someone's scripts? I'd go for tweaking the
> suggestion to say "limit" instead.
>
> I went to look what does the manual say, but I wasn't able to find where this
> comment is documented in the manual... Odd.
>
> The online help does say "limit", though:
>
> (gdb) help set remote memory-read-packet-size Set the maximum number of
> bytes per memory-read packet.
> Specify the number of bytes in a packet or 0 (zero) for the default packet size.
> The actual limit is further reduced dependent on the target. Specify ``fixed'' to
> disable the further restriction and ``limit'' to enable that restriction.
> (gdb)
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-06-01 10:45 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-29 13:11 [PATCH v2 0/3] Apply fixme notes for multi-target support Christina Schimpe
2022-03-29 13:11 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
2022-03-29 13:45 ` Eli Zaretskii
2022-04-18 14:56 ` Tom Tromey
2022-04-18 19:01 ` Pedro Alves
2022-04-20 11:30 ` "show remote foo-packet" regression (Re: [PATCH v2 1/3] gdb: Make global feature array a per-remote target array) Pedro Alves
2022-04-20 11:31 ` Pedro Alves
2022-04-21 10:25 ` Andrew Burgess
2022-04-21 10:31 ` Pedro Alves
2022-04-21 11:01 ` Andrew Burgess
2022-04-21 16:28 ` Andrew Burgess
2022-04-21 18:20 ` Pedro Alves
2022-04-27 13:55 ` [PATCH v2 1/3] gdb: Make global feature array a per-remote target array Schimpe, Christina
2022-05-25 14:27 ` Pedro Alves
2022-06-01 10:45 ` Schimpe, Christina
2022-03-29 13:11 ` [PATCH v2 2/3] gdb: Add per-remote target variables for memory read and write config Christina Schimpe
2022-03-29 13:48 ` Eli Zaretskii
2022-04-18 14:56 ` Tom Tromey
2022-03-29 13:11 ` [PATCH v2 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
2022-04-18 14:59 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).