Re: [PATCH v3] of [PATCH v2 42/47] gdb_test_multiple: Anchor prompt match if -lbl

Tom de Vries
2025-06-06T10:00:19Z

On 6/5/25 19:20, Pedro Alves wrote:
> On 2025-06-05 17:37, Pedro Alves wrote:
>> Reading back, I realized that this example looks a bit silly as is, because you could fix it by
>> just moving the "prefix " case earlier than the "prefix foo/bar" cases. I should make the command output
>> something after "prefix", like "prefix xxx", and then the regexp should match something like:
>>
>> -re "^\r\nprefix [^\r\n]*(?=\r\n)" {
>>
>> That's more like what the original testcase that exposed the issue does.
>>
>> I'll send an updated patch soon.
>
> Here it is.
>

Hi Pedro,

[ FWIW, the way this patch is posted doesn't work well with patchwork:
...
$ git-pw patch apply 113744
Failed to apply patch:
Applying: of [PATCH v2 42/47] gdb_test_multiple: Anchor prompt match if -lbl
error: No valid patches in input (allow with "--allow-empty")
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 of [PATCH v2 42/47] gdb_test_multiple: Anchor
prompt match if -lbl
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

$ git am --show-current-patch=diff
--- 8< ---
$
...

I managed to apply it by saving the email in thunderbird and using git
am, though that drops the commit message. ]

> --- 8< ---
> From 93ce02b9ad0aefefc5f71b6c5d23f565f3f2e87a Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Thu, 5 Jun 2025 18:09:44 +0100
> Subject: [PATCH] gdb_test_multiple: Anchor prompt match if -lbl
>
> The testcase added by this patch has a gdb_test_multiple call that
> wants to match different lines of output that all have a common
> prefix, and do different actions on each. Instead of a single regular
> expression with alternatives, its clearer code if the different

its -> it's

> expressions are handled with different "-re", like so:
>
> gdb_test_multiple "command" "" -lbl {
> -re "^command(?=\r\n)" {
> exp_continue
> }
> -re "^\r\nprefix foo(?=\r\n)" {
> # Some action for "foo".
> exp_continue
> }
> -re "^\r\nprefix bar(?=\r\n)" {
> # Some action for "bar".
> exp_continue
> }
> -re "^\r\nprefix \[^\r\n\]*(?=\r\n)" {
> # Some action for all others.
> exp_continue
> }
> -re "^\r\n$::gdb_prompt $" {
> gdb_assert {$all_prefixes_were_seen} $gdb_test_name
> }
> }
>
> Above, the leading anchors in the "^\r\nprefix..." matches are needed
> to avoid too-eager matching due to the common prefix. Without the
> anchors, if the expect output buffer happens to contain at least:
>
> "\r\nprefix xxx\r\nprefix foo\r\n"
>
> ... then the "prefix foo" pattern match inadvertently consumes the
> first "prefix xxx" line.
>
> Without the anchor in the prompt match, like:
>
> -re "\r\n$::gdb_prompt $" {
> gdb_assert {$all_prefixes_were_seen} $gdb_test_name
> }
>
> Or the equivalent:
>
> -re -wrap "" {
> gdb_assert {$all_prefixes_were_seen} $gdb_test_name
> }
>
> ... then if the expect buffer contains:
>
> "\r\nmeant-to-be-matched-by-lbl\r\nprefix foo\r\n$gdb_prompt "
>
> ... then the prompt regexp matches this, consuming the "prefix" line
> inadvertently, and we get a FAIL. The built-in regexp matcher for
> -lbl doesn't get a chance to match the
> "\r\nmeant-to-be-matched-by-lbl\r\n" part, because the built-in prompt
> match appears first within gdb_test_multiple.
>
> By adding the anchor to the prompt regexp, then we avoid that problem.
>

Should "then" be dropped in this line?

> However, the same expect output buffer contents will still match the
> built-in prompt match. That is what is fixed by this patch. It makes
> it so that if -lbl is specified, the built-in prompt regexp has a
> leading anchor.
>

Thanks for doing this.

LGTM.

Approved-By: Tom de Vries <tdevries@suse.de>

FWIW, I propose to apply this independently from the patch series, since
AFAICT it's unrelated.

Thanks,
- Tom

> Original idea for turning this into a gdb.testsuite/ testcase by Tom
> de Vries <tdevries@suse.de>.
>
> Change-Id: Ic2571ec793d856a89ee0d533ec363e2ac6036ea2
> ---
> .../gdb.testsuite/gdb_test_multiple-lbl.exp | 84 +++++++++++++++++++
> .../gdb.testsuite/gdb_test_multiple-lbl.gdb | 25 ++++++
> gdb/testsuite/lib/gdb.exp | 4 +-
> 3 files changed, 112 insertions(+), 1 deletion(-)
> create mode 100644 gdb/testsuite/gdb.testsuite/gdb_test_multiple-lbl.exp
> create mode 100755 gdb/testsuite/gdb.testsuite/gdb_test_multiple-lbl.gdb
>
> diff --git a/gdb/testsuite/gdb.testsuite/gdb_test_multiple-lbl.exp b/gdb/testsuite/gdb.testsuite/gdb_test_multiple-lbl.exp
> new file mode 100644
> index 00000000000..a05ce614b1d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.testsuite/gdb_test_multiple-lbl.exp
> @@ -0,0 +1,84 @@
> +# Copyright 2025 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test gdb_test_multiple -lbl, particularly with patterns that share a
> +# common prefix.
> +
> +standard_testfile
> +
> +clean_restart
> +
> +gdb_test_no_output "source ${srcdir}/${subdir}/$testfile.gdb" \
> + "source gdb test script"
> +
> +set saw_prompt 0
> +set saw_prefix 0
> +set saw_command 0
> +set saw_prefix_foo 0
> +set saw_prefix_bar 0
> +
> +# #1 - We need anchors so that the "prefix foo" pattern below does not
> +# match when the expect output buffer contains:
> +#
> +# "\r\nprefix xxx\r\n\prefix foo\r\n"
> +#
> +# #2 - We need an anchor on the prompt match as otherwise the prompt
> +# regexp would match:
> +#
> +# "\r\nmeant-to-be-matched-by-lbl-2\r\nprefix xxx\r\n(gdb) "
> +#
> +# This test would fail if -lbl did not force the built-in prompt match
> +# regexp to have an anchor as well, as without it, the built-in prompt
> +# regexp would have the exact same issue as #2 above.
> +
> +gdb_test_multiple "command" "" -lbl {
> + -re "^command(?=\r\n)" {
> + verbose -log <COMMAND>
> + incr saw_command
> + exp_continue
> + }
> + -re "^\r\nprefix foo(?=\r\n)" {
> + verbose -log <PREFIX-FOO>
> + incr saw_prefix_foo
> + exp_continue
> + }
> + -re "^\r\nprefix bar(?=\r\n)" {
> + verbose -log <PREFIX-BAR>
> + incr saw_prefix_bar
> + exp_continue
> + }
> + -re "^\r\nprefix \[^\r\n\]*(?=\r\n)" {
> + verbose -log <PREFIX>
> + incr saw_prefix
> + exp_continue
> + }
> + -re "^\r\n$gdb_prompt $" {
> + verbose -log <PROMPT>
> + incr saw_prompt
> + pass $gdb_test_name
> + }
> +}
> +
> +verbose -log "saw_command: $saw_command"
> +verbose -log "saw_prefix_foo: $saw_prefix_foo"
> +verbose -log "saw_prefix_bar: $saw_prefix_bar"
> +verbose -log "saw_prefix: $saw_prefix"
> +verbose -log "saw_prompt: $saw_prompt"
> +
> +gdb_assert {$saw_command == 1}
> +gdb_assert {$saw_prefix_foo == 1}
> +gdb_assert {$saw_prefix_bar == 1}
> +gdb_assert {$saw_prefix == 3}
> +gdb_assert {$saw_prompt == 1}
> diff --git a/gdb/testsuite/gdb.testsuite/gdb_test_multiple-lbl.gdb b/gdb/testsuite/gdb.testsuite/gdb_test_multiple-lbl.gdb
> new file mode 100755
> index 00000000000..8c94dfa9211
> --- /dev/null
> +++ b/gdb/testsuite/gdb.testsuite/gdb_test_multiple-lbl.gdb
> @@ -0,0 +1,25 @@
> +# Copyright 2025 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +define command
> + echo prefix xxx\n
> + echo meant-to-be-matched-by-lbl-1\n
> + echo prefix foo\n
> + echo prefix bar\n
> + echo meant-to-be-matched-by-lbl-2\n
> + echo prefix xxx\n
> + echo prefix xxx\n
> + echo meant-to-be-matched-by-lbl-3\n
> +end
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 0caa9cccd80..eefb6382a27 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -1127,6 +1127,7 @@ proc gdb_test_multiple { command message args } {
> global any_spawn_id
>
> set line_by_line 0
> + set lbl_anchor_re ""
> set prompt_regexp ""
> set prompt_anchor 1
> for {set i 0} {$i < [llength $args]} {incr i} {
> @@ -1136,6 +1137,7 @@ proc gdb_test_multiple { command message args } {
> set prompt_regexp [lindex $args $i]
> } elseif { $arg == "-lbl" } {
> set line_by_line 1
> + set lbl_anchor_re "^"
> } elseif { $arg == "-no-prompt-anchor" } {
> set prompt_anchor 0
> } else {
> @@ -1394,7 +1396,7 @@ proc gdb_test_multiple { command message args } {
> fail "$errmsg"
> set result -1
> }
> - -re "\r\n$prompt_regexp" {
> + -re "${lbl_anchor_re}\r\n$prompt_regexp" {
> if {![string match "" $message]} {
> fail "$message"
> }
>
> base-commit: f601ffb52199a883f16df385b73a14e756b3e19a