diff mbox series

gdb/testsuite/gdb.rocm: Fix incorrect use of continue N in multi-inferior-gpu.exp

Message ID 20231017111629.307261-1-lancelot.six@amd.com
State New
Headers
Series gdb/testsuite/gdb.rocm: Fix incorrect use of continue N in multi-inferior-gpu.exp |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Lancelot SIX Oct. 17, 2023, 11:16 a.m. UTC 
  The gdb.rocm/multi-inferior-gpu.exp testcase uses a "continue $thread"
command, but this is incorrect.  If "continue" is given an argument, it
sets the ignore count of the breakpoint the thread stopped at.

For this testcase it does not really matter since the breakpoint is not
meant to be hit anymore, so whatever the ignore count is won't influence
the outcome of the test.  It is worth fixing nevertheless.

While at changing it, switch to using "continue&" to consume the GDB
prompt right away.  This makes the following pattern matching more
reliable.

Change-Id: I0eb674d5529cdeb9e808b74870a29b6077265737
---
 gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Lancelot SIX Oct. 17, 2023, 11:21 a.m. UTC | #1 
Hi,


> diff --git a/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp b/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp
> index 18b4172ff09..958b70e6df1 100644
> --- a/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp
> +++ b/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp
> @@ -68,8 +68,10 @@ proc do_test {} {
>   	foreach thread $stopped_gpu_threads {
>   	    set infnumber [lindex [split $thread .] 0]
>   	    gdb_test "thread $thread" "Switching to thread.*"
> -	    gdb_test_multiple "continue $thread" "" {
> -		-re "\\\[Inferior $infnumber \[^\n\r\]* exited normally\\]\r\n$::gdb_prompt " {
> +	    # Continue this thread, and consime the prompt
                                             ^
There is a typo here.  I have fixed it locally.

Best,
Lancelot.

> +	    gdb_test "continue&" "" "continue GPU thread in $infnumber"
> +	    gdb_test_multiple "" "wait for inferior $infnumber" {
> +		-re "\\\[Inferior $infnumber \[^\n\r\]* exited normally\\\]\r\n" {
>   		    pass $gdb_test_name
>   		}
>   	    }
Simon Marchi Oct. 17, 2023, 2:25 p.m. UTC | #2 
On 10/17/23 07:16, Lancelot Six wrote:
> The gdb.rocm/multi-inferior-gpu.exp testcase uses a "continue $thread"
> command, but this is incorrect.  If "continue" is given an argument, it
> sets the ignore count of the breakpoint the thread stopped at.
> 
> For this testcase it does not really matter since the breakpoint is not
> meant to be hit anymore, so whatever the ignore count is won't influence
> the outcome of the test.  It is worth fixing nevertheless.
> 
> While at changing it, switch to using "continue&" to consume the GDB
> prompt right away.  This makes the following pattern matching more
> reliable.

I don't understand the "more reliable" part, can you explain?

Simon
Lancelot SIX Oct. 17, 2023, 2:41 p.m. UTC | #3 
> On 10/17/23 07:16, Lancelot Six wrote:
>> The gdb.rocm/multi-inferior-gpu.exp testcase uses a "continue $thread"
>> command, but this is incorrect.  If "continue" is given an argument, it
>> sets the ignore count of the breakpoint the thread stopped at.
>>
>> For this testcase it does not really matter since the breakpoint is not
>> meant to be hit anymore, so whatever the ignore count is won't influence
>> the outcome of the test.  It is worth fixing nevertheless.
>>
>> While at changing it, switch to using "continue&" to consume the GDB
>> prompt right away.  This makes the following pattern matching more
>> reliable.
> 
> I don't understand the "more reliable" part, can you explain?
> 
> Simon

There will be a "(gdb) " prompt appearing at some point after a 
"continue", but in non-stop we don't really control when it appears (if 
I understand correctly).  It seems that other non-stop tests use the "&" 
variant and consume the prompt immediately.

With this, before the next command is issued, we know that 1) we got a 
prompt and 2) we have seen the process we just released finish.

Overall, I am mostly trying to follow patterns I caught in other 
non-stop tests.

Best,
Lancelot.
Simon Marchi Oct. 17, 2023, 2:48 p.m. UTC | #4 
On 10/17/23 10:41, Lancelot SIX wrote:
> 
>> On 10/17/23 07:16, Lancelot Six wrote:
>>> The gdb.rocm/multi-inferior-gpu.exp testcase uses a "continue $thread"
>>> command, but this is incorrect.  If "continue" is given an argument, it
>>> sets the ignore count of the breakpoint the thread stopped at.
>>>
>>> For this testcase it does not really matter since the breakpoint is not
>>> meant to be hit anymore, so whatever the ignore count is won't influence
>>> the outcome of the test.  It is worth fixing nevertheless.
>>>
>>> While at changing it, switch to using "continue&" to consume the GDB
>>> prompt right away.  This makes the following pattern matching more
>>> reliable.
>>
>> I don't understand the "more reliable" part, can you explain?
>>
>> Simon
> 
> There will be a "(gdb) " prompt appearing at some point after a "continue", but in non-stop we don't really control when it appears (if I understand correctly).  It seems that other non-stop tests use the "&" variant and consume the prompt immediately.
> 
> With this, before the next command is issued, we know that 1) we got a prompt and 2) we have seen the process we just released finish.
> 
> Overall, I am mostly trying to follow patterns I caught in other non-stop tests.

Ah, I missed that we were in non-stop.

If we run a single thread at a time, I think it will be reliable to use
the sync variant of continue.  Using the async variant is useful when
you resume multiple threads and expect multiple events (like breakpoint
hits) which can happen in any order.  At least that's my understanding.

Simon
Lancelot SIX Oct. 18, 2023, 8:32 a.m. UTC | #5 
On 17/10/2023 15:48, Simon Marchi wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On 10/17/23 10:41, Lancelot SIX wrote:
>>
>>> On 10/17/23 07:16, Lancelot Six wrote:
>>>> The gdb.rocm/multi-inferior-gpu.exp testcase uses a "continue $thread"
>>>> command, but this is incorrect.  If "continue" is given an argument, it
>>>> sets the ignore count of the breakpoint the thread stopped at.
>>>>
>>>> For this testcase it does not really matter since the breakpoint is not
>>>> meant to be hit anymore, so whatever the ignore count is won't influence
>>>> the outcome of the test.  It is worth fixing nevertheless.
>>>>
>>>> While at changing it, switch to using "continue&" to consume the GDB
>>>> prompt right away.  This makes the following pattern matching more
>>>> reliable.
>>>
>>> I don't understand the "more reliable" part, can you explain?
>>>
>>> Simon
>>
>> There will be a "(gdb) " prompt appearing at some point after a "continue", but in non-stop we don't really control when it appears (if I understand correctly).  It seems that other non-stop tests use the "&" variant and consume the prompt immediately.
>>
>> With this, before the next command is issued, we know that 1) we got a prompt and 2) we have seen the process we just released finish.
>>
>> Overall, I am mostly trying to follow patterns I caught in other non-stop tests.
> 
> Ah, I missed that we were in non-stop.
> 
> If we run a single thread at a time, I think it will be reliable to use
> the sync variant of continue.  Using the async variant is useful when
> you resume multiple threads and expect multiple events (like breakpoint
> hits) which can happen in any order.  At least that's my understanding.
> 
> Simon

OK, if you find it is overdoing it, I'll revert this part of the patch 
and submit a V2 shortly.

Thanks,
Lancelot.
diff mbox series

Patch

diff --git a/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp b/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp
index 18b4172ff09..958b70e6df1 100644
--- a/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp
+++ b/gdb/testsuite/gdb.rocm/multi-inferior-gpu.exp
@@ -68,8 +68,10 @@  proc do_test {} {
 	foreach thread $stopped_gpu_threads {
 	    set infnumber [lindex [split $thread .] 0]
 	    gdb_test "thread $thread" "Switching to thread.*"
-	    gdb_test_multiple "continue $thread" "" {
-		-re "\\\[Inferior $infnumber \[^\n\r\]* exited normally\\]\r\n$::gdb_prompt " {
+	    # Continue this thread, and consime the prompt
+	    gdb_test "continue&" "" "continue GPU thread in $infnumber"
+	    gdb_test_multiple "" "wait for inferior $infnumber" {
+		-re "\\\[Inferior $infnumber \[^\n\r\]* exited normally\\\]\r\n" {
 		    pass $gdb_test_name
 		}
 	    }