[PATCH 0/2] [gdb/testsuite] Fixes in gdb.threads/inf-thr-count.exp

[PATCH 0/2] [gdb/testsuite] Fixes in gdb.threads/inf-thr-count.exp

[Tom de Vries]

I ran into a timeout in test-case gdb.threads/inf-thr-count.exp.

The first patch fixes this.

The second patch makes the test-case more readable.

Tested on x86_64-linux using make-check-all.sh.

Tom de Vries (2):
  [gdb/testsuite] Fix timeout in gdb.threads/inf-thr-count.exp
  [gdb/testsuite] Make gdb.threads/inf-thr-count.exp more readable

 gdb/testsuite/gdb.threads/inf-thr-count.exp | 57 +++++++++++++--------
 1 file changed, 35 insertions(+), 22 deletions(-)


base-commit: 637e0dfb04a6d3b0453c88f696ce053d978140e6
-- 
2.43.0

[PATCH 1/2] [gdb/testsuite] Fix timeout in gdb.threads/inf-thr-count.exp

[Tom de Vries]

With test-case gdb.threads/inf-thr-count.exp, check-readmore and
READMORE_SLEEP=1000 I run into:
...
(gdb) set variable spin = 0^M
(gdb) ^M
Thread 1 "inf-thr-count" hit Breakpoint 2, breakpt () at /data/vries/gdb/src/gdb/testsuite/gdb.threads/inf-thr-count.c:49^M
49      }^M
FAIL: gdb.threads/inf-thr-count.exp: set 'spin' flag to allow main thread to exit (timeout)
PASS: gdb.threads/inf-thr-count.exp: wait for main thread to stop
...

Fix this by using -no-prompt-anchor.

Tested on x86_64-linux.
---
 gdb/testsuite/gdb.threads/inf-thr-count.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.threads/inf-thr-count.exp b/gdb/testsuite/gdb.threads/inf-thr-count.exp
index d7a76874082..9f52655364a 100644
--- a/gdb/testsuite/gdb.threads/inf-thr-count.exp
+++ b/gdb/testsuite/gdb.threads/inf-thr-count.exp
@@ -163,7 +163,7 @@ gdb_test "with print thread-events on -- p \$_inferior_thread_count" \
 
 # Set a variable in the inferior, this will cause the second thread to
 # exit.
-gdb_test_no_output "set variable spin = 0" \
+gdb_test_no_output -no-prompt-anchor "set variable spin = 0" \
     "set 'spin' flag to allow main thread to exit"
 
 # When the second thread exits, the main thread joins with it, and
-- 
2.43.0

[PATCH 2/2] [gdb/testsuite] Make gdb.threads/inf-thr-count.exp more readable

[Tom de Vries]

While investigating a timeout in gdb.threads/inf-thr-count.exp I noticed that
it uses quite some escaping, resulting in hard-to-parse regexps like
"\\\$$::decimal".

Fix this by reducing the escaping using:
- quoting strings using {} instead of "", and
- string_to_regexp.

Also use multi_line to split up long multi-line regexps.

Tested on x86_64-linux.
---
 gdb/testsuite/gdb.threads/inf-thr-count.exp | 55 +++++++++++++--------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/inf-thr-count.exp b/gdb/testsuite/gdb.threads/inf-thr-count.exp
index 9f52655364a..1174326f2c6 100644
--- a/gdb/testsuite/gdb.threads/inf-thr-count.exp
+++ b/gdb/testsuite/gdb.threads/inf-thr-count.exp
@@ -43,7 +43,7 @@ if {[build_executable "failed to prepare" $testfile $srcfile \
 # Start GDB.  Ensure we are in non-stop mode as we need to read from
 # the inferior while it is running.
 save_vars {GDBFLAGS} {
-    append GDBFLAGS " -ex \"set non-stop on\""
+    append GDBFLAGS { -ex "set non-stop on"}
     clean_restart $binfile
 }
 
@@ -54,22 +54,20 @@ if ![runto_main] {
 gdb_breakpoint breakpt
 gdb_continue_to_breakpoint "first breakpt call"
 
+set re_var [string_to_regexp "$"]$decimal
+
 # Check we can see a single thread to begin with.
-gdb_test "p \$_inferior_thread_count" \
-    "^\\\$$::decimal = 1" \
-    "only one thread in \$_inferior_thread_count"
+gdb_test {p $_inferior_thread_count} \
+    "^$re_var = 1" \
+    {only one thread in $_inferior_thread_count}
 
 # We don't want thread events, it makes it harder to match GDB's
 # output.
 gdb_test_no_output "set print thread-events off"
 
 # Continue the program in the background.
-set test "continue&"
-gdb_test_multiple "continue&" $test {
-    -re "Continuing\\.\r\n$gdb_prompt " {
-	pass $test
-    }
-}
+gdb_test -no-prompt-anchor "continue&" \
+    [string_to_regexp "Continuing."]
 
 # Read the 'stage' flag from the inferior.  This is initially 0, but
 # will be set to 1 once the extra thread has been created, and then 2
@@ -88,8 +86,17 @@ proc wait_for_stage { num } {
     set failure_count 0
     set cmd "print /d stage"
     set stage_flag 0
+
+    set re_int [string_to_regexp "-"]?$::decimal
+
+    set re_msg \
+	[multi_line \
+	     "Cannot execute this command while the target is running" \
+	     {Use the "interrupt" command to stop the target} \
+	     [string_to_regexp "and then try again."]]
+
     gdb_test_multiple "$cmd" "wait for 'stage' flag to be $num" {
-	-re -wrap "^Cannot execute this command while the target is running\\.\r\nUse the \"interrupt\" command to stop the target\r\nand then try again\\." {
+	-re -wrap ^$re_msg {
 	    fail "$gdb_test_name (can't read asynchronously)"
 	    gdb_test_no_output "interrupt"
 
@@ -101,7 +108,7 @@ proc wait_for_stage { num } {
 	    }
 	}
 
-	-re -wrap "^\\$\[0-9\]* = (\[-\]*\[0-9\]*).*" {
+	-re -wrap "^$::re_var = ($re_int).*" {
 	    set stage_flag $expect_out(1,string)
 	    if {$stage_flag != $num} {
 		set stage_flag 0
@@ -131,8 +138,8 @@ if {![wait_for_stage 1]} {
 if {[target_info exists gdb_protocol]
     && ([target_info gdb_protocol] == "remote"
 	|| [target_info gdb_protocol] == "extended-remote")} {
-    set new_thread_re "\\\[New Thread \[^\r\n\]+\\\]\r\n"
-    set exit_thread_re "\\\[Thread \[^\r\n\]+ exited\\\]\r\n"
+    set new_thread_re {\[New Thread [^\r\n]+\]\r\n}
+    set exit_thread_re {\[Thread [^\r\n]+ exited\]\r\n}
 } else {
     set new_thread_re ""
     set exit_thread_re ""
@@ -141,9 +148,9 @@ if {[target_info exists gdb_protocol]
 # This is the test we actually care about.  Check that the
 # $_inferior_thread_count convenience variable shows the correct
 # thread count; the new thread should be visible.
-gdb_test "with print thread-events on -- p \$_inferior_thread_count" \
-    "^${new_thread_re}\\\$$::decimal = 2" \
-    "second thread visible in \$_inferior_thread_count"
+gdb_test {with print thread-events on -- p $_inferior_thread_count} \
+    "^${new_thread_re}$re_var = 2" \
+    {second thread visible in $_inferior_thread_count}
 
 # Set a variable in the inferior, this will cause the second thread to
 # exit.
@@ -157,9 +164,9 @@ if {![wait_for_stage 2]} {
 }
 
 # Check that the second thread has gone away.
-gdb_test "with print thread-events on -- p \$_inferior_thread_count" \
-    "^${exit_thread_re}\\\$$::decimal = 1" \
-    "back to one thread visible in \$_inferior_thread_count"
+gdb_test {with print thread-events on -- p $_inferior_thread_count} \
+    "^${exit_thread_re}$re_var = 1" \
+    {back to one thread visible in $_inferior_thread_count}
 
 # Set a variable in the inferior, this will cause the second thread to
 # exit.
@@ -168,8 +175,14 @@ gdb_test_no_output -no-prompt-anchor "set variable spin = 0" \
 
 # When the second thread exits, the main thread joins with it, and
 # then proceeds to hit the breakpt function again.
+set re_breakpt [string_to_regexp "breakpt ()"]
+set re \
+    [multi_line \
+	 "Thread 1 \[^\r\n\]+ hit Breakpoint $decimal, $re_breakpt\[^\r\n\]+" \
+	 "\[^\r\n\]+" \
+	 ""]
 gdb_test_multiple "" "wait for main thread to stop" {
-    -re "Thread 1 \[^\r\n\]+ hit Breakpoint $decimal, breakpt \\(\\)\[^\r\n\]+\r\n\[^\r\n\]+\r\n" {
+    -re $re {
 	pass $gdb_test_name
     }
 }
-- 
2.43.0

Re: [PATCH 2/2] [gdb/testsuite] Make gdb.threads/inf-thr-count.exp more readable

[Andreas Schwab]

On Apr 18 2025, Tom de Vries wrote:

> +    set re_int [string_to_regexp "-"]?$::decimal

Why do you need to use string_to_regexp for such a simple string?

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH 2/2] [gdb/testsuite] Make gdb.threads/inf-thr-count.exp more readable

[Tom de Vries]

On 4/18/25 11:39, Andreas Schwab wrote:
> On Apr 18 2025, Tom de Vries wrote:
> 
>> +    set re_int [string_to_regexp "-"]?$::decimal
> 
> Why do you need to use string_to_regexp for such a simple string?
> 

Hi Andreas,

thanks for the review.

Indeed, [string_to_regexp "-"] == "-", I didn't realize that.

I guess that using {[-]} comes from problems when using the minus at the 
start of a regexp:
...
% puts [regexp {-} {-}]
bad option "-": must be -all, -about, -indices, -inline, -expanded, 
-line, -linestop, -lineanchor, -nocase, -start, or --
% puts [regexp {[-]} {-}]
1
%
...
while this also works:
...
% puts [regexp {\-} {-}]
1
...

So perhaps string_to_regexp should also escape "-" to fix this failure:
...
set re [string_to_regexp "-"]
gdb_assert { [eval regexp $re "-"] }
...

Thanks,
- Tom

Re: [PATCH 2/2] [gdb/testsuite] Make gdb.threads/inf-thr-count.exp more readable

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> So perhaps string_to_regexp should also escape "-" to fix this failure:
Tom> ...
Tom> set re [string_to_regexp "-"]
Tom> gdb_assert { [eval regexp $re "-"] }
Tom> ...

IMO it's better to use "--" to separate switches from arguments, like

regexp -- $re ...

Tom

Re: [PATCH 2/2] [gdb/testsuite] Make gdb.threads/inf-thr-count.exp more readable

[Tom de Vries]

On 4/18/25 12:47, Tom de Vries wrote:
> On 4/18/25 11:39, Andreas Schwab wrote:
>> On Apr 18 2025, Tom de Vries wrote:
>>
>>> +    set re_int [string_to_regexp "-"]?$::decimal
>>
>> Why do you need to use string_to_regexp for such a simple string?
>>
> 
> Hi Andreas,
> 
> thanks for the review.
> 
> Indeed, [string_to_regexp "-"] == "-", I didn't realize that.
> 

Anyway, committed using "-" instead of [string_to_regexp "-"].

Thanks,
- Tom

> I guess that using {[-]} comes from problems when using the minus at the 
> start of a regexp:
> ...
> % puts [regexp {-} {-}]
> bad option "-": must be -all, -about, -indices, -inline, -expanded, - 
> line, -linestop, -lineanchor, -nocase, -start, or --
> % puts [regexp {[-]} {-}]
> 1
> %
> ...
> while this also works:
> ...
> % puts [regexp {\-} {-}]
> 1
> ...
> 
> So perhaps string_to_regexp should also escape "-" to fix this failure:
> ...
> set re [string_to_regexp "-"]
> gdb_assert { [eval regexp $re "-"] }
> ...
> 
> Thanks,
> - Tom

Re: [PATCH 2/2] [gdb/testsuite] Make gdb.threads/inf-thr-count.exp more readable

[Tom de Vries]

On 4/18/25 18:57, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> So perhaps string_to_regexp should also escape "-" to fix this failure:
> Tom> ...
> Tom> set re [string_to_regexp "-"]
> Tom> gdb_assert { [eval regexp $re "-"] }
> Tom> ...
> 
> IMO it's better to use "--" to separate switches from arguments, like
> 
> regexp -- $re ...

Hi Tom,

I think you're right, but that knowledge may be lacking in someone 
trying to get their regexp to match.

Anyway, I'll drop the patch I proposed ( 
https://sourceware.org/pipermail/gdb-patches/2025-April/217279.html ) 
until someone seconds it, since it's in the not-strictly-necessary category.

Thanks,
- Tom