From patchwork Mon Feb 26 13:08:01 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Metzger, Markus T" X-Patchwork-Id: 26056 Received: (qmail 9234 invoked by alias); 26 Feb 2018 13:08:08 -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 9212 invoked by uid 89); 26 Feb 2018 13:08:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=1.2.3.4, Protocol X-HELO: mga04.intel.com Received: from mga04.intel.com (HELO mga04.intel.com) (192.55.52.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Feb 2018 13:08:05 +0000 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Feb 2018 05:08:04 -0800 X-ExtLoop1: 1 Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by orsmga002.jf.intel.com with ESMTP; 26 Feb 2018 05:08:03 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.101]) by IRSMSX101.ger.corp.intel.com ([169.254.1.188]) with mapi id 14.03.0319.002; Mon, 26 Feb 2018 13:08:02 +0000 From: "Metzger, Markus T" To: "Maciej W. Rozycki" CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH v2 5/7] btrace, gdbserver: remove the to_supports_btrace target method Date: Mon, 26 Feb 2018 13:08:01 +0000 Message-ID: References: <1516976072-19282-1-git-send-email-markus.t.metzger@intel.com> <1516976072-19282-6-git-send-email-markus.t.metzger@intel.com> In-Reply-To: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjg2ODIwN2YtMzU2MS00OTA0LTkxYjktNGM0Yzc5YjFiNjdkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJaVkdvVVRVR3I0N0F2dWpLdzlRVHlXVXAxbURmU2IwRGZCNXlHUU1Rb0xNR2RLOSttYURqa2hLd2FIWU5QNmcyIn0= dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action MIME-Version: 1.0 X-IsSubscribed: yes Hello Maciej, > Process .../gdb/testsuite/outputs/gdb.base/advance/advance created; pid = > 25519 Listening on port 2346 target remote 1.2.3.4:2346 Remote debugging using > 1.2.3.4:2346 Reading symbols from .../lib/ld.so.1...done. > 0x77fc8de0 in __start () from .../lib/ld.so.1 Protocol error: qXfer:btrace-conf > (read-btrace-conf) conflicting enabled responses. > (gdb) continue > The program is not being run. > (gdb) FAIL: gdb.base/advance.exp: can't run to main > > See the attached RSP packet exchange log for details. Please investigate. Below is a patch to address this. I tested it on IA by undefining HAVE_LINUX_BTRACE. Does it fix the issue you reported? thanks, Markus. --- commit c4d017399e2cc62cfed793bb93967bd6b3d573bb Author: Markus Metzger Date: Mon Feb 26 11:59:43 2018 +0100 btrace, gdbserver: check btrace target pointers By removing the supports_btrace gdbserver target method we relied on GDB trying to enable branch tracing and failing on the attempt. For targets that do not provide the btrace methods, however, an initial request from GDB for the branch trace configuration to detect whether gdbserver is already recording resulted in a protocol error. Have the btrace target methods throw a "Target does not suppor branch tracing" error and be prepared to handle exceptions in all functions that call btrace target methods. We therefore turn the target_* macros into static inline functions. Also remove the additional btrace target method checks that resulted in the above protocol error. Thanks to Maciej W. Rozycki for reporting this. Signed-off-by: Markus Metzger gdbserver/ * target.h (target_enable_btrace, target_disable_btrace, target_read_btrace, target_read_btrace_conf): Turn macro into inline function. Throw error if target method not defined. * server.c (handle_qxfer_btrace, handle_qxfer_btrace_conf): Remove check for btrace target method. Be prepared to handle exceptions from btrace target methods. Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Christian Lamprechter Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index cb02b58..4fd274f 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -1818,7 +1818,7 @@ handle_qxfer_btrace (const char *annex, enum btrace_read_type type; int result; - if (the_target->read_btrace == NULL || writebuf != NULL) + if (writebuf != NULL) return -2; if (ptid_equal (general_thread, null_ptid) @@ -1857,12 +1857,21 @@ handle_qxfer_btrace (const char *annex, { buffer_free (&cache); - result = target_read_btrace (thread->btrace, &cache, type); - if (result != 0) + TRY { - memcpy (own_buf, cache.buffer, cache.used_size); - return -3; + result = target_read_btrace (thread->btrace, &cache, type); + if (result != 0) + memcpy (own_buf, cache.buffer, cache.used_size); } + CATCH (exception, RETURN_MASK_ERROR) + { + sprintf (own_buf, "E.%s", exception.message); + result = -1; + } + END_CATCH + + if (result != 0) + return -3; } else if (offset > cache.used_size) { @@ -1889,7 +1898,7 @@ handle_qxfer_btrace_conf (const char *annex, struct thread_info *thread; int result; - if (the_target->read_btrace_conf == NULL || writebuf != NULL) + if (writebuf != NULL) return -2; if (annex[0] != '\0') @@ -1919,12 +1928,21 @@ handle_qxfer_btrace_conf (const char *annex, { buffer_free (&cache); - result = target_read_btrace_conf (thread->btrace, &cache); - if (result != 0) + TRY { - memcpy (own_buf, cache.buffer, cache.used_size); - return -3; + result = target_read_btrace_conf (thread->btrace, &cache); + if (result != 0) + memcpy (own_buf, cache.buffer, cache.used_size); } + CATCH (exception, RETURN_MASK_ERROR) + { + sprintf (own_buf, "E.%s", exception.message); + result = -1; + } + END_CATCH + + if (result != 0) + return -3; } else if (offset > cache.used_size) { diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h index dcefe1a..923a63b 100644 --- a/gdb/gdbserver/target.h +++ b/gdb/gdbserver/target.h @@ -620,17 +620,42 @@ int kill_inferior (int); (the_target->supports_agent ? \ (*the_target->supports_agent) () : 0) -#define target_enable_btrace(ptid, conf) \ - (*the_target->enable_btrace) (ptid, conf) +static inline struct btrace_target_info * +target_enable_btrace(ptid_t ptid, const struct btrace_config *conf) +{ + if (the_target->enable_btrace == nullptr) + error ("Target does not support branch tracing."); + + return (*the_target->enable_btrace) (ptid, conf); +} + +static inline int +target_disable_btrace(struct btrace_target_info *tinfo) +{ + if (the_target->disable_btrace == nullptr) + error ("Target does not support branch tracing."); -#define target_disable_btrace(tinfo) \ - (*the_target->disable_btrace) (tinfo) + return (*the_target->disable_btrace) (tinfo); +} -#define target_read_btrace(tinfo, buffer, type) \ - (*the_target->read_btrace) (tinfo, buffer, type) +static inline int +target_read_btrace(struct btrace_target_info *tinfo, struct buffer *buffer, + enum btrace_read_type type) +{ + if (the_target->read_btrace == nullptr) + error ("Target does not support branch tracing."); + + return (*the_target->read_btrace) (tinfo, buffer, type); +} + +static inline int +target_read_btrace_conf(struct btrace_target_info *tinfo, struct buffer *buffer) +{ + if (the_target->read_btrace_conf == nullptr) + error ("Target does not support branch tracing."); -#define target_read_btrace_conf(tinfo, buffer) \ - (*the_target->read_btrace_conf) (tinfo, buffer) + return (*the_target->read_btrace_conf) (tinfo, buffer); +} #define target_supports_range_stepping() \ (the_target->supports_range_stepping ? \