Re: [PATCH v3] gdb/testsuite: Avoid infinite loop in gdb.reverse/step-reverse.exp

[Bruno Larsen <blarsen@redhat.com>]

On 06/06/2023 01:28, Thiago Jung Bauermann wrote:
> This testcase sometimes gets stuck in a loop for hours when running in our
> CI.  The problem is that due to an issue unrelated to reverse debugging the
> inferior exits early, and because of the overly generic ".*" pattern the
> testcase keeps sending the "next" command without noticing that the
> inferior is gone.
>
> gdb_test_multiple has a pattern to detect that "The program is not being
> run.", but since it is placed after the patterns from the caller it won't
> be triggered.  It also has a timeout pattern but because it is triggered
> between successful matches, each time the test matches the '-re -wrap ".*"'
> this counts as a successful match and the timeout is reset.
>
> Since the test binary is compiled with debug information, fix by changing
> one of the generic patterns to match entering the main function and the
> other one to match the source code line number that is shown by GDB right
> after the "step" command.
>
> Also, as a precaution add a maximum number of times the "next" command will
> be sent.
>
> Co-Authored-By: Tom de Vries <tdevries@suse.de>
> ---

Thanks for fixing all the nits, looks good to me now. Reviewed-By: Bruno 
Larsen <blarsen@redhat.com>

(Just making sure you know, the rb tag is not enough to push, you need 
an approval)

-- 
Cheers,
Bruno

>
> Changes since v2:
> - Put "^main" in the correct re in "$step_out == 1" case (pointed out by Bruno).
>
> Changes since v1:
> - Added maximum number of iterations (code provided by Tom).
> - Changed one of the patterns to match the main function (suggested by Bruno).
> - Clarified why the timeout pattern isn't effective (explanation from Andrew).
>
> Tom,
>
> Most of the patch now is the iteration-counting code. Since I copied it
> from your email, I thought it made sense to record you as co-author.
> I hope it's ok.
>
>   gdb/testsuite/gdb.reverse/step-reverse.exp | 28 +++++++++++++++++++---
>   1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.reverse/step-reverse.exp b/gdb/testsuite/gdb.reverse/step-reverse.exp
> index 729218d4cb8c..4b78a8f8fb75 100644
> --- a/gdb/testsuite/gdb.reverse/step-reverse.exp
> +++ b/gdb/testsuite/gdb.reverse/step-reverse.exp
> @@ -247,6 +247,7 @@ gdb_test_multiple "step" "$test_message" {
>   gdb_test "next" ".*NEXT OVER THIS CALL.*" "reverse next over call"
>   
>   set step_out 0
> +set max_iterations 1000
>   gdb_test_multiple "next" "reverse next over recursion" {
>       -re -wrap ".*NEXT OVER THIS RECURSION.*" {
>   	pass "$gdb_test_name"
> @@ -257,11 +258,19 @@ gdb_test_multiple "next" "reverse next over recursion" {
>       }
>   }
>   if { "$step_out" == 1 } {
> +    set iterations 0
>       gdb_test_multiple "next" "stepping out of recursion" {
> -	-re -wrap "NEXT OVER THIS RECURSION.*" {
> +	-re -wrap "^main.*NEXT OVER THIS RECURSION.*" {
>   	    set step_out 0
> +	    pass "$gdb_test_name"
>   	}
> -	-re -wrap ".*" {
> +	-re -wrap "^\[0-9\].*" {
> +	    incr iterations
> +	    if { $iterations == $max_iterations } {
> +		fail "$gdb_test_name (reached $max_iterations iterations)"
> +		return
> +	    }
> +
>   	    send_gdb "next\n"
>   	    exp_continue
>   	}
> @@ -276,9 +285,16 @@ gdb_test_no_output "set exec-dir reverse" "reverse again to test recursion"
>   
>   gdb_test "step" ".*EXIT RECURSIVE FUNCTION.*" "enter recursive function"
>   set seen_recursive_call 0
> +set iterations 0
>   gdb_test_multiple "next" "step over recursion inside the recursion" {
>       -re -wrap ".*RECURSIVE CALL.*" {
>   	incr seen_recursive_call
> +	incr iterations
> +	if { $iterations == $max_iterations } {
> +	    fail "$gdb_test_name (reached $max_iterations iterations)"
> +	    return
> +	}
> +
>   	send_gdb "next\n"
>   	exp_continue
>       }
> @@ -286,7 +302,13 @@ gdb_test_multiple "next" "step over recursion inside the recursion" {
>   	gdb_assert {"$seen_recursive_call" == 1} \
>   	    "step over recursion inside the recursion"
>       }
> -    -re -wrap ".*" {
> +    -re -wrap "^\[0-9\].*" {
> +	incr iterations
> +	if { $iterations == $max_iterations } {
> +	    fail "$gdb_test_name (reached $max_iterations iterations)"
> +	    return
> +	}
> +
>   	send_gdb "next\n"
>   	exp_continue
>       }
>
> base-commit: 3c5e824b9cee93a987a77906240c509add260a0d
>