From patchwork Wed Nov 4 16:48:15 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 9555 Received: (qmail 66771 invoked by alias); 4 Nov 2015 16:48:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 66609 invoked by uid 89); 4 Nov 2015 16:48:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 04 Nov 2015 16:48:19 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 92FD58BA62 for ; Wed, 4 Nov 2015 16:48:18 +0000 (UTC) Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tA4GmFwR014651 for ; Wed, 4 Nov 2015 11:48:17 -0500 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 2/2] gdbserver: Fix qSupported:xmlRegisters=i386; UnknownFeature+ handling Date: Wed, 4 Nov 2015 16:48:15 +0000 Message-Id: <1446655695-27983-3-git-send-email-palves@redhat.com> In-Reply-To: <1446655695-27983-1-git-send-email-palves@redhat.com> References: <1446655695-27983-1-git-send-email-palves@redhat.com> 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 * 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/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() \