RE: [PATCH 17/17] infrun: scheduler-locking reverse

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----Original Message-----
> From: Metzger, Markus T
> Sent: Wednesday, September 16, 2015 9:59 AM
> To: Jan Kratochvil
> Cc: Pedro Alves; gdb-patches@sourceware.org
> Subject: RE: [PATCH 17/17] infrun: scheduler-locking reverse

Hello Jan, Pedro,

> We could also make record btrace honour scheduler-locking for (soon legacy)
> all-stop targets.

How about something like this?  See also users/mmetzger/btrace-non-stop.
I'm going to send this in v3 of the non-stop series once the name of the new
scheduler-locking mode has been decided.


commit 6b8b0c23b57890cf0b8023b2b7a40bd3abf10758
Author: Markus Metzger <markus.t.metzger@intel.com>
Date:   Wed Sep 16 09:05:22 2015 +0200

    btrace: honour scheduler-locking for all-stop targets
    
    In all-stop mode, record btrace maintains the old behaviour of an implicit
    scheduler-locking on.
    
    Now that we added a scheduler-locking mode to model this old behaviour, we
    don't need the respective code in record btrace anymore.  Remove it.
    
    For all-stop targets, step inferior_ptid and continue other threads matching
    the argument ptid.
    
    This assumes that inferior_ptid matches the argument ptid.  If it doesn't,
    behave as if the target were non-stop and apply the move command to all
    threads matching the argument ptid.
    
    This should make record btrace honour scheduler-locking.
    
    Signed-off-by: Markus Metzger <markus.t.metzger@intel.com>
    
    gdb/
    	* record-btrace.c (record_btrace_resume): Honour scheduler-locking.
    
    testsuite/
    	* gdb.btrace/multi-thread-step.exp: Test scheduler-locking on, step,
    	and reverse.

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index b8e6466..057f2f9 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1880,33 +1880,18 @@ record_btrace_resume (struct target_ops *ops, ptid_t ptid, int step,
 		      enum gdb_signal signal)
 {
   struct thread_info *tp;
-  enum btrace_thread_flag flag;
-  ptid_t orig_ptid;
+  enum btrace_thread_flag flag, cflag;
 
   DEBUG ("resume %s: %s%s", target_pid_to_str (ptid),
 	 execution_direction == EXEC_REVERSE ? "reverse-" : "",
 	 step ? "step" : "cont");
 
-  orig_ptid = ptid;
-
   /* Store the execution direction of the last resume.
 
      If there is more than one to_resume call, we have to rely on infrun
      to not change the execution direction in-between.  */
   record_btrace_resume_exec_dir = execution_direction;
 
-  /* For all-stop targets we pick the current thread when asked to resume an
-     entire process or everything.  */
-  if (!target_is_non_stop_p ())
-    {
-      if (ptid_equal (minus_one_ptid, ptid) || ptid_is_pid (ptid))
-	ptid = inferior_ptid;
-
-      tp = find_thread_ptid (ptid);
-      if (tp == NULL)
-	error (_("Cannot find thread to resume."));
-    }
-
   /* As long as we're not replaying, just forward the request.
 
      For non-stop targets this means that no thread is replaying.  In order to
@@ -1916,20 +1901,42 @@ record_btrace_resume (struct target_ops *ops, ptid_t ptid, int step,
       && !record_btrace_is_replaying (ops, minus_one_ptid))
     {
       ops = ops->beneath;
-      return ops->to_resume (ops, orig_ptid, step, signal);
+      return ops->to_resume (ops, ptid, step, signal);
     }
 
   /* Compute the btrace thread flag for the requested move.  */
-  if (step == 0)
-    flag = execution_direction == EXEC_REVERSE ? BTHR_RCONT : BTHR_CONT;
+  if (execution_direction == EXEC_REVERSE)
+    {
+      flag = step == 0 ? BTHR_RCONT : BTHR_RSTEP;
+      cflag = BTHR_RCONT;
+    }
   else
-    flag = execution_direction == EXEC_REVERSE ? BTHR_RSTEP : BTHR_STEP;
+    {
+      flag = step == 0 ? BTHR_CONT : BTHR_STEP;
+      cflag = BTHR_CONT;
+    }
 
   /* We just indicate the resume intent here.  The actual stepping happens in
-     record_btrace_wait below.  */
-  ALL_NON_EXITED_THREADS (tp)
-    if (ptid_match (tp->ptid, ptid))
-      record_btrace_resume_thread (tp, flag);
+     record_btrace_wait below.
+
+     For all-stop targets, we only step INFERIOR_PTID and continue others.  */
+  if (!target_is_non_stop_p () && ptid_match (inferior_ptid, ptid))
+    {
+      ALL_NON_EXITED_THREADS (tp)
+	if (ptid_match (tp->ptid, ptid))
+	  {
+	    if (ptid_match (tp->ptid, inferior_ptid))
+	      record_btrace_resume_thread (tp, flag);
+	    else
+	      record_btrace_resume_thread (tp, cflag);
+	  }
+    }
+  else
+    {
+      ALL_NON_EXITED_THREADS (tp)
+	if (ptid_match (tp->ptid, ptid))
+	  record_btrace_resume_thread (tp, flag);
+    }
 
   /* Async support.  */
   if (target_can_async_p ())
diff --git a/gdb/testsuite/gdb.btrace/multi-thread-step.exp b/gdb/testsuite/gdb.btrace/multi-thread-step.exp
index 2295f71..249d075 100644
--- a/gdb/testsuite/gdb.btrace/multi-thread-step.exp
+++ b/gdb/testsuite/gdb.btrace/multi-thread-step.exp
@@ -37,9 +37,29 @@ set bp_2 [gdb_get_line_number "bp.2" $srcfile]
 set bp_3 [gdb_get_line_number "bp.3" $srcfile]
 
 proc gdb_cont_to_line { line } {
-	gdb_breakpoint $line
-	gdb_continue_to_breakpoint "cont to $line" ".*$line\r\n.*"
-	delete_breakpoints
+    gdb_breakpoint $line
+    gdb_continue_to_breakpoint "cont to $line" ".*$line\r\n.*"
+    delete_breakpoints
+}
+
+proc check_replay_insn { thread insn } {
+    gdb_test "thread apply $thread info record" \
+        "Replay in progress\.  At instruction $insn\."
+}
+
+proc check_not_replaying { thread } {
+    global gdb_prompt
+
+    set test "thread $thread not replaying"
+
+    gdb_test_multiple "thread apply $thread info record" $test {
+        -re "Replay in progress" {
+            fail $test
+        }
+        -re "$gdb_prompt $" {
+            pass $test
+        }
+    }
 }
 
 # trace the code between the two breakpoints
@@ -50,86 +70,159 @@ gdb_test "info threads" ".*"
 gdb_test_no_output "record btrace"
 gdb_cont_to_line $srcfile:$bp_2
 
-# navigate in the trace history for both threads
-with_test_prefix "navigate" {
-  gdb_test "thread 1" ".*"
-  with_test_prefix "thread 1" {
-    gdb_test "record goto begin" ".*"
-    gdb_test "info record" ".*Replay in progress\.  At instruction 1\."
-  }
-  gdb_test "thread 2" ".*"
-  with_test_prefix "thread 2" {
-    gdb_test "record goto begin" ".*"
-    gdb_test "info record" ".*Replay in progress\.  At instruction 1\."
-  }
+proc test_navigate {} {
+    with_test_prefix "navigate" {
+        gdb_test "thread 1" ".*"
+        with_test_prefix "thread 1" {
+            gdb_test "record goto begin" ".*"
+
+            check_replay_insn 1 1
+            check_not_replaying 2
+        }
+        gdb_test "thread 2" ".*"
+        with_test_prefix "thread 2" {
+            gdb_test "record goto begin" ".*"
+
+            check_replay_insn 1 1
+            check_replay_insn 2 1
+        }
+    }
 }
 
-# step both threads
-with_test_prefix "step" {
-  gdb_test "thread 1" ".*"
-  with_test_prefix "thread 1" {
-    gdb_test "info record" ".*Replay in progress\.  At instruction 1\."
-    gdb_test "stepi" ".*"
-    gdb_test "info record" ".*Replay in progress\.  At instruction 2\."
-  }
-  gdb_test "thread 2" ".*"
-  with_test_prefix "thread 2" {
-    gdb_test "info record" ".*Replay in progress\.  At instruction 1\."
-    gdb_test "stepi" ".*"
-    gdb_test "info record" ".*Replay in progress\.  At instruction 2\."
-  }
+proc test_step {} {
+    with_test_prefix "step" {
+        gdb_test "thread 1" ".*"
+        with_test_prefix "thread 1" {
+            gdb_test "stepi" ".*"
+
+            check_replay_insn 1 2
+            check_replay_insn 2 1
+        }
+        gdb_test "thread 2" ".*"
+        with_test_prefix "thread 2" {
+            gdb_test "stepi" ".*"
+
+            check_replay_insn 1 2
+            check_replay_insn 2 2
+        }
+    }
 }
 
-# run to the end of the history for both threads
-with_test_prefix "cont" {
-  gdb_test "thread 1" ".*"
-  with_test_prefix "thread 1" {
-    gdb_test "info record" ".*Replay in progress\.  At instruction 2\."
-    gdb_test "continue" "No more reverse-execution history.*"
-  }
-  gdb_test "thread 2" ".*"
-  with_test_prefix "thread 2" {
-    gdb_test "info record" ".*Replay in progress\.  At instruction 2\."
-    gdb_test "continue" "No more reverse-execution history.*"
-  }
+proc test_cont {} {
+    with_test_prefix "cont" {
+        gdb_test "thread 1" ".*"
+        with_test_prefix "thread 1" {
+            gdb_test "continue" "No more reverse-execution history.*"
+
+            check_not_replaying 1
+            check_replay_insn 2 2
+        }
+        gdb_test "thread 2" ".*"
+        with_test_prefix "thread 2" {
+            gdb_test "continue" "No more reverse-execution history.*"
+
+            check_not_replaying 1
+            check_not_replaying 2
+        }
+    }
 }
 
-# reverse-step both threads
-with_test_prefix "reverse-step" {
-  gdb_test "thread 1" ".*"
-  with_test_prefix "thread 1" {
-    gdb_test "reverse-stepi" ".*"
-    gdb_test "info record" ".*Replay in progress\..*"
-  }
-  gdb_test "thread 2" ".*"
-  with_test_prefix "thread 2" {
-    gdb_test "reverse-stepi" ".*"
-    gdb_test "info record" ".*Replay in progress\..*"
-  }
+proc test_cont_all {} {
+    with_test_prefix "cont-all" {
+        gdb_test "continue" "No more reverse-execution history.*"
+
+        # this works because we're lock-stepping threads that executed exactly
+        # the same code starting from the same instruction.
+
+        check_not_replaying 1
+        check_not_replaying 2
+    }
 }
 
-# both threads are still replaying
-with_test_prefix "check" {
-  gdb_test "thread 1" ".*"
-  with_test_prefix "thread 1" {
-    gdb_test "info record" ".*Replay in progress\..*"
-  }
-  gdb_test "thread 2" ".*"
-  with_test_prefix "thread 2" {
-    gdb_test "info record" ".*Replay in progress\..*"
-  }
+proc test_rstep {} {
+    with_test_prefix "reverse-step" {
+        gdb_test "thread apply all record goto 3"
+
+        gdb_test "thread 1" ".*"
+        with_test_prefix "thread 1" {
+            gdb_test "reverse-stepi" ".*"
+
+            check_replay_insn 1 2
+            check_replay_insn 2 3
+        }
+        gdb_test "thread 2" ".*"
+        with_test_prefix "thread 2" {
+            gdb_test "reverse-stepi" ".*"
+
+            check_replay_insn 1 2
+            check_replay_insn 2 2
+        }
+    }
 }
 
+proc test_goto_end {} {
+    with_test_prefix "goto-end" {
+        gdb_test "thread apply all record goto end"
+
+        check_not_replaying 1
+        check_not_replaying 2
+    }
+}
+
+with_test_prefix "schedlock-reverse" {
+    gdb_test_no_output "set scheduler-locking reverse"
+
+    test_navigate
+    test_step
+    test_cont
+    test_rstep
+    test_goto_end
+}
+
+with_test_prefix "schedlock-on" {
+    gdb_test_no_output "set scheduler-locking on"
+
+    test_navigate
+    test_step
+    test_cont
+    test_rstep
+    test_goto_end
+}
+
+with_test_prefix "schedlock-step" {
+    gdb_test_no_output "set scheduler-locking step"
+
+    test_navigate
+    test_step
+    test_cont_all
+    test_rstep
+    test_goto_end
+}
+
+# schedlock-off is difficult to test since we can't really say where the other
+# thread will be when the resumed thread stops.
+
 # navigate back into the history for thread 1 and continue thread 2
-with_test_prefix "cont" {
-  gdb_test "thread 1" ".*"
-  with_test_prefix "thread 1" {
-    gdb_test "record goto begin" ".*"
-    gdb_test "info record" ".*Replay in progress\.  At instruction 1\."
-  }
-  gdb_test "thread 2" ".*"
-  with_test_prefix "thread 2" {
-    gdb_test "record goto end" ".*"
-    gdb_cont_to_line $srcfile:$bp_3
-  }
+with_test_prefix "cont-to-end" {
+    # this test only works for scheduler-locking reverse and off
+    gdb_test_no_output "set scheduler-locking reverse"
+
+    gdb_test "thread 1" ".*"
+    with_test_prefix "thread 1" {
+        gdb_test "record goto begin" ".*"
+
+        check_replay_insn 1 1
+    }
+    gdb_test "thread 2" ".*"
+    with_test_prefix "thread 2" {
+        gdb_test "record goto end" ".*"
+
+        check_not_replaying 2
+
+        # if we reach the breakpoint, thread 2 terminated...
+        gdb_cont_to_line $srcfile:$bp_3
+
+        # and thread 1 stopped replaying
+        check_not_replaying 1
+    }
 }
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, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928