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

  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(-)
  

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.
  

>> @@ -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
  

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
  

>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
  

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"
  

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