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

Message ID 20240214091712.223928-4-blarsen@redhat.com
State New
Headers
Series Fixes to gdb.threads/threadcrash.exp |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Testing failed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Guinevere Larsen Feb. 14, 2024, 9:17 a.m. UTC
  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(-)
  

Comments

Tom de Vries March 6, 2024, 5:16 p.m. UTC | #1
On 2/14/24 10:17, Guinevere Larsen wrote:
> 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

Hi Gwen,

Nit: Missing dot at end of line.

With that fixed, LGTM.

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

Thanks,
- Tom

> 
> 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 bf4534179e2..2bbedcce58e 100644
> --- a/gdb/testsuite/gdb.threads/threadcrash.exp
> +++ b/gdb/testsuite/gdb.threads/threadcrash.exp
> @@ -40,26 +40,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
>   	    }
> @@ -67,7 +64,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
>   	    }
> @@ -75,7 +72,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
>   	    }
  
Guinevere Larsen March 7, 2024, 9:11 a.m. UTC | #2
On 06/03/2024 18:16, Tom de Vries wrote:
> On 2/14/24 10:17, Guinevere Larsen wrote:
>> 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
>
> Hi Gwen,
>
> Nit: Missing dot at end of line.
>
> With that fixed, LGTM.
>
> Approved-By: Tom de Vries <tdevries@suse.de>

Thanks for the review!

Would this apply to both patches or just this second one?
  
Tom de Vries March 7, 2024, 6:23 p.m. UTC | #3
On 3/7/24 10:11, Guinevere Larsen wrote:
> On 06/03/2024 18:16, Tom de Vries wrote:
>> On 2/14/24 10:17, Guinevere Larsen wrote:
>>> 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
>>
>> Hi Gwen,
>>
>> Nit: Missing dot at end of line.
>>
>> With that fixed, LGTM.
>>
>> Approved-By: Tom de Vries <tdevries@suse.de>
> 
> Thanks for the review!
> 
> Would this apply to both patches or just this second one?
> 

Just this one, but after looking at the other patch, I realize also this 
patch is not ideal: if there are libc symbols for sleep and pthread_kill 
in the live_inferior case, they also should be there for the other cases.

I'm working on a patch series based on yours, addressing this and a few 
more issues, hoping to submit tomorrow.

Thanks,
- Tom
  

Patch

diff --git a/gdb/testsuite/gdb.threads/threadcrash.exp b/gdb/testsuite/gdb.threads/threadcrash.exp
index bf4534179e2..2bbedcce58e 100644
--- a/gdb/testsuite/gdb.threads/threadcrash.exp
+++ b/gdb/testsuite/gdb.threads/threadcrash.exp
@@ -40,26 +40,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
 	    }
@@ -67,7 +64,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
 	    }
@@ -75,7 +72,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
 	    }