Re: [PATCH v2] gdb/testsuite: fix gdb.base/dlmopen-ns-ids.exp racy test

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

On 4/10/25 15:01, Guinevere Larsen wrote:
> The recently included gdb.base/dlmopen-ns-ids.exp test can sometimes
> fail the call to get_integer_valueof when trying to check the namespace
> ID of the fourth dlopened SO, for apparently no reason.
> 
> What's happening is that the call to get_first_so_ns doesn't necessarily
> consume the GDB prompt, and so get_integer_valueof will see the prompt
> immediately and not find the value the test is looking for.
> 
> To fix this, the test was changed so that we consume all of the output
> of the command "info sharedlibrary", but only set the namespace ID for
> the first occurence of the SO we're looking for.  The command now also
> gets the solib name as a parameter, to reduce the amount of output.
> 

Hi Gwen,

LGTM.

I've also ran it through make-check-all.sh, and found no issues.

I did notice a typo, and indeed, the proposed codespell-log check 
confirmed it ( 
https://sourceware.org/pipermail/gdb-patches/2025-April/217118.html ):
...
codespell-log............................................................Passed
- hook id: codespell-log
- duration: 0.19s

codespell-log-internal...................................................Failed
- hook id: codespell
- exit code: 65

.git/COMMIT_EDITMSG:13: occurence ==> occurrence

...

OK with that fixed.

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

FWIW, I don't much like the style of regexp used, I find it hard to parse.

I prefer something more spaced out and less escape-heavy, like:
...
     set re1 [list "From" "To" "(NS" ")?Syms" "Read" "Shared Object 
Library"]
     set re1 [join $re1 {\s+}]

     set re_ns [list {\[\[} ($::decimal) {\]\]}]
     set re_ns [join $re_ns ""]
     set re2 [list "$::hex" "$::hex" $re_ns "\[^\r\n]+${lib_regexp}"]
     set re2 [join $re2 {\s+}]
...

But that's a pre-exising style issue, so I suppose it's better to not 
address this in this bug-fix commit.

Thanks,
- Tom

> Co-Authored-By: tdevries@suse.de
> ---
>   gdb/testsuite/gdb.base/dlmopen-ns-ids.exp | 20 +++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
> index 03b7a527af5..3ddc07e7773 100644
> --- a/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
> +++ b/gdb/testsuite/gdb.base/dlmopen-ns-ids.exp
> @@ -24,7 +24,8 @@ require allow_dlmopen_tests
>   standard_testfile -main.c -lib.c
>   
>   set srcfile_lib $srcfile2
> -set binfile_lib [standard_output_file dlmopen-lib.so]
> +set so_name dlmopen-lib.so
> +set binfile_lib [standard_output_file $so_name]
>   
>   if { [build_executable "build shlib" $binfile_lib $srcfile_lib \
>   	  [list debug shlib]] == -1 } {
> @@ -41,18 +42,19 @@ if { [build_executable "failed to build" $testfile $srcfile \
>   # for the so
>   proc get_first_so_ns {} {
>       set ns -1
> -    gdb_test_multiple "info sharedlibrary" "get SO namespace" -lbl {
> -	-re "From\\s+To\\s+\(NS\\s+\)?Syms\\s+Read\\s+Shared Object Library\r\n" {
> +    set lib_regexp [string_to_regexp ${::binfile_lib}]
> +    gdb_test_multiple "info sharedlibrary $::so_name" "get SO namespace" -lbl {
> +	-re "\r\nFrom\\s+To\\s+\(NS\\s+\)?Syms\\s+Read\\s+Shared Object Library(?=\r\n)" {
>   	    exp_continue
>   	}
> -	-re "^$::hex\\s+$::hex\\s+\\\[\\\[($::decimal)\\\]\\\]\\s+\[^\r\n]+$::binfile_lib.*" {
> -	    set ns $expect_out(1,string)
> -	}
> -	-re "^$::gdb_prompt $" {
> -	}
> -	-re "^\[^\r\n\]+\r\n" {
> +	-re "\r\n$::hex\\s+$::hex\\s+\\\[\\\[($::decimal)\\\]\\\]\\s+\[^\r\n]+${lib_regexp}(?=\r\n)" {
> +	    if {$ns == -1} {
> +		set ns $expect_out(1,string)
> +	    }
>   	    exp_continue
>   	}
> +	-re -wrap "" {
> +	}
>       }
>       return $ns
>   }
> 
> base-commit: d1458933830456e54223d9fc61f0d9b3a19256f5