gdbserver: Fix qSupported:xmlRegisters=i386;UnknownFeature+ handling

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-19  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.
This commit is contained in:
Pedro Alves 2015-11-19 18:31:50 +00:00
parent b35d5edb03
commit 06e03fff31
6 changed files with 53 additions and 24 deletions

View file

@ -1,3 +1,18 @@
2015-11-19 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.
2015-11-19 Pedro Alves <palves@redhat.com> 2015-11-19 Pedro Alves <palves@redhat.com>
* configure.ac (ERROR_ON_WARNING): Don't check whether in C++ * configure.ac (ERROR_ON_WARNING): Don't check whether in C++

View file

@ -6128,10 +6128,10 @@ linux_read_loadmap (const char *annex, CORE_ADDR offset,
#endif /* defined PT_GETDSBT || defined PTRACE_GETFDPIC */ #endif /* defined PT_GETDSBT || defined PTRACE_GETFDPIC */
static void static void
linux_process_qsupported (const char *query) linux_process_qsupported (char **features, int count)
{ {
if (the_low_target.process_qsupported != NULL) if (the_low_target.process_qsupported != NULL)
the_low_target.process_qsupported (query); the_low_target.process_qsupported (features, count);
} }
static int static int

View file

@ -199,7 +199,7 @@ struct linux_target_ops
void (*prepare_to_resume) (struct lwp_info *); void (*prepare_to_resume) (struct lwp_info *);
/* Hook to support target specific qSupported. */ /* 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. */ /* Returns true if the low target supports tracepoints. */
int (*supports_tracepoints) (void); int (*supports_tracepoints) (void);

View file

@ -1356,29 +1356,35 @@ x86_linux_update_xmltarget (void)
PTRACE_GETREGSET. */ PTRACE_GETREGSET. */
static void 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=" /* Return if gdb doesn't support XML. If gdb sends "xmlRegisters="
with "i386" in qSupported query, it supports x86 XML target with "i386" in qSupported query, it supports x86 XML target
descriptions. */ descriptions. */
use_xml = 0; use_xml = 0;
if (query != NULL && startswith (query, "xmlRegisters=")) for (i = 0; i < count; i++)
{ {
char *copy = xstrdup (query + 13); const char *feature = features[i];
char *p;
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; if (strcmp (p, "i386") == 0)
break; {
use_xml = 1;
break;
}
} }
}
free (copy); free (copy);
}
} }
x86_linux_update_xmltarget (); x86_linux_update_xmltarget ();
} }

View file

@ -2054,9 +2054,6 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
char *p = &own_buf[10]; char *p = &own_buf[10];
int gdb_supports_qRelocInsn = 0; int gdb_supports_qRelocInsn = 0;
/* Start processing qSupported packet. */
target_process_qsupported (NULL);
/* Process each feature being provided by GDB. The first /* Process each feature being provided by GDB. The first
feature will follow a ':', and latter features will follow 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; char **qsupported = NULL;
int count = 0; int count = 0;
int unknown = 0;
int i; int i;
/* Two passes, to avoid nested strtok calls in /* 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) else if (strcmp (p, "vContSupported+") == 0)
vCont_supported = 1; vCont_supported = 1;
else else
target_process_qsupported (p); {
/* Move the unknown features all together. */
free (p); 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); free (qsupported);
} }

View file

@ -306,8 +306,9 @@ struct target_ops
int (*read_loadmap) (const char *annex, CORE_ADDR offset, int (*read_loadmap) (const char *annex, CORE_ADDR offset,
unsigned char *myaddr, unsigned int len); unsigned char *myaddr, unsigned int len);
/* Target specific qSupported support. */ /* Target specific qSupported support. FEATURES is an array of
void (*process_qsupported) (const char *); features with COUNT elements. */
void (*process_qsupported) (char **features, int count);
/* Return 1 if the target supports tracepoints, 0 (or leave the /* Return 1 if the target supports tracepoints, 0 (or leave the
callback NULL) otherwise. */ callback NULL) otherwise. */
@ -519,11 +520,11 @@ int kill_inferior (int);
(the_target->supports_multi_process ? \ (the_target->supports_multi_process ? \
(*the_target->supports_multi_process) () : 0) (*the_target->supports_multi_process) () : 0)
#define target_process_qsupported(query) \ #define target_process_qsupported(features, count) \
do \ do \
{ \ { \
if (the_target->process_qsupported) \ if (the_target->process_qsupported) \
the_target->process_qsupported (query); \ the_target->process_qsupported (features, count); \
} while (0) } while (0)
#define target_supports_tracepoints() \ #define target_supports_tracepoints() \