[PATCH] process-dies-while-detaching.exp: Exit early if GDB misses sync breakpoint

[PATCH] process-dies-while-detaching.exp: Exit early if GDB misses sync breakpoint

[Thiago Jung Bauermann]

I'm seeing a lot of variability in the failures of
gdb.threads/process-dies-while-detaching.exp on aarch64-linux.  On this
platform, a problem yet to be investigated causes GDB to miss the _exit
breakpoint.  What happens next is random because after missing that
breakpoint, GDB is out of sync with the inferior.  This causes the tests
following that point in the testcase to fail in a random way.

In this scenario it's better to exit the testcase early to avoid random
results in the testsuite.

We are relying on gdb_continue_to_breakpoint to return the result of
gdb_test_multiple.  This is already the case because in Tcl the return
value of a function is the return value of the last command it runs.  But
change gdb_continue_to_breakpoint to explicitly return this value, to make
it clear this is the intended behaviour.

Tested on aarch64-linux.
---
 .../gdb.threads/process-dies-while-detaching.exp       | 10 +++++-----
 gdb/testsuite/lib/gdb.exp                              |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
index bbbe82df30cd..c7d43d7dddcc 100644
--- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
+++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
@@ -127,7 +127,7 @@ proc detach_and_expect_exit {inf_output_re test} {
 
 proc continue_to_exit_bp {} {
     gdb_breakpoint "_exit" temporary
-    gdb_continue_to_breakpoint "_exit" ".*_exit.*"
+    return [gdb_continue_to_breakpoint "_exit" ".*_exit.*"]
 }
 
 # If testing single-process, simply detach from the process.
@@ -226,7 +226,7 @@ proc test_detach {multi_process cmd} {
 	}
 
 	# Run to _exit in the child.
-	continue_to_exit_bp
+	return_if_fail [continue_to_exit_bp]
 
 	do_detach $multi_process $cmd "normal"
     }
@@ -265,13 +265,13 @@ proc test_detach_watch {wp multi_process cmd} {
 	    # them out, and handle the case of the thread disappearing
 	    # while doing that (on targets that need to detach from each
 	    # thread individually).
-	    continue_to_exit_bp
+	    return_if_fail [continue_to_exit_bp]
 	} else {
 	    # Force software watchpoints.
 	    gdb_test_no_output "set can-use-hw-watchpoints 0"
 
 	    # As above, but flip order, other wise things take too long.
-	    continue_to_exit_bp
+	    return_if_fail [continue_to_exit_bp]
 	    gdb_test "watch globalvar" "Watchpoint $decimal: globalvar"
 
 	    if { $multi_process == 0 && $cmd == "continue" } {
@@ -304,7 +304,7 @@ proc test_detach_killed_outside {multi_process cmd} {
 	}
 
 	# Run to _exit in the child.
-	continue_to_exit_bp
+	return_if_fail [continue_to_exit_bp]
 
 	set childpid [get_integer_valueof "mypid" -1]
 	if { $childpid == -1 } {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 1b9179401c45..dfabc99fe4be 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -822,14 +822,14 @@ proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
     set full_name "continue to breakpoint: $name"
 
     set kfail_pattern "Process record does not support instruction 0xfae64 at.*"
-    gdb_test_multiple "continue" $full_name {
+    return [gdb_test_multiple "continue" $full_name {
 	-re "(?:Breakpoint|Temporary breakpoint) .* (at|in) $location_pattern\r\n$gdb_prompt $" {
 	    pass $full_name
 	}
 	-re "(?:$kfail_pattern)\r\n$gdb_prompt $" {
 	    kfail "gdb/25038" $full_name
 	}
-    }
+    }]
 }
 
 

Re: [PATCH] process-dies-while-detaching.exp: Exit early if GDB misses sync breakpoint

[Guinevere Larsen]

On 29/09/2023 19:53, Thiago Jung Bauermann via Gdb-patches wrote:
> I'm seeing a lot of variability in the failures of
> gdb.threads/process-dies-while-detaching.exp on aarch64-linux.  On this
> platform, a problem yet to be investigated causes GDB to miss the _exit
> breakpoint.  What happens next is random because after missing that
> breakpoint, GDB is out of sync with the inferior.  This causes the tests
> following that point in the testcase to fail in a random way.
>
> In this scenario it's better to exit the testcase early to avoid random
> results in the testsuite.
Thanks for working on this test, it has been driving me up the wall for 
a while!
>
> We are relying on gdb_continue_to_breakpoint to return the result of
> gdb_test_multiple.  This is already the case because in Tcl the return
> value of a function is the return value of the last command it runs.  But
> change gdb_continue_to_breakpoint to explicitly return this value, to make
> it clear this is the intended behaviour.
I like this change, since I didn't actually know that this was TCL's 
behavior!
>
> Tested on aarch64-linux.

I tested on x86_64 and I see no regressions.

Tested-By: Guinevere Larsen <blarsen@redhat.com>

I hope this gets approved soon!

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> ---
>   .../gdb.threads/process-dies-while-detaching.exp       | 10 +++++-----
>   gdb/testsuite/lib/gdb.exp                              |  4 ++--
>   2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> index bbbe82df30cd..c7d43d7dddcc 100644
> --- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> +++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> @@ -127,7 +127,7 @@ proc detach_and_expect_exit {inf_output_re test} {
>   
>   proc continue_to_exit_bp {} {
>       gdb_breakpoint "_exit" temporary
> -    gdb_continue_to_breakpoint "_exit" ".*_exit.*"
> +    return [gdb_continue_to_breakpoint "_exit" ".*_exit.*"]
>   }
>   
>   # If testing single-process, simply detach from the process.
> @@ -226,7 +226,7 @@ proc test_detach {multi_process cmd} {
>   	}
>   
>   	# Run to _exit in the child.
> -	continue_to_exit_bp
> +	return_if_fail [continue_to_exit_bp]
>   
>   	do_detach $multi_process $cmd "normal"
>       }
> @@ -265,13 +265,13 @@ proc test_detach_watch {wp multi_process cmd} {
>   	    # them out, and handle the case of the thread disappearing
>   	    # while doing that (on targets that need to detach from each
>   	    # thread individually).
> -	    continue_to_exit_bp
> +	    return_if_fail [continue_to_exit_bp]
>   	} else {
>   	    # Force software watchpoints.
>   	    gdb_test_no_output "set can-use-hw-watchpoints 0"
>   
>   	    # As above, but flip order, other wise things take too long.
> -	    continue_to_exit_bp
> +	    return_if_fail [continue_to_exit_bp]
>   	    gdb_test "watch globalvar" "Watchpoint $decimal: globalvar"
>   
>   	    if { $multi_process == 0 && $cmd == "continue" } {
> @@ -304,7 +304,7 @@ proc test_detach_killed_outside {multi_process cmd} {
>   	}
>   
>   	# Run to _exit in the child.
> -	continue_to_exit_bp
> +	return_if_fail [continue_to_exit_bp]
>   
>   	set childpid [get_integer_valueof "mypid" -1]
>   	if { $childpid == -1 } {
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1b9179401c45..dfabc99fe4be 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -822,14 +822,14 @@ proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
>       set full_name "continue to breakpoint: $name"
>   
>       set kfail_pattern "Process record does not support instruction 0xfae64 at.*"
> -    gdb_test_multiple "continue" $full_name {
> +    return [gdb_test_multiple "continue" $full_name {
>   	-re "(?:Breakpoint|Temporary breakpoint) .* (at|in) $location_pattern\r\n$gdb_prompt $" {
>   	    pass $full_name
>   	}
>   	-re "(?:$kfail_pattern)\r\n$gdb_prompt $" {
>   	    kfail "gdb/25038" $full_name
>   	}
> -    }
> +    }]
>   }
>   
>   
>

Re: [PATCH] process-dies-while-detaching.exp: Exit early if GDB misses sync breakpoint

[Thiago Jung Bauermann]

Guinevere Larsen <blarsen@redhat.com> writes:

> On 29/09/2023 19:53, Thiago Jung Bauermann via Gdb-patches wrote:
>> I'm seeing a lot of variability in the failures of
>> gdb.threads/process-dies-while-detaching.exp on aarch64-linux.  On this
>> platform, a problem yet to be investigated causes GDB to miss the _exit
>> breakpoint.  What happens next is random because after missing that
>> breakpoint, GDB is out of sync with the inferior.  This causes the tests
>> following that point in the testcase to fail in a random way.
>>
>> In this scenario it's better to exit the testcase early to avoid random
>> results in the testsuite.
> Thanks for working on this test, it has been driving me up the wall for a while!

Yes, this caused a few false positives in our CI. :-/

>> We are relying on gdb_continue_to_breakpoint to return the result of
>> gdb_test_multiple.  This is already the case because in Tcl the return
>> value of a function is the return value of the last command it runs.  But
>> change gdb_continue_to_breakpoint to explicitly return this value, to make
>> it clear this is the intended behaviour.
> I like this change, since I didn't actually know that this was TCL's behavior!

Tcl works in mysterious ways. :-)

>> Tested on aarch64-linux.
>
> I tested on x86_64 and I see no regressions.
>
> Tested-By: Guinevere Larsen <blarsen@redhat.com>

Thank you for testing!

> I hope this gets approved soon!

Me too. :-)

-- 
Thiago

Re: [PATCH] process-dies-while-detaching.exp: Exit early if GDB misses sync breakpoint

[Andrew Burgess]

Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org>
writes:

> I'm seeing a lot of variability in the failures of
> gdb.threads/process-dies-while-detaching.exp on aarch64-linux.  On this
> platform, a problem yet to be investigated causes GDB to miss the _exit
> breakpoint.  What happens next is random because after missing that
> breakpoint, GDB is out of sync with the inferior.  This causes the tests
> following that point in the testcase to fail in a random way.
>
> In this scenario it's better to exit the testcase early to avoid random
> results in the testsuite.
>
> We are relying on gdb_continue_to_breakpoint to return the result of
> gdb_test_multiple.  This is already the case because in Tcl the return
> value of a function is the return value of the last command it runs.  But
> change gdb_continue_to_breakpoint to explicitly return this value, to make
> it clear this is the intended behaviour.
>
> Tested on aarch64-linux.

Looks great.  Thanks for improving this.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

> ---
>  .../gdb.threads/process-dies-while-detaching.exp       | 10 +++++-----
>  gdb/testsuite/lib/gdb.exp                              |  4 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> index bbbe82df30cd..c7d43d7dddcc 100644
> --- a/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> +++ b/gdb/testsuite/gdb.threads/process-dies-while-detaching.exp
> @@ -127,7 +127,7 @@ proc detach_and_expect_exit {inf_output_re test} {
>  
>  proc continue_to_exit_bp {} {
>      gdb_breakpoint "_exit" temporary
> -    gdb_continue_to_breakpoint "_exit" ".*_exit.*"
> +    return [gdb_continue_to_breakpoint "_exit" ".*_exit.*"]
>  }
>  
>  # If testing single-process, simply detach from the process.
> @@ -226,7 +226,7 @@ proc test_detach {multi_process cmd} {
>  	}
>  
>  	# Run to _exit in the child.
> -	continue_to_exit_bp
> +	return_if_fail [continue_to_exit_bp]
>  
>  	do_detach $multi_process $cmd "normal"
>      }
> @@ -265,13 +265,13 @@ proc test_detach_watch {wp multi_process cmd} {
>  	    # them out, and handle the case of the thread disappearing
>  	    # while doing that (on targets that need to detach from each
>  	    # thread individually).
> -	    continue_to_exit_bp
> +	    return_if_fail [continue_to_exit_bp]
>  	} else {
>  	    # Force software watchpoints.
>  	    gdb_test_no_output "set can-use-hw-watchpoints 0"
>  
>  	    # As above, but flip order, other wise things take too long.
> -	    continue_to_exit_bp
> +	    return_if_fail [continue_to_exit_bp]
>  	    gdb_test "watch globalvar" "Watchpoint $decimal: globalvar"
>  
>  	    if { $multi_process == 0 && $cmd == "continue" } {
> @@ -304,7 +304,7 @@ proc test_detach_killed_outside {multi_process cmd} {
>  	}
>  
>  	# Run to _exit in the child.
> -	continue_to_exit_bp
> +	return_if_fail [continue_to_exit_bp]
>  
>  	set childpid [get_integer_valueof "mypid" -1]
>  	if { $childpid == -1 } {
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1b9179401c45..dfabc99fe4be 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -822,14 +822,14 @@ proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
>      set full_name "continue to breakpoint: $name"
>  
>      set kfail_pattern "Process record does not support instruction 0xfae64 at.*"
> -    gdb_test_multiple "continue" $full_name {
> +    return [gdb_test_multiple "continue" $full_name {
>  	-re "(?:Breakpoint|Temporary breakpoint) .* (at|in) $location_pattern\r\n$gdb_prompt $" {
>  	    pass $full_name
>  	}
>  	-re "(?:$kfail_pattern)\r\n$gdb_prompt $" {
>  	    kfail "gdb/25038" $full_name
>  	}
> -    }
> +    }]
>  }
>  
>  

Re: [PATCH] process-dies-while-detaching.exp: Exit early if GDB misses sync breakpoint

[Thiago Jung Bauermann]

Andrew Burgess <aburgess@redhat.com> writes:

> Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org>
> writes:
>
>> I'm seeing a lot of variability in the failures of
>> gdb.threads/process-dies-while-detaching.exp on aarch64-linux.  On this
>> platform, a problem yet to be investigated causes GDB to miss the _exit
>> breakpoint.  What happens next is random because after missing that
>> breakpoint, GDB is out of sync with the inferior.  This causes the tests
>> following that point in the testcase to fail in a random way.
>>
>> In this scenario it's better to exit the testcase early to avoid random
>> results in the testsuite.
>>
>> We are relying on gdb_continue_to_breakpoint to return the result of
>> gdb_test_multiple.  This is already the case because in Tcl the return
>> value of a function is the return value of the last command it runs.  But
>> change gdb_continue_to_breakpoint to explicitly return this value, to make
>> it clear this is the intended behaviour.
>>
>> Tested on aarch64-linux.
>
> Looks great.  Thanks for improving this.
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>

Thank you! Pushed as commit 0f3efefb34f8.

-- 
Thiago