[binutils-gdb] gdb, btrace: diagnose double and failed enable

[binutils-gdb] gdb, btrace: diagnose double and failed enable

[Markus Metzger]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5897fd4994effd21cbe951e6d2c417097adea162

commit 5897fd4994effd21cbe951e6d2c417097adea162
Author: Markus Metzger <markus.t.metzger@intel.com>
Date:   Fri Mar 13 09:58:10 2020 +0100

    gdb, btrace: diagnose double and failed enable
    
    GDB silently ignores attempts to enable branch tracing on a thread that is
    already recorded.  This shouldn't happen as recording is enabled exactly
    once:
    
      - when the btrace record target is opened for existing threads
      - when a new thread is added while the btrace record target is pushed
    
    GDB also silently ignores if recording is disabled on threads that were not
    recorded.  This shouldn't happen, either, since when stopping recording,
    we only disable recording on threads that were recorded.
    
    GDB further silently ignores if recording was not enabled by the
    corresponding target method.  Also this shouldn't happen since the target
    is supposed to already throw an error if recording cannot be enabled.
    This new error in btrace_enable catches cases where the target silently
    failed to enable recording.
    
    Throw an error in those cases.
    
    This allows us to detect an actual issue more easily.  It will be
    addressed in the next patch.
    
    gdb/ChangeLog:
    
    2020-03-19  Markus Metzger  <markus.t.metzger@intel.com>
    
            * btrace.c (btrace_enable): Throw an error on double enables and
            when enabling recording fails.
            (btrace_disable): Throw an error if the thread is not recorded.

Diff:
---
 gdb/ChangeLog |  6 ++++++
 gdb/btrace.c  | 10 ++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c761d498b7f..05110002e78 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2020-04-21  Markus Metzger  <markus.t.metzger@intel.com>
+
+	* btrace.c (btrace_enable): Throw an error on double enables and
+	when enabling recording fails.
+	(btrace_disable): Throw an error if the thread is not recorded.
+
 2020-04-21  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* record-btrace.c (record_btrace_target::fetch_registers): Forward
diff --git a/gdb/btrace.c b/gdb/btrace.c
index 9f90d59e2b5..d41e3c4f8f9 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1592,7 +1592,8 @@ void
 btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
 {
   if (tp->btrace.target != NULL)
-    return;
+    error (_("Recording already enabled on thread %s (%s)."),
+	   print_thread_id (tp), target_pid_to_str (tp->ptid).c_str ());
 
 #if !defined (HAVE_LIBIPT)
   if (conf->format == BTRACE_FORMAT_PT)
@@ -1604,9 +1605,9 @@ btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
 
   tp->btrace.target = target_enable_btrace (tp->ptid, conf);
 
-  /* We're done if we failed to enable tracing.  */
   if (tp->btrace.target == NULL)
-    return;
+    error (_("Failed to enable recording on thread %s (%s)."),
+	   print_thread_id (tp), target_pid_to_str (tp->ptid).c_str ());
 
   /* We need to undo the enable in case of errors.  */
   try
@@ -1651,7 +1652,8 @@ btrace_disable (struct thread_info *tp)
   struct btrace_thread_info *btp = &tp->btrace;
 
   if (btp->target == NULL)
-    return;
+    error (_("Recording not enabled on thread %s (%s)."),
+	   print_thread_id (tp), target_pid_to_str (tp->ptid).c_str ());
 
   DEBUG ("disable thread %s (%s)", print_thread_id (tp),
 	 target_pid_to_str (tp->ptid).c_str ());