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

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

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