[PATCH 0/6] pr gdb/19340

[PATCH 0/6] pr gdb/19340

[Markus Metzger]

Fix PR gdb/19340 by stopping replaying the current thread before disabling
recording.

While writing the test for this, I wondered whether it would also work if
there was more than one process and if some were recording while others
were replaying.

It turned out that recording one process while some other process is
replaying doesn't work.  You first have to stop replaying everything
before you can continue recording.  Since btrace is per inferior, this
seems like an unnecessary restriction.

The first patch fixes gdb/pr19340; the rest makes recording work together
with replaying on inferior level.

Markus Metzger (6):
  gdb, btrace: fix pr19340
  gdb, btrace: simplify gdb.btrace/multi-inferior.exp
  gdb, btrace: remove record_btrace_target::supports_*()
  gdb, btrace: set wait status to ignore if nothing is moving
  gdb, infrun: wait for single inferior
  gdb, btrace, infrun: per-inferior run-control

 gdb/infrun.c                                | 39 ++++++++---
 gdb/linux-nat.c                             | 16 +++--
 gdb/record-btrace.c                         | 78 +++++++--------------
 gdb/testsuite/gdb.btrace/multi-inferior.c   | 10 ++-
 gdb/testsuite/gdb.btrace/multi-inferior.exp | 33 ++++-----
 gdb/testsuite/gdb.btrace/step.exp           | 22 ++++--
 6 files changed, 110 insertions(+), 88 deletions(-)

-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 1/6] gdb, btrace: fix pr19340

[Markus Metzger]

GDB fails with an assertion when stopping recording on a replaying thread
and then resuming that thread.  Stopping recording left the thread
replaying but the record target is gone.

Stop replaying all threads in the selected inferior before stopping recording.

If the selected thread had been replaying, print the (potentially updated)
location.

I had to change the stepping test slightly to account for different
compilers generating slightly different debug information, so when
stepping the 'return 0' after 'record stop' I would end up in a different
location depending on which compiler I used.  The test still covers all
stepping commands.

Fixes PR gdb/19340.
---
 gdb/record-btrace.c               | 17 +++++++++++++++++
 gdb/testsuite/gdb.btrace/step.exp | 22 +++++++++++++++-------
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 6350400c318..dd6ef8db3a3 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -415,11 +415,28 @@ record_btrace_target::stop_recording ()
 {
   DEBUG ("stop recording");
 
+  /* Remember the selected thread if it exists and is replaying.  */
+  thread_info *curr = nullptr;
+  if (inferior_ptid.lwp_p ())
+    {
+      curr = inferior_thread ();
+      if (!btrace_is_replaying (curr))
+	curr = nullptr;
+    }
+
+  record_stop_replaying ();
   record_btrace_auto_disable ();
 
   for (thread_info *tp : current_inferior ()->non_exited_threads ())
     if (tp->btrace.target != NULL)
       btrace_disable (tp);
+
+  /* Print the updated location in case we had stopped a replaying thread.  */
+  if (curr != nullptr)
+    {
+      curr->set_stop_pc (regcache_read_pc (get_thread_regcache (curr)));
+      print_stack_frame (get_selected_frame (), 1, SRC_AND_LOC, 1);
+    }
 }
 
 /* The disconnect method of target record-btrace.  */
diff --git a/gdb/testsuite/gdb.btrace/step.exp b/gdb/testsuite/gdb.btrace/step.exp
index 0adc18bd924..ed6d2140bdd 100644
--- a/gdb/testsuite/gdb.btrace/step.exp
+++ b/gdb/testsuite/gdb.btrace/step.exp
@@ -31,18 +31,26 @@ if ![runto_main] {
 # trace the call to the test function
 with_test_prefix "record" {
     gdb_test_no_output "record btrace"
-    gdb_test "next"
+    gdb_test "step"
+    gdb_test "next 2"
 }
 
 # let's step around a bit
 with_test_prefix "replay" {
-    gdb_test "reverse-next" ".*main\.2.*" "reverse-next.1"
+    gdb_test "reverse-finish" ".*main\.2.*" "reverse-finish.1"
     gdb_test "step" ".*fun4\.2.*" "step.1"
     gdb_test "next" ".*fun4\.3.*" "next.1"
     gdb_test "step" ".*fun2\.2.*" "step.2"
-    gdb_test "finish" ".*fun4\.4.*" "finish.1"
-    gdb_test "reverse-step" ".*fun2\.3.*" "reverse-step.1"
-    gdb_test "reverse-finish" ".*fun4\.3.*" "reverse-finish.1"
-    gdb_test "reverse-next" ".*fun4\.2.*" "reverse-next.2"
-    gdb_test "reverse-finish" ".*main\.2.*" "reverse-finish.2"
+    gdb_test "reverse-finish" ".*fun4\.3.*" "reverse-finish.2"
+    gdb_test "reverse-step" ".*fun1\.2.*" "reverse-step.1"
+    gdb_test "finish" ".*fun4\.3.*" "finish.1"
+    gdb_test "reverse-next" ".*fun4\.2.*" "reverse-next.1"
+    gdb_test "reverse-finish" ".*main\.2.*" "reverse-finish.3"
+}
+
+# stop recording and try to step live (pr19340)
+with_test_prefix "live" {
+    gdb_test "record stop" "fun4\.4.*Process record is stopped.*"
+    gdb_test "reverse-next" "Target .* does not support this command.*"
+    gdb_test "step" ".*fun3\.2.*"
 }
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 2/6] gdb, btrace: simplify gdb.btrace/multi-inferior.exp

[Markus Metzger]

We don't really need three inferiors to test multi-inferior recording.
We don't really need to start recording on the second inferior first.
We don't really need to check info record before starting recording.
If we were recording, there would be output, causing a fail.

This just complicates the test when there is something to debug.
---
 gdb/testsuite/gdb.btrace/multi-inferior.exp | 24 +++------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.btrace/multi-inferior.exp b/gdb/testsuite/gdb.btrace/multi-inferior.exp
index 6996b182e65..174d38364a4 100644
--- a/gdb/testsuite/gdb.btrace/multi-inferior.exp
+++ b/gdb/testsuite/gdb.btrace/multi-inferior.exp
@@ -37,6 +37,8 @@ with_test_prefix "inferior 1" {
     if ![runto_main] {
 	return -1
     }
+
+    gdb_test_no_output "record btrace"
 }
 
 with_test_prefix "inferior 2" {
@@ -48,25 +50,5 @@ with_test_prefix "inferior 2" {
 	return -1
     }
 
-    gdb_test_no_output "record btrace" "record btrace"
-}
-
-with_test_prefix "inferior 1" {
-    gdb_test "inferior 1" "Switching to inferior 1.*"
-
-    gdb_test "info record" "No recording is currently active\\."
-    gdb_test_no_output "record btrace" "record btrace"
-}
-
-with_test_prefix "inferior 3" {
-    gdb_test "add-inferior -exec ${host_binfile}" "Added inferior 3.*" \
-	"add third inferior"
-    gdb_test "inferior 3" "Switching to inferior 3.*"
-
-    if ![runto_main] {
-	return -1
-    }
-
-    gdb_test "info record" "No recording is currently active\\."
-    gdb_test_no_output "record btrace" "record btrace"
+    gdb_test_no_output "record btrace"
 }
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 3/6] gdb, btrace: remove record_btrace_target::supports_*()

[Markus Metzger]

Let's not introduce support for breakpoint types the target beneath does
not support, even though we could while replaying.

Otherwise, users may set breakpoints during replay that then couldn't be
inserted into the target when switching back to recording.
---
 gdb/record-btrace.c | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index dd6ef8db3a3..9f2999268d8 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -131,10 +131,7 @@ class record_btrace_target final : public target_ops
   bool can_execute_reverse () override;
 
   bool stopped_by_sw_breakpoint () override;
-  bool supports_stopped_by_sw_breakpoint () override;
-
   bool stopped_by_hw_breakpoint () override;
-  bool supports_stopped_by_hw_breakpoint () override;
 
   enum exec_direction_kind execution_direction () override;
   void prepare_to_generate_core () override;
@@ -2707,18 +2704,6 @@ record_btrace_target::stopped_by_sw_breakpoint ()
   return this->beneath ()->stopped_by_sw_breakpoint ();
 }
 
-/* The supports_stopped_by_sw_breakpoint method of target
-   record-btrace.  */
-
-bool
-record_btrace_target::supports_stopped_by_sw_breakpoint ()
-{
-  if (record_is_replaying (minus_one_ptid))
-    return true;
-
-  return this->beneath ()->supports_stopped_by_sw_breakpoint ();
-}
-
 /* The stopped_by_sw_breakpoint method of target record-btrace.  */
 
 bool
@@ -2734,18 +2719,6 @@ record_btrace_target::stopped_by_hw_breakpoint ()
   return this->beneath ()->stopped_by_hw_breakpoint ();
 }
 
-/* The supports_stopped_by_hw_breakpoint method of target
-   record-btrace.  */
-
-bool
-record_btrace_target::supports_stopped_by_hw_breakpoint ()
-{
-  if (record_is_replaying (minus_one_ptid))
-    return true;
-
-  return this->beneath ()->supports_stopped_by_hw_breakpoint ();
-}
-
 /* The update_thread_list method of target record-btrace.  */
 
 void
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 4/6] gdb, btrace: set wait status to ignore if nothing is moving

[Markus Metzger]

When record_btrace::wait() is called and no threads are moving, we set the
wait status to no_resumed.  Change that to ignore.

This helps with enabling per-inferior run-control for the record btrace
target as it avoids breaking out of do_target_wait() too early with
no_resumed when there would have been an event on another thread.
---
 gdb/record-btrace.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 9f2999268d8..8b20ab53ca7 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2280,14 +2280,14 @@ btrace_step_spurious (void)
   return status;
 }
 
-/* Return a target_waitstatus indicating that the thread was not resumed.  */
+/* Return a target_waitstatus indicating that nothing is moving.  */
 
 static struct target_waitstatus
-btrace_step_no_resumed (void)
+btrace_step_no_moving_threads (void)
 {
   struct target_waitstatus status;
 
-  status.set_no_resumed ();
+  status.set_ignore ();
 
   return status;
 }
@@ -2556,7 +2556,7 @@ record_btrace_target::wait (ptid_t ptid, struct target_waitstatus *status,
 
   if (moving.empty ())
     {
-      *status = btrace_step_no_resumed ();
+      *status = btrace_step_no_moving_threads ();
 
       DEBUG ("wait ended by %s: %s", null_ptid.to_string ().c_str (),
 	     status->to_string ().c_str ());
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 5/6] gdb, infrun: wait for single inferior

[Markus Metzger]

In do_target_wait(), we iterate over inferiors and call
do_target_wait_1(), which eventually calls target_wait() per inferior.
Each time, we wait for minus_one_ptid.

For the record btrace target, with one inferior replaying and the other
recording, calling wait(minus_one_ptid) prevents forwarding the request to
the target beneath for the recording inferior.

We could try to handle this in record_btrace::wait(), e.g. by forwarding
and also checking replaying threads, but it seems better to more clearly
specify the intent of the wait() request.

Wait for a single inferior, instead.  Since we iterate over all inferiors,
we still cover everything.
---
 gdb/infrun.c    | 22 +++++++++++++++++++++-
 gdb/linux-nat.c | 16 +++++++++++-----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index bbb98f6dcdb..d9ab69723b8 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4176,7 +4176,27 @@ do_target_wait (execution_control_state *ecs, target_wait_flags options)
 
   auto do_wait = [&] (inferior *inf)
   {
-    ecs->ptid = do_target_wait_1 (inf, minus_one_ptid, &ecs->ws, options);
+    /* FIXME: I don't see why we should have inferiors with zero pid,
+       which is also indicating that the respective ptid is not a process.
+
+       We do have them, though, and we could either skip them or do a
+       generic wait-for-anything.
+
+       Targets that cannot async will be asked for a blocking wait.
+
+       Blocking wait does not work inferior-by-inferior if the target
+       provides more than one inferior.
+
+       Blocking wait does not work target-by-target, either, if we have
+       more than one target.  We cannot do much about that, here, other
+       than to preserve the existing behavior of passing
+       minus_one_ptid.  */
+    ptid_t ptid { inf->pid };
+    if (!ptid.is_pid () || !target_can_async_p ()
+	|| ((options & TARGET_WNOHANG) != 0))
+      ptid = minus_one_ptid;
+
+    ecs->ptid = do_target_wait_1 (inf, ptid, &ecs->ws, options);
     ecs->target = inf->process_target ();
     return (ecs->ws.kind () != TARGET_WAITKIND_IGNORE);
   };
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 3ba072bc8d7..908623c9af7 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3330,12 +3330,18 @@ linux_nat_wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
      moment at which we know its PID.  */
   if (ptid.is_pid () && find_lwp_pid (ptid) == nullptr)
     {
-      ptid_t lwp_ptid (ptid.pid (), ptid.pid ());
+      /* Unless we already did and this is simply a request to wait for a
+	 particular inferior.  */
+      inferior *inf = find_inferior_ptid (linux_target, ptid);
+      if (inf && inf->find_thread (ptid))
+	{
+	  ptid_t lwp_ptid (ptid.pid (), ptid.pid ());
 
-      /* Upgrade the main thread's ptid.  */
-      thread_change_ptid (linux_target, ptid, lwp_ptid);
-      lp = add_initial_lwp (lwp_ptid);
-      lp->resumed = 1;
+	  /* Upgrade the main thread's ptid.  */
+	  thread_change_ptid (linux_target, ptid, lwp_ptid);
+	  lp = add_initial_lwp (lwp_ptid);
+	  lp->resumed = 1;
+	}
     }
 
   /* Make sure SIGCHLD is blocked until the sigsuspend below.  */
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

[PATCH 6/6] gdb, btrace, infrun: per-inferior run-control

[Markus Metzger]

While recording is already per inferior, run-control isn't.  As soon as
any thread in any inferior is replaying, no other inferior can be resumed.

This is controlled by many calls to record_is_replaying(minus_one_ptid).
Instead of minus_one_ptid, pass the ptid of the inferior to be checked.
---
 gdb/infrun.c                                | 17 ++++++++------
 gdb/record-btrace.c                         | 26 +++++----------------
 gdb/testsuite/gdb.btrace/multi-inferior.c   | 10 +++++++-
 gdb/testsuite/gdb.btrace/multi-inferior.exp | 19 +++++++++++++++
 4 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index d9ab69723b8..b3f7f7aa19d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2400,7 +2400,8 @@ user_visible_resume_ptid (int step)
       resume_ptid = inferior_ptid;
     }
   else if ((scheduler_mode == schedlock_replay)
-	   && target_record_will_replay (minus_one_ptid, execution_direction))
+	   && target_record_will_replay (ptid_t (inferior_ptid.pid ()),
+					 execution_direction))
     {
       /* User-settable 'scheduler' mode requires solo thread resume in replay
 	 mode.  */
@@ -3110,15 +3111,17 @@ clear_proceed_status (int step)
      This is a convenience feature to not require the user to explicitly
      stop replaying the other threads.  We're assuming that the user's
      intent is to resume tracing the recorded process.  */
+  ptid_t resume_ptid = user_visible_resume_ptid (step);
   if (!non_stop && scheduler_mode == schedlock_replay
-      && target_record_is_replaying (minus_one_ptid)
-      && !target_record_will_replay (user_visible_resume_ptid (step),
-				     execution_direction))
-    target_record_stop_replaying ();
+      && target_record_is_replaying (ptid_t (resume_ptid.pid ()))
+      && !target_record_will_replay (resume_ptid, execution_direction))
+    {
+      target_record_stop_replaying ();
+      resume_ptid = user_visible_resume_ptid (step);
+    }
 
   if (!non_stop && inferior_ptid != null_ptid)
     {
-      ptid_t resume_ptid = user_visible_resume_ptid (step);
       process_stratum_target *resume_target
 	= user_visible_resume_target (resume_ptid);
 
@@ -3197,7 +3200,7 @@ schedlock_applies (struct thread_info *tp)
 	  || (scheduler_mode == schedlock_step
 	      && tp->control.stepping_command)
 	  || (scheduler_mode == schedlock_replay
-	      && target_record_will_replay (minus_one_ptid,
+	      && target_record_will_replay (ptid_t (tp->inf->pid),
 					    execution_direction)));
 }
 
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 8b20ab53ca7..cf0b894e344 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -122,7 +122,6 @@ class record_btrace_target final : public target_ops
   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
 
   void stop (ptid_t) override;
-  void update_thread_list () override;
   bool thread_alive (ptid_t ptid) override;
   void goto_record_begin () override;
   void goto_record_end () override;
@@ -2160,7 +2159,7 @@ record_btrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
      make progress, we may need to explicitly move replaying threads to the end
      of their execution history.  */
   if ((::execution_direction != EXEC_REVERSE)
-      && !record_is_replaying (minus_one_ptid))
+      && !record_is_replaying (ptid_t (ptid.pid ())))
     {
       this->beneath ()->resume (ptid, step, signal);
       return;
@@ -2543,7 +2542,7 @@ record_btrace_target::wait (ptid_t ptid, struct target_waitstatus *status,
 
   /* As long as we're not replaying, just forward the request.  */
   if ((::execution_direction != EXEC_REVERSE)
-      && !record_is_replaying (minus_one_ptid))
+      && !record_is_replaying (ptid_t (ptid.pid ())))
     {
       return this->beneath ()->wait (ptid, status, options);
     }
@@ -2664,7 +2663,7 @@ record_btrace_target::stop (ptid_t ptid)
 
   /* As long as we're not replaying, just forward the request.  */
   if ((::execution_direction != EXEC_REVERSE)
-      && !record_is_replaying (minus_one_ptid))
+      && !record_is_replaying (ptid_t (ptid.pid ())))
     {
       this->beneath ()->stop (ptid);
     }
@@ -2694,7 +2693,7 @@ record_btrace_target::can_execute_reverse ()
 bool
 record_btrace_target::stopped_by_sw_breakpoint ()
 {
-  if (record_is_replaying (minus_one_ptid))
+  if (record_is_replaying (ptid_t (inferior_ptid.pid ())))
     {
       struct thread_info *tp = inferior_thread ();
 
@@ -2709,7 +2708,7 @@ record_btrace_target::stopped_by_sw_breakpoint ()
 bool
 record_btrace_target::stopped_by_hw_breakpoint ()
 {
-  if (record_is_replaying (minus_one_ptid))
+  if (record_is_replaying (ptid_t (inferior_ptid.pid ())))
     {
       struct thread_info *tp = inferior_thread ();
 
@@ -2719,26 +2718,13 @@ record_btrace_target::stopped_by_hw_breakpoint ()
   return this->beneath ()->stopped_by_hw_breakpoint ();
 }
 
-/* The update_thread_list method of target record-btrace.  */
-
-void
-record_btrace_target::update_thread_list ()
-{
-  /* We don't add or remove threads during replay.  */
-  if (record_is_replaying (minus_one_ptid))
-    return;
-
-  /* Forward the request.  */
-  this->beneath ()->update_thread_list ();
-}
-
 /* The thread_alive method of target record-btrace.  */
 
 bool
 record_btrace_target::thread_alive (ptid_t ptid)
 {
   /* We don't add or remove threads during replay.  */
-  if (record_is_replaying (minus_one_ptid))
+  if (record_is_replaying (ptid_t (ptid.pid ())))
     return true;
 
   /* Forward the request.  */
diff --git a/gdb/testsuite/gdb.btrace/multi-inferior.c b/gdb/testsuite/gdb.btrace/multi-inferior.c
index fb4ffc22a17..6f1052a7f25 100644
--- a/gdb/testsuite/gdb.btrace/multi-inferior.c
+++ b/gdb/testsuite/gdb.btrace/multi-inferior.c
@@ -15,8 +15,16 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+static int
+fun (void)
+{
+  int x = fun (); /* fun.1 */
+  return x;       /* fun.2 */
+}
+
 int
 main (void)
 {
-  return 0;
+  int x = fun (); /* main.1 */
+  return x;       /* main.2 */
 }
diff --git a/gdb/testsuite/gdb.btrace/multi-inferior.exp b/gdb/testsuite/gdb.btrace/multi-inferior.exp
index 174d38364a4..df7f423a088 100644
--- a/gdb/testsuite/gdb.btrace/multi-inferior.exp
+++ b/gdb/testsuite/gdb.btrace/multi-inferior.exp
@@ -39,6 +39,8 @@ with_test_prefix "inferior 1" {
     }
 
     gdb_test_no_output "record btrace"
+    gdb_test "step 4" "fun\.1.*"
+    gdb_test "reverse-step" "fun\.1.*"
 }
 
 with_test_prefix "inferior 2" {
@@ -51,4 +53,21 @@ with_test_prefix "inferior 2" {
     }
 
     gdb_test_no_output "record btrace"
+    gdb_test "step 4" "fun\.1.*"
+    gdb_test "reverse-step" "fun\.1.*"
+
+    gdb_test "info record" "Replay in progress.*"
+    gdb_test "record stop" "Process record is stopped.*"
+
+    gdb_test "step" "fun\.1.*"
+}
+
+with_test_prefix "inferior 1" {
+    gdb_test "inferior 1" "Switching to inferior 1.*"
+
+    gdb_test "info record" "Replay in progress.*"
+    gdb_test "reverse-finish" "fun\.1.*"
+    gdb_test "record goto end" "fun\.1.*"
+    gdb_test "step 2" "fun\.1.*"
+    gdb_test "reverse-step 3"
 }
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 6/6] gdb, btrace, infrun: per-inferior run-control

[Guinevere Larsen]

On 07/03/2024 14:28, Markus Metzger wrote:
> While recording is already per inferior, run-control isn't.  As soon as
> any thread in any inferior is replaying, no other inferior can be resumed.
>
> This is controlled by many calls to record_is_replaying(minus_one_ptid).
> Instead of minus_one_ptid, pass the ptid of the inferior to be checked.
> ---
>   gdb/infrun.c                                | 17 ++++++++------
>   gdb/record-btrace.c                         | 26 +++++----------------
>   gdb/testsuite/gdb.btrace/multi-inferior.c   | 10 +++++++-
>   gdb/testsuite/gdb.btrace/multi-inferior.exp | 19 +++++++++++++++
>   4 files changed, 44 insertions(+), 28 deletions(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index d9ab69723b8..b3f7f7aa19d 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2400,7 +2400,8 @@ user_visible_resume_ptid (int step)
>         resume_ptid = inferior_ptid;
>       }
>     else if ((scheduler_mode == schedlock_replay)
> -	   && target_record_will_replay (minus_one_ptid, execution_direction))
> +	   && target_record_will_replay (ptid_t (inferior_ptid.pid ()),
> +					 execution_direction))
>       {
>         /* User-settable 'scheduler' mode requires solo thread resume in replay
>   	 mode.  */
> @@ -3110,15 +3111,17 @@ clear_proceed_status (int step)
>        This is a convenience feature to not require the user to explicitly
>        stop replaying the other threads.  We're assuming that the user's
>        intent is to resume tracing the recorded process.  */
> +  ptid_t resume_ptid = user_visible_resume_ptid (step);
>     if (!non_stop && scheduler_mode == schedlock_replay
> -      && target_record_is_replaying (minus_one_ptid)
> -      && !target_record_will_replay (user_visible_resume_ptid (step),
> -				     execution_direction))
> -    target_record_stop_replaying ();
> +      && target_record_is_replaying (ptid_t (resume_ptid.pid ()))
> +      && !target_record_will_replay (resume_ptid, execution_direction))
> +    {
> +      target_record_stop_replaying ();
> +      resume_ptid = user_visible_resume_ptid (step);
> +    }
>   
>     if (!non_stop && inferior_ptid != null_ptid)
>       {
> -      ptid_t resume_ptid = user_visible_resume_ptid (step);
>         process_stratum_target *resume_target
>   	= user_visible_resume_target (resume_ptid);
>   
> @@ -3197,7 +3200,7 @@ schedlock_applies (struct thread_info *tp)
>   	  || (scheduler_mode == schedlock_step
>   	      && tp->control.stepping_command)
>   	  || (scheduler_mode == schedlock_replay
> -	      && target_record_will_replay (minus_one_ptid,
> +	      && target_record_will_replay (ptid_t (tp->inf->pid),
>   					    execution_direction)));
>   }
>   
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index 8b20ab53ca7..cf0b894e344 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -122,7 +122,6 @@ class record_btrace_target final : public target_ops
>     ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
>   
>     void stop (ptid_t) override;
> -  void update_thread_list () override;
>     bool thread_alive (ptid_t ptid) override;
>     void goto_record_begin () override;
>     void goto_record_end () override;
> @@ -2160,7 +2159,7 @@ record_btrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
>        make progress, we may need to explicitly move replaying threads to the end
>        of their execution history.  */
>     if ((::execution_direction != EXEC_REVERSE)
> -      && !record_is_replaying (minus_one_ptid))
> +      && !record_is_replaying (ptid_t (ptid.pid ())))
>       {
>         this->beneath ()->resume (ptid, step, signal);
>         return;
> @@ -2543,7 +2542,7 @@ record_btrace_target::wait (ptid_t ptid, struct target_waitstatus *status,
>   
>     /* As long as we're not replaying, just forward the request.  */
>     if ((::execution_direction != EXEC_REVERSE)
> -      && !record_is_replaying (minus_one_ptid))
> +      && !record_is_replaying (ptid_t (ptid.pid ())))
>       {
>         return this->beneath ()->wait (ptid, status, options);
>       }
> @@ -2664,7 +2663,7 @@ record_btrace_target::stop (ptid_t ptid)
>   
>     /* As long as we're not replaying, just forward the request.  */
>     if ((::execution_direction != EXEC_REVERSE)
> -      && !record_is_replaying (minus_one_ptid))
> +      && !record_is_replaying (ptid_t (ptid.pid ())))
>       {
>         this->beneath ()->stop (ptid);
>       }
> @@ -2694,7 +2693,7 @@ record_btrace_target::can_execute_reverse ()
>   bool
>   record_btrace_target::stopped_by_sw_breakpoint ()
>   {
> -  if (record_is_replaying (minus_one_ptid))
> +  if (record_is_replaying (ptid_t (inferior_ptid.pid ())))
>       {
>         struct thread_info *tp = inferior_thread ();
>   
> @@ -2709,7 +2708,7 @@ record_btrace_target::stopped_by_sw_breakpoint ()
>   bool
>   record_btrace_target::stopped_by_hw_breakpoint ()
>   {
> -  if (record_is_replaying (minus_one_ptid))
> +  if (record_is_replaying (ptid_t (inferior_ptid.pid ())))
>       {
>         struct thread_info *tp = inferior_thread ();
>   
> @@ -2719,26 +2718,13 @@ record_btrace_target::stopped_by_hw_breakpoint ()
>     return this->beneath ()->stopped_by_hw_breakpoint ();
>   }
>   
> -/* The update_thread_list method of target record-btrace.  */
> -
> -void
> -record_btrace_target::update_thread_list ()
> -{
> -  /* We don't add or remove threads during replay.  */
> -  if (record_is_replaying (minus_one_ptid))
> -    return;
> -
> -  /* Forward the request.  */
> -  this->beneath ()->update_thread_list ();
> -}
> -
>   /* The thread_alive method of target record-btrace.  */
>   
>   bool
>   record_btrace_target::thread_alive (ptid_t ptid)
>   {
>     /* We don't add or remove threads during replay.  */
> -  if (record_is_replaying (minus_one_ptid))
> +  if (record_is_replaying (ptid_t (ptid.pid ())))
>       return true;
>   
>     /* Forward the request.  */
> diff --git a/gdb/testsuite/gdb.btrace/multi-inferior.c b/gdb/testsuite/gdb.btrace/multi-inferior.c
> index fb4ffc22a17..6f1052a7f25 100644
> --- a/gdb/testsuite/gdb.btrace/multi-inferior.c
> +++ b/gdb/testsuite/gdb.btrace/multi-inferior.c
> @@ -15,8 +15,16 @@
>      You should have received a copy of the GNU General Public License
>      along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>   
> +static int
> +fun (void)
> +{
> +  int x = fun (); /* fun.1 */
> +  return x;       /* fun.2 */
> +}
> +
>   int
>   main (void)
>   {
> -  return 0;
> +  int x = fun (); /* main.1 */
> +  return x;       /* main.2 */
>   }
> diff --git a/gdb/testsuite/gdb.btrace/multi-inferior.exp b/gdb/testsuite/gdb.btrace/multi-inferior.exp
> index 174d38364a4..df7f423a088 100644
> --- a/gdb/testsuite/gdb.btrace/multi-inferior.exp
> +++ b/gdb/testsuite/gdb.btrace/multi-inferior.exp
> @@ -39,6 +39,8 @@ with_test_prefix "inferior 1" {
>       }
>   
>       gdb_test_no_output "record btrace"
> +    gdb_test "step 4" "fun\.1.*"
> +    gdb_test "reverse-step" "fun\.1.*"
>   }
>   
>   with_test_prefix "inferior 2" {
> @@ -51,4 +53,21 @@ with_test_prefix "inferior 2" {
>       }
>   
>       gdb_test_no_output "record btrace"
> +    gdb_test "step 4" "fun\.1.*"

I have not looked at all the code or dug into it, but these changes fail 
on my system.

 From this test onward GDB stopped responding at all on my machine. No 
responses whatsoever, it seems like GDB got lost in some endless loop or 
something.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> +    gdb_test "reverse-step" "fun\.1.*"
> +
> +    gdb_test "info record" "Replay in progress.*"
> +    gdb_test "record stop" "Process record is stopped.*"
> +
> +    gdb_test "step" "fun\.1.*"
> +}
> +
> +with_test_prefix "inferior 1" {
> +    gdb_test "inferior 1" "Switching to inferior 1.*"
> +
> +    gdb_test "info record" "Replay in progress.*"
> +    gdb_test "reverse-finish" "fun\.1.*"
> +    gdb_test "record goto end" "fun\.1.*"
> +    gdb_test "step 2" "fun\.1.*"
> +    gdb_test "reverse-step 3"
>   }

RE: [PATCH 6/6] gdb, btrace, infrun: per-inferior run-control

[Metzger, Markus T]

>> @@ -51,4 +53,21 @@ with_test_prefix "inferior 2" {
>>       }
>>
>>       gdb_test_no_output "record btrace"
>> +    gdb_test "step 4" "fun\.1.*"
>
>I have not looked at all the code or dug into it, but these changes fail
>on my system.
>
> From this test onward GDB stopped responding at all on my machine. No
>responses whatsoever, it seems like GDB got lost in some endless loop or
>something.

What OS and what h/w are you using?

Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 6/6] gdb, btrace, infrun: per-inferior run-control

[Guinevere Larsen]

On 08/03/2024 11:57, Metzger, Markus T wrote:
>>> @@ -51,4 +53,21 @@ with_test_prefix "inferior 2" {
>>>        }
>>>
>>>        gdb_test_no_output "record btrace"
>>> +    gdb_test "step 4" "fun\.1.*"
>> I have not looked at all the code or dug into it, but these changes fail
>> on my system.
>>
>>  From this test onward GDB stopped responding at all on my machine. No
>> responses whatsoever, it seems like GDB got lost in some endless loop or
>> something.
> What OS and what h/w are you using?
>
Fedora 39, and the cpu is Intel(R) Core(TM) i7-10850H

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

RE: [PATCH 6/6] gdb, btrace, infrun: per-inferior run-control

[Metzger, Markus T]

>On 08/03/2024 11:57, Metzger, Markus T wrote:
>>>> @@ -51,4 +53,21 @@ with_test_prefix "inferior 2" {
>>>>        }
>>>>
>>>>        gdb_test_no_output "record btrace"
>>>> +    gdb_test "step 4" "fun\.1.*"
>>> I have not looked at all the code or dug into it, but these changes fail
>>> on my system.
>>>
>>>  From this test onward GDB stopped responding at all on my machine. No
>>> responses whatsoever, it seems like GDB got lost in some endless loop or
>>> something.
>> What OS and what h/w are you using?
>>
>Fedora 39, and the cpu is Intel(R) Core(TM) i7-10850H

Hmmmm.  I tested this on Fedora 39 (in a container running on ubuntu 22.04).

Do the other gdb.btrace tests pass?
Where exactly does it start hanging?
What board file are you using?

Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 6/6] gdb, btrace, infrun: per-inferior run-control

[Guinevere Larsen]

On 08/03/2024 12:11, Metzger, Markus T wrote:
>> On 08/03/2024 11:57, Metzger, Markus T wrote:
>>>>> @@ -51,4 +53,21 @@ with_test_prefix "inferior 2" {
>>>>>         }
>>>>>
>>>>>         gdb_test_no_output "record btrace"
>>>>> +    gdb_test "step 4" "fun\.1.*"
>>>> I have not looked at all the code or dug into it, but these changes fail
>>>> on my system.
>>>>
>>>>   From this test onward GDB stopped responding at all on my machine. No
>>>> responses whatsoever, it seems like GDB got lost in some endless loop or
>>>> something.
>>> What OS and what h/w are you using?
>>>
>> Fedora 39, and the cpu is Intel(R) Core(TM) i7-10850H
> Hmmmm.  I tested this on Fedora 39 (in a container running on ubuntu 22.04).
hmmm, that's odd
>
> Do the other gdb.btrace tests pass?
Yes. Only the TSX test shows unsupported, otherwise, everything passes 
just fine
> Where exactly does it start hanging?
The test is "inferior 2: step 4". I can try and debug gdb with gdb to 
figure out where if you want.
> What board file are you using?
The basic one, I think. my test CLI was just "make check 
TESTS=gdb.btrace/multi-inferior.exp"

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

RE: [PATCH 6/6] gdb, btrace, infrun: per-inferior run-control

[Metzger, Markus T]

>> Where exactly does it start hanging?
>The test is "inferior 2: step 4". I can try and debug gdb with gdb to
>figure out where if you want.
>> What board file are you using?
>The basic one, I think. my test CLI was just "make check
>TESTS=gdb.btrace/multi-inferior.exp"

I could reproduce it and I have a fix.  I missed it because perf was
disabled by default in the container where I did the full test run.
That is fixed, too.

Thanks for testing this and for reporting the fail.

Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928