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

  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 | 29 ++++++-----------------
 1 file changed, 7 insertions(+), 22 deletions(-)
  

Oups,

I replied to the V2 before I saw a V3 was available and the V3 addresses
the point I raised!

Please discard my previous message, and FWIW this version looks good to me.

Reviewed-By: Lancelot Six <lancelot.six@amd.com>

On Wed, Feb 14, 2024 at 10:17:12AM +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 | 29 ++++++-----------------
>  1 file changed, 7 insertions(+), 22 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.threads/threadcrash.exp b/gdb/testsuite/gdb.threads/threadcrash.exp
> index 996e020d1e8..bf4534179e2 100644
> --- a/gdb/testsuite/gdb.threads/threadcrash.exp
> +++ b/gdb/testsuite/gdb.threads/threadcrash.exp
> @@ -20,26 +20,6 @@
>  # a gcore.
>  
>  
> -# Check that the inferior has 7 threads, and return the number of threads (7).
> -# We return the thread count so that, even if there is some error in the test,
> -# the final log doesn't get flooded with failures.
> -
> -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_assert {$thread_count == 7}
> -	}
> -    }
> -
> -    return $thread_count
> -}
> -
>  # Use 'thread apply all backtrace' to check if all expected threads
>  # are present, and stopped in the expected locations.  Set the global
>  # TEST_LIST to be the a list of regexps expected to match all the
> @@ -123,7 +103,8 @@ proc thread_apply_all {} {
>  
>  proc do_full_test {} {
>      global test_list
> -    set thread_count [test_thread_count]
> +    set thread_count [get_valueof "" "\$_inferior_thread_count" 0]
> +    gdb_assert {$thread_count == 7}
>  
>      thread_apply_all
>  
> @@ -230,4 +211,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
>
  

On 2/14/24 10:17, Guinevere Larsen wrote:
> -test_gcore
> +if { [is_aarch32_target] } {
> +    kfail "gcore tests fail on 32-bit arm, see PR corefiles/31294"
> +} else {
> +    test_gcore
> +}

The xfail and kfail procs don't have the same args:
...
$ egrep -d skip "proc (xfail|kfail)" /usr/share/dejagnu/*
/usr/share/dejagnu/framework.exp:proc xfail { message } {
/usr/share/dejagnu/framework.exp:proc kfail { bugid message } {
...
and consequently this patch runs into an error.

Furthermore, I don't run into the reported problem on my setup (pinebook 
aarch64 manjaro, chroot debian 32-bit), so this style of kfail actually 
prevents me from successfully running the tests.

I proposed a more precise fix in v4, though I can't test it ...

Thanks,
- Tom
  

On 3/8/24 11:01, Tom de Vries wrote:
> On 2/14/24 10:17, Guinevere Larsen wrote:
>> -test_gcore
>> +if { [is_aarch32_target] } {
>> +    kfail "gcore tests fail on 32-bit arm, see PR corefiles/31294"
>> +} else {
>> +    test_gcore
>> +}
> 
> The xfail and kfail procs don't have the same args:
> ...
> $ egrep -d skip "proc (xfail|kfail)" /usr/share/dejagnu/*
> /usr/share/dejagnu/framework.exp:proc xfail { message } {
> /usr/share/dejagnu/framework.exp:proc kfail { bugid message } {
> ...
> and consequently this patch runs into an error.
> 

And ... I just realized I made the same mistake in v4.  I already 
spotted another mistake, I intended to bail out after the kfail and 
forgot to add that.  I'll follow up on the patch once it arrives on the ml.

Thanks,
- Tom

> Furthermore, I don't run into the reported problem on my setup (pinebook 
> aarch64 manjaro, chroot debian 32-bit), so this style of kfail actually 
> prevents me from successfully running the tests.
> 
> I proposed a more precise fix in v4, though I can't test it ...
> 
> Thanks,
> - Tom