* [PATCH 2/2] gdbserver: Fix qSupported:xmlRegisters=i386;UnknownFeature+ handling
2015-11-04 16:48 [PATCH 0/2] Bad qSupported:xmlRegisters=i386;UnknownFeature+ handling Pedro Alves
@ 2015-11-04 16:48 ` Pedro Alves
2015-11-04 16:48 ` [PATCH 1/2] gdb: Workaround bad gdbserver qSupported:xmlRegisters=i386;UnknwnFeat+ handling Pedro Alves
2015-11-19 18:36 ` [PATCH 0/2] Bad qSupported:xmlRegisters=i386;UnknownFeature+ handling Pedro Alves
2 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2015-11-04 16:48 UTC (permalink / raw)
To: gdb-patches
The target_process_qsupported method is called for each qSupported
feature that the common code does not recognize. The only current
implementation, for x86 Linux (x86_linux_process_qsupported), assumes
that it either is called with the "xmlRegisters=i386" feature, or that
it is isn't called at all, indicating the connected GDB predates x86
XML descriptions.
That's a bad assumption however. If GDB sends in a new/unknown (to
core gdbserver) feature after "xmlRegisters=i386", say, something like
qSupported:xmlRegisters=i386;UnknownFeature+, then when
target_process_qsupported is called for "UnknownFeature+",
x86_linux_process_qsupported clears the 'use_xml' global and calls
x86_linux_update_xmltarget, and gdbserver ends up _not_ reporting a
XML description...
This commit changes the target_process_qsupported API to instead pass
down a vector of unprocessed qSupported features in one go.
(There's an early call to target_process_qsupported(NULL) that
indicates "starting qSupported processing". There's no matching call
to mark the end of processing, though. I first fixed this by passing
(char *)-1 to indicate that, and adjusted the x86 backend to only
clear 'use_xml' when qSupported processing starts, and then only call
x86_linux_update_xmltarget() when (char *)-1 was passed. However, I
wasn't that happy with the hack and came up this alternative version.)
gdb/gdbserver/ChangeLog:
2015-11-04 Pedro Alves <palves@redhat.com>
* linux-low.c (linux_process_qsupported): Change prototype.
Adjust.
* linux-low.h (struct linux_target_ops) <process_qsupported>:
Change prototype.
* linux-x86-low.c (x86_linux_process_qsupported): Change prototype
and adjust to loop over all features.
* server.c (handle_query) <qSupported>: Adjust to call
target_process_qsupported once, passing it a vector of unprocessed
features.
* target.h (struct target_ops) <process_qsupported>: Change
prototype.
(target_process_qsupported): Adjust.
---
gdb/gdbserver/linux-low.c | 4 ++--
gdb/gdbserver/linux-low.h | 2 +-
gdb/gdbserver/linux-x86-low.c | 28 +++++++++++++++++-----------
gdb/gdbserver/server.c | 19 +++++++++++++------
gdb/gdbserver/target.h | 9 +++++----
5 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 41ab510..e3a56a7 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -6128,10 +6128,10 @@ linux_read_loadmap (const char *annex, CORE_ADDR offset,
#endif /* defined PT_GETDSBT || defined PTRACE_GETFDPIC */
static void
-linux_process_qsupported (const char *query)
+linux_process_qsupported (char **features, int count)
{
if (the_low_target.process_qsupported != NULL)
- the_low_target.process_qsupported (query);
+ the_low_target.process_qsupported (features, count);
}
static int
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index ccf4c94..f1d4f0f 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -199,7 +199,7 @@ struct linux_target_ops
void (*prepare_to_resume) (struct lwp_info *);
/* Hook to support target specific qSupported. */
- void (*process_qsupported) (const char *);
+ void (*process_qsupported) (char **, int count);
/* Returns true if the low target supports tracepoints. */
int (*supports_tracepoints) (void);
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 89ec4e5..7f07194 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -1356,29 +1356,35 @@ x86_linux_update_xmltarget (void)
PTRACE_GETREGSET. */
static void
-x86_linux_process_qsupported (const char *query)
+x86_linux_process_qsupported (char **features, int count)
{
+ int i;
+
/* Return if gdb doesn't support XML. If gdb sends "xmlRegisters="
with "i386" in qSupported query, it supports x86 XML target
descriptions. */
use_xml = 0;
- if (query != NULL && startswith (query, "xmlRegisters="))
+ for (i = 0; i < count; i++)
{
- char *copy = xstrdup (query + 13);
- char *p;
+ const char *feature = features[i];
- for (p = strtok (copy, ","); p != NULL; p = strtok (NULL, ","))
+ if (startswith (feature, "xmlRegisters="))
{
- if (strcmp (p, "i386") == 0)
+ char *copy = xstrdup (feature + 13);
+ char *p;
+
+ for (p = strtok (copy, ","); p != NULL; p = strtok (NULL, ","))
{
- use_xml = 1;
- break;
+ if (strcmp (p, "i386") == 0)
+ {
+ use_xml = 1;
+ break;
+ }
}
- }
- free (copy);
+ free (copy);
+ }
}
-
x86_linux_update_xmltarget ();
}
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index fd2804b..7d6c9cc 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2054,9 +2054,6 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
char *p = &own_buf[10];
int gdb_supports_qRelocInsn = 0;
- /* Start processing qSupported packet. */
- target_process_qsupported (NULL);
-
/* Process each feature being provided by GDB. The first
feature will follow a ':', and latter features will follow
';'. */
@@ -2064,6 +2061,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
{
char **qsupported = NULL;
int count = 0;
+ int unknown = 0;
int i;
/* Two passes, to avoid nested strtok calls in
@@ -2128,11 +2126,20 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
else if (strcmp (p, "vContSupported+") == 0)
vCont_supported = 1;
else
- target_process_qsupported (p);
-
- free (p);
+ {
+ /* Move the unknown features all together. */
+ qsupported[i] = NULL;
+ qsupported[unknown] = p;
+ unknown++;
+ }
}
+ /* Give the target backend a chance to process the unknown
+ features. */
+ target_process_qsupported (qsupported, unknown);
+
+ for (i = 0; i < count; i++)
+ free (qsupported[i]);
free (qsupported);
}
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 769c876..6fa454d 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -306,8 +306,9 @@ struct target_ops
int (*read_loadmap) (const char *annex, CORE_ADDR offset,
unsigned char *myaddr, unsigned int len);
- /* Target specific qSupported support. */
- void (*process_qsupported) (const char *);
+ /* Target specific qSupported support. FEATURES is an array of
+ features with COUNT elements. */
+ void (*process_qsupported) (char **features, int count);
/* Return 1 if the target supports tracepoints, 0 (or leave the
callback NULL) otherwise. */
@@ -519,11 +520,11 @@ int kill_inferior (int);
(the_target->supports_multi_process ? \
(*the_target->supports_multi_process) () : 0)
-#define target_process_qsupported(query) \
+#define target_process_qsupported(features, count) \
do \
{ \
if (the_target->process_qsupported) \
- the_target->process_qsupported (query); \
+ the_target->process_qsupported (features, count); \
} while (0)
#define target_supports_tracepoints() \
--
1.9.3
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 1/2] gdb: Workaround bad gdbserver qSupported:xmlRegisters=i386;UnknwnFeat+ handling
2015-11-04 16:48 [PATCH 0/2] Bad qSupported:xmlRegisters=i386;UnknownFeature+ handling Pedro Alves
2015-11-04 16:48 ` [PATCH 2/2] gdbserver: Fix " Pedro Alves
@ 2015-11-04 16:48 ` Pedro Alves
2015-11-19 18:36 ` [PATCH 0/2] Bad qSupported:xmlRegisters=i386;UnknownFeature+ handling Pedro Alves
2 siblings, 0 replies; 4+ messages in thread
From: Pedro Alves @ 2015-11-04 16:48 UTC (permalink / raw)
To: gdb-patches
gdbserver's target_process_qsupported is called for each feature that
the gdbserver common code does not recognize. The only current
implementation, for x86 Linux, does this:
static void
x86_linux_process_qsupported (const char *query)
{
/* Return if gdb doesn't support XML. If gdb sends "xmlRegisters="
with "i386" in qSupported query, it supports x86 XML target
descriptions. */
use_xml = 0;
if (query != NULL && startswith (query, "xmlRegisters="))
{
char *copy = xstrdup (query + 13);
char *p;
for (p = strtok (copy, ","); p != NULL; p = strtok (NULL, ","))
{
if (strcmp (p, "i386") == 0)
{
use_xml = 1;
break;
}
}
free (copy);
}
x86_linux_update_xmltarget ();
}
Notice that this clears use_xml and calls x86_linux_update_xmltarget
each time target_process_qsupported is called. So if gdb sends in any
unknown feature after "xmlRegisters=i386", like e.g.,
"xmlRegisters=i386;UnknownFeature+" gdbserver ends up not reporting a
XML description...
Work around this by having GDB send the "xmlRegisters=" feature last.
gdb/ChangeLog:
2015-11-04 Pedro Alves <palves@redhat.com>
* remote.c (remote_query_supported): Send the "xmlRegisters="
last.
---
gdb/remote.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index fed397a..593bf63 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4467,9 +4467,6 @@ remote_query_supported (void)
if (packet_set_cmd_state (PACKET_hwbreak_feature) != AUTO_BOOLEAN_FALSE)
q = remote_query_supported_append (q, "hwbreak+");
- if (remote_support_xml)
- q = remote_query_supported_append (q, remote_support_xml);
-
q = remote_query_supported_append (q, "qRelocInsn+");
if (rs->extended)
@@ -4488,6 +4485,11 @@ remote_query_supported (void)
if (packet_set_cmd_state (PACKET_vContSupported) != AUTO_BOOLEAN_FALSE)
q = remote_query_supported_append (q, "vContSupported+");
+ /* Keep this one last to work around a gdbserver <= 7.10 bug in
+ the qSupported:xmlRegisters=i386 handling. */
+ if (remote_support_xml != NULL)
+ q = remote_query_supported_append (q, remote_support_xml);
+
q = reconcat (q, "qSupported:", q, (char *) NULL);
putpkt (q);
--
1.9.3
^ permalink raw reply [flat|nested] 4+ messages in thread