RE: [PATCH v2 5/7] btrace, gdbserver: remove the to_supports_btrace target method

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

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 <markus.t.metzger@intel.com>
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 <macro@mips.com> for reporting this.
    
    Signed-off-by: Markus Metzger  <markus.t.metzger@intel.com>
    
    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.

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 ? \

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