Re: [PATCH] [gdb/testsuite] Document return behavior in gdb_test_multiple

[Guinevere Larsen <guinevere@redhat.com>]

On 8/22/26 6:30 AM, Tom de Vries wrote:
> There are a few places in gdb_test_multiple where "return -1" is used.
>
> The first two simply cause proc gdb_test_multiple to return -1.
>
> The other two are in implicit clauses dealing with eof.  Showing the first:
> ...
>      append code {
> 	...
> 	eof {
> 	    perror "GDB process no longer exists"
> 	    set wait_status [wait -i $gdb_spawn_id]
> 	    verbose -log "GDB process exited with wait status $wait_status"
> 	    if { $message != "" } {
> 		fail "$message"
> 	    }
> 	    # This does not return from gdb_test_multiple, but from the proc
> 	    # above it.
> 	    return -1
> 	}
> ...
>
> The result of this is not that gdb_test_multiple returns -1.  Instead, its
> caller does.  This is defined behavior, focused mainly on handling break and
> continue in such a way that it has effect on the caller in code like this [1]:
> ...
> for {set i 1} {$i <= 10} {incr i} {
>      gdb_test_multiple "print $i" "" {
>          -re -wrap " = 5" {break}
> 	-re -wrap ""     {}
>      }
> }
> ...
> and return is simply treated the same way.
>
> The next question is why only for eof we use "return -1" instead of
> "set result -1".
>
> It could be argued that this facilitates a style:
> ...
> gdb_test_multiple "<command 1>" "" {}
> gdb_test_multiple "<command 2>" "" {}
> ...
> where at command 2 we can assume that gdb is still running, without having to
> check for the result of command 1.
>
> But if that were the intention, it would make sense that gdb_test would have
> the same behavior, and it doesn't.
>
> For now, document the behavior at the eof returns in proc gdb_test_multiple
> and add two tests in a pre-existing test-case checking current behavior.
>
> [1] https://sourceware.org/pipermail/gdb-patches/2011-November/086792.html
> ---

Hi! Thanks for working on this! I love documentation patches.

I ran the new test and it works, so I think this is fine, but I think 
there should be a bit more, especially since the new test shows this as 
expected behavior.

I think there should be a note in the pre-function comment, perhaps 
around the "return value" section, mentioning that the gdb_test_multiple 
may make the proc calling it return early, and explaining the situation. 
That will make it easier to debug if this ever gets in the way.

--
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)

>   gdb/testsuite/gdb.testsuite/gdb-test.exp | 43 ++++++++++++++++++++++++
>   gdb/testsuite/lib/gdb.exp                |  7 ++++
>   2 files changed, 50 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.testsuite/gdb-test.exp b/gdb/testsuite/gdb.testsuite/gdb-test.exp
> index dab65f21bdb..8ae5325b5e6 100644
> --- a/gdb/testsuite/gdb.testsuite/gdb-test.exp
> +++ b/gdb/testsuite/gdb.testsuite/gdb-test.exp
> @@ -49,6 +49,49 @@ with_test_prefix "cmd with trailing control code" {
>       }
>   }
>   
> +proc quit {proc} {
> +    with_override perror nop {
> +	with_override fail nop {
> +	    if {$proc == "gdb_test_multiple"} {
> +		set res [gdb_test_multiple "quit" "" {}]
> +		# Not reached.
> +	    } elseif {$proc == "gdb_test"} {
> +		set res [gdb_test "quit"]
> +		# Reached.
> +	    }
> +	    return [expr {100 + $res}]
> +	}
> +    }
> +    # Not reached.
> +    return 200
> +}
> +
> +with_test_prefix "eof handling" {
> +    set res 300
> +    try {
> +	set res [quit gdb_test_multiple]
> +    } on return {result} {
> +    } finally {
> +	# Proc gdb_test_multiple does "return -1" on eof, which makes its
> +	# caller return.  Check that behavior.
> +	gdb_assert {$res == -1} "gdb_test_multiple"
> +    }
> +
> +    clean_restart
> +
> +    set res 300
> +    try {
> +	set res [quit gdb_test]
> +    } on return {result} {
> +    } finally {
> +	# Proc gdb_test_multiple does "return -1" on eof, which makes its caller
> +	# return, which is gdb_test.  Check that behavior.
> +	gdb_assert {$res == 99} "gdb_test"
> +    }
> +
> +    clean_restart
> +}
> +
>   # Change the prompt.
>   set prompt "(GDB) "
>   set prompt_re "\\(GDB\\) $"
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1a6438f36a5..15ec71cb542 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1522,6 +1522,8 @@ proc gdb_test_multiple { command message args } {
>   	    if { $message != "" } {
>   		fail "$message"
>   	    }
> +	    # This does not return from gdb_test_multiple, but from the proc
> +	    # above it.
>   	    return -1
>   	}
>       }
> @@ -1542,6 +1544,8 @@ proc gdb_test_multiple { command message args } {
>   	    if { $message != "" } {
>   		fail "$message"
>   	    }
> +	    # This does not return from gdb_test_multiple, but from the proc
> +	    # above it.
>   	    return -1
>   	}
>   	full_buffer {
> @@ -12417,6 +12421,9 @@ proc unprintable_to_octal { input_string } {
>       return $result
>   }
>   
> +# Ignore args and don't do anything.  Can be used with proc with_override.
> +proc nop {args} {}
> +
>   require {tcl_version_at_least 8 6 2}
>   
>   # Always load compatibility stuff.
>
> base-commit: 7c1f6faaf3bcd28a37d5f5a9737f6c1dcd0f13ef