[PATCH v2 0/2] Fixes to gdb.threads/threadcrash.exp

[PATCH v2 0/2] Fixes to gdb.threads/threadcrash.exp

[Guinevere Larsen]

The test was recently pushed to master but it had a couple of small
issues. This is a quick series to fix the 2 major issues that were
brought up to my attention.

Changes for v2:
* changed xfail from patch 1 to a kfail

Guinevere Larsen (2):
  gdb/testsuite: fix gdb.threads/threadcrash.exp on 32-bit arm targets
  gdb/testsuite: reduce gdb.threads/threadcrash.exp reliance on libc
    symbols

 gdb/testsuite/gdb.threads/threadcrash.exp | 32 +++++++++++------------
 1 file changed, 15 insertions(+), 17 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] gdb/testsuite: fix gdb.threads/threadcrash.exp on 32-bit arm targets

[Guinevere Larsen]

There are 2 issues with the test gdb.threads/threadcrash.exp on arm
targets, both relating to issues in how the targets handles gcores. The
first is that the test fails to cout the number of threads in the
inferior and the second is that GDB can't properly backtrace from a
gcore.

The first error is fixed on this commit by getting the convenience
variable _inferior_thread_count as opposed to calculating it based on
the output of "info threads"

For the second, this test just emits a single xfail referring back to PR
corefiles/31294, which tracks the issues with gcores in 32-bit arm
targets.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31294
---
 gdb/testsuite/gdb.threads/threadcrash.exp | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/threadcrash.exp b/gdb/testsuite/gdb.threads/threadcrash.exp
index 996e020d1e8..6c638838043 100644
--- a/gdb/testsuite/gdb.threads/threadcrash.exp
+++ b/gdb/testsuite/gdb.threads/threadcrash.exp
@@ -27,12 +27,9 @@
 proc test_thread_count {} {
     set thread_count 0
 
-    gdb_test_multiple "info threads" "getting thread count" -lbl {
-	-re "Thread" {
-	    incr thread_count
-	    exp_continue
-	}
-	-re "$::gdb_prompt " {
+    gdb_test_multiple "print \$_inferior_thread_count" "getting thread count" {
+	-re ".* = (\[0-9]+).*" {
+	    set thread_count $expect_out(1,string)
 	    gdb_assert {$thread_count == 7}
 	}
     }
@@ -230,4 +227,8 @@ test_live_inferior
 
 test_corefile
 
-test_gcore
+if { [is_aarch32_target] } {
+    kfail "gcore tests fail on 32-bit arm, see PR corefiles/31294"
+} else {
+    test_gcore
+}
-- 
2.43.0

Re: [PATCH v2 1/2] gdb/testsuite: fix gdb.threads/threadcrash.exp on 32-bit arm targets

[Lancelot SIX]

Hi Gwen,

On Tue, Feb 13, 2024 at 12:09:24PM +0100, Guinevere Larsen wrote:
> There are 2 issues with the test gdb.threads/threadcrash.exp on arm
> targets, both relating to issues in how the targets handles gcores. The
> first is that the test fails to cout the number of threads in the
> inferior and the second is that GDB can't properly backtrace from a
> gcore.
> 
> The first error is fixed on this commit by getting the convenience
> variable _inferior_thread_count as opposed to calculating it based on
> the output of "info threads"
> 
> For the second, this test just emits a single xfail referring back to PR
> corefiles/31294, which tracks the issues with gcores in 32-bit arm
> targets.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31294
> ---
>  gdb/testsuite/gdb.threads/threadcrash.exp | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.threads/threadcrash.exp b/gdb/testsuite/gdb.threads/threadcrash.exp
> index 996e020d1e8..6c638838043 100644
> --- a/gdb/testsuite/gdb.threads/threadcrash.exp
> +++ b/gdb/testsuite/gdb.threads/threadcrash.exp
> @@ -27,12 +27,9 @@
>  proc test_thread_count {} {
>      set thread_count 0
>  
> -    gdb_test_multiple "info threads" "getting thread count" -lbl {
> -	-re "Thread" {
> -	    incr thread_count
> -	    exp_continue
> -	}
> -	-re "$::gdb_prompt " {
> +    gdb_test_multiple "print \$_inferior_thread_count" "getting thread count" {
> +	-re ".* = (\[0-9]+).*" {
> +	    set thread_count $expect_out(1,string)
>  	    gdb_assert {$thread_count == 7}
>  	}
>      }

Out of the box, this fails for me

    Running /home/lancesix/src/worktrees/gdb/upstream-master/_build/gdb/testsuite/../../../gdb/testsuite/gdb.threads/threadcrash.exp ...
    FAIL: gdb.threads/threadcrash.exp: test_live_inferior: $thread_count == [llength $test_list]
    FAIL: gdb.threads/threadcrash.exp: test_corefile: $thread_count == [llength $test_list]
    FAIL: gdb.threads/threadcrash.exp: test_gcore: $thread_count == [llength $test_list]

Also, I think you could use get_valueof here, to make things simpler,
and avoid the issue of not not consuming the entire exp buffer:

    proc test_thread_count {} {
        set thread_count [get_valueof /d \$_inferior_thread_count 0]
        gdb_assert {$thread_count == 7}
        return $thread_count
    }

Best,
Lancelot.

> @@ -230,4 +227,8 @@ test_live_inferior
>  
>  test_corefile
>  
> -test_gcore
> +if { [is_aarch32_target] } {
> +    kfail "gcore tests fail on 32-bit arm, see PR corefiles/31294"
> +} else {
> +    test_gcore
> +}
> 
> -- 
> 2.43.0
> 

[PATCH v2 2/2] gdb/testsuite: reduce gdb.threads/threadcrash.exp reliance on libc symbols

[Guinevere Larsen]

The test gdb.threads/threadcrash.exp demanded GDB to fully unwind and
print the names of all functions. However, some of the functions are
from the libc library, and so the test implicitly demanded libc symbols
to be available, and would fail otherwise, as was raised in PR
gdb/31293.

This commit changes it so we only explicitly check for functions that
are provided by threadcrash.c to fix that

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31293
---
 gdb/testsuite/gdb.threads/threadcrash.exp | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/threadcrash.exp b/gdb/testsuite/gdb.threads/threadcrash.exp
index 6c638838043..3905ad6f936 100644
--- a/gdb/testsuite/gdb.threads/threadcrash.exp
+++ b/gdb/testsuite/gdb.threads/threadcrash.exp
@@ -57,26 +57,23 @@ proc thread_apply_all {} {
 		exp_continue
 	    }
 	    -re "\[^\n\]*syscall_task .location=SIGNAL_ALT_STACK\[^\n\]*" {
-		lappend test_list [multi_line ".*sleep.*" \
-					      ".*do_syscall_task .location=SIGNAL_ALT_STACK.*" \
+		lappend test_list [multi_line ".*do_syscall_task .location=SIGNAL_ALT_STACK.*" \
 					      ".*signal_handler.*" \
 					      ".*signal handler called.*" \
-					      ".*pthread_kill.*" \
+					      ".*" \
 					      ".*thread_function.*"]
 		exp_continue
 	    }
 	    -re "\[^\n\]*syscall_task .location=SIGNAL_HANDLER\[^\n\]*" {
-		lappend test_list [multi_line ".*sleep.*" \
-					      ".*do_syscall_task .location=SIGNAL_HANDLER.*" \
+		lappend test_list [multi_line ".*do_syscall_task .location=SIGNAL_HANDLER.*" \
 					      ".*signal_handler.*" \
 					      ".*signal handler called.*" \
-					      ".*pthread_kill.*" \
+					      ".*" \
 					      ".*thread_function.*"]
 		exp_continue
 	    }
 	    -re "\[^\n\]*syscall_task .location=NORMAL\[^\n\]*" {
-		lappend test_list [multi_line ".*sleep.*" \
-					      ".*do_syscall_task .location=NORMAL.*" \
+		lappend test_list [multi_line ".*do_syscall_task .location=NORMAL.*" \
 					      ".*thread_function.*"]
 		exp_continue
 	    }
@@ -84,7 +81,7 @@ proc thread_apply_all {} {
 		lappend test_list [multi_line ".*do_spin_task .location=SIGNAL_ALT_STACK.*" \
 					      ".*signal_handler.*" \
 					      ".*signal handler called.*" \
-					      ".*pthread_kill.*" \
+					      ".*" \
 					      ".*thread_function.*"]
 		exp_continue
 	    }
@@ -92,7 +89,7 @@ proc thread_apply_all {} {
 		lappend test_list [multi_line ".*do_spin_task .location=SIGNAL_HANDLER.*" \
 					      ".*signal_handler.*" \
 					      ".*signal handler called.*" \
-					      ".*pthread_kill.*" \
+					      ".*" \
 					      ".*thread_function.*"]
 		exp_continue
 	    }
-- 
2.43.0