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

Tom de Vries
2025-04-11T10:27:21Z

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