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

[Guinevere Larsen <blarsen@redhat.com>]

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"
>   }