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

[Tom de Vries <tdevries@suse.de>]

On 8/27/26 4:25 PM, Guinevere Larsen wrote:
> 
> 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.
> 

Hi Guinevere,

thanks for the review.

Good point, I've added a comment.

I also noticed that the test that I added doesn't work with remote host, 
so I've disabled that.

Pushed with those changes.

Thanks,
- Tom


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