From 06e03fff313dfcc5f344280f8ac70b0ec8521f3a Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 19 Nov 2015 18:31:50 +0000 Subject: [PATCH] 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 * linux-low.c (linux_process_qsupported): Change prototype. Adjust. * linux-low.h (struct linux_target_ops) : Change prototype. * linux-x86-low.c (x86_linux_process_qsupported): Change prototype and adjust to loop over all features. * server.c (handle_query) : Adjust to call target_process_qsupported once, passing it a vector of unprocessed features. * target.h (struct target_ops) : Change prototype. (target_process_qsupported): Adjust. --- gdb/gdbserver/ChangeLog | 15 +++++++++++++++ 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 +++++---- 6 files changed, 53 insertions(+), 24 deletions(-) diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index c515249137..0d2963a6bf 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,18 @@ +2015-11-19 Pedro Alves + + * linux-low.c (linux_process_qsupported): Change prototype. + Adjust. + * linux-low.h (struct linux_target_ops) : + Change prototype. + * linux-x86-low.c (x86_linux_process_qsupported): Change prototype + and adjust to loop over all features. + * server.c (handle_query) : Adjust to call + target_process_qsupported once, passing it a vector of unprocessed + features. + * target.h (struct target_ops) : Change + prototype. + (target_process_qsupported): Adjust. + 2015-11-19 Pedro Alves * configure.ac (ERROR_ON_WARNING): Don't check whether in C++ diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 41ab510fa4..e3a56a7c16 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 ccf4c9421d..f1d4f0f5a4 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 89ec4e54b3..7f0719401a 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 fd2804beaa..7d6c9cc47a 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 769c876ede..6fa454dfb4 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() \ -- 2.34.1