* [PATCH] [gdb/testsuite] Document return behavior in gdb_test_multiple
@ 2026-08-22 9:30 Tom de Vries
2026-08-27 14:25 ` Guinevere Larsen
0 siblings, 1 reply; 3+ messages in thread
From: Tom de Vries @ 2026-08-22 9:30 UTC (permalink / raw)
To: gdb-patches
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
---
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
--
2.51.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] [gdb/testsuite] Document return behavior in gdb_test_multiple 2026-08-22 9:30 [PATCH] [gdb/testsuite] Document return behavior in gdb_test_multiple Tom de Vries @ 2026-08-27 14:25 ` Guinevere Larsen 2026-09-02 16:25 ` Tom de Vries 0 siblings, 1 reply; 3+ messages in thread From: Guinevere Larsen @ 2026-08-27 14:25 UTC (permalink / raw) To: gdb-patches 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] [gdb/testsuite] Document return behavior in gdb_test_multiple 2026-08-27 14:25 ` Guinevere Larsen @ 2026-09-02 16:25 ` Tom de Vries 0 siblings, 0 replies; 3+ messages in thread From: Tom de Vries @ 2026-09-02 16:25 UTC (permalink / raw) To: Guinevere Larsen, gdb-patches 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 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-02 16:26 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-22 9:30 [PATCH] [gdb/testsuite] Document return behavior in gdb_test_multiple Tom de Vries 2026-08-27 14:25 ` Guinevere Larsen 2026-09-02 16:25 ` Tom de Vries
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox