[PATCH] gdb/testsuite: make gdb_get_worker_threads reads default number of threads

[PATCH] gdb/testsuite: make gdb_get_worker_threads reads default number of threads

[Guinevere Larsen]

When a test in the testsuite attempts to collect the amount of worker
threads GDB is able to use, it calls the proc gdb_get_worker_threads,
which could understand if the number was unlimited or when it had been
explicitly defined to a number, but it was unable to understand the
default message. That lead to the following TCL error in some
situations:

ERROR: -------------------------------------------
ERROR: in testcase <path>
ERROR:  invalid bareword "UNKNOWN"
in expression "UNKNOWN / 2";
should be "$UNKNOWN" or "{UNKNOWN}" or "UNKNOWN(...)" or ...
(...)

One such example is the current buildbot instance (at least the clang
version). This commit adds a new clause to the gdb_get_worker_threads
that detects the default worker thread message.
---
 gdb/testsuite/lib/gdb.exp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index d0990dcfe0e..148476cf42d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -10034,6 +10034,9 @@ proc gdb_get_worker_threads { {testname ""} } {
 	-wrap -re "^The number of worker threads GDB can use is unlimited \\(currently ($::decimal)\\)\\." {
 	    set worker_threads $expect_out(1,string)
 	}
+	-wrap -re "^The number of worker threads GDB can use is the default \\(currently ($::decimal)\\)\\." {
+	    set worker_threads $expect_out(1,string)
+	}
 	-wrap -re "^The number of worker threads GDB can use is ($::decimal)\\." {
 	    set worker_threads $expect_out(1,string)
 	}
-- 
2.41.0

Re: [PATCH] gdb/testsuite: make gdb_get_worker_threads reads default number of threads

[Keith Seitz]

Hi,

On 12/5/23 07:26, Guinevere Larsen wrote:

> One such example is the current buildbot instance (at least the clang
> version). This commit adds a new clause to the gdb_get_worker_threads
> that detects the default worker thread message.
> ---
>   gdb/testsuite/lib/gdb.exp | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index d0990dcfe0e..148476cf42d 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -10034,6 +10034,9 @@ proc gdb_get_worker_threads { {testname ""} } {
>   	-wrap -re "^The number of worker threads GDB can use is unlimited \\(currently ($::decimal)\\)\\." {
>   	    set worker_threads $expect_out(1,string)
>   	}
> +	-wrap -re "^The number of worker threads GDB can use is the default \\(currently ($::decimal)\\)\\." {
> +	    set worker_threads $expect_out(1,string)
> +	}
>   	-wrap -re "^The number of worker threads GDB can use is ($::decimal)\\." {
>   	    set worker_threads $expect_out(1,string)
>   	}

This is new:

33ae45434d0ab1f7de365b9140ad4e4ffc34b8a2
Date:   Mon Dec 4 14:23:17 2023 +0000

     gdb: Enable early init of thread pool size

That commit changes the "is unlimited" text from 
maintenance_show_worker_threads to "is the default":

        gdb_printf (file, _("The number of worker threads GDB "
-                         "can use is unlimited (currently %zu).\n"),
+                         "can use is the default (currently %zu).\n"),
                   gdb::thread_pool::g_thread_pool->thread_count ());

So this patch can be further simplified. Otherwise LGTM.

Reviewed-by: Keith Seitz <keiths@redhat.com>

Keith

Re: [PATCH] gdb/testsuite: make gdb_get_worker_threads reads default number of threads

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

Guinevere> When a test in the testsuite attempts to collect the amount of worker
Guinevere> threads GDB is able to use, it calls the proc gdb_get_worker_threads,
Guinevere> which could understand if the number was unlimited or when it had been
Guinevere> explicitly defined to a number, but it was unable to understand the
Guinevere> default message. That lead to the following TCL error in some
Guinevere> situations:

Thanks for the patch..  I think this was fixed yesterday by:

commit 66e00622a89a0fd1c37539a72c6cdd7c6bd334e7
Author: Richard Bunt <richard.bunt@linaro.org>
Date:   Tue Dec 5 09:54:12 2023 +0000

    gdb/testsuite: Update worker thread show assertion

Tom

Re: [PATCH] gdb/testsuite: make gdb_get_worker_threads reads default number of threads

[Guinevere Larsen]

On 06/12/2023 16:39, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
> Guinevere> When a test in the testsuite attempts to collect the amount of worker
> Guinevere> threads GDB is able to use, it calls the proc gdb_get_worker_threads,
> Guinevere> which could understand if the number was unlimited or when it had been
> Guinevere> explicitly defined to a number, but it was unable to understand the
> Guinevere> default message. That lead to the following TCL error in some
> Guinevere> situations:
>
> Thanks for the patch..  I think this was fixed yesterday by:
>
> commit 66e00622a89a0fd1c37539a72c6cdd7c6bd334e7
> Author: Richard Bunt <richard.bunt@linaro.org>
> Date:   Tue Dec 5 09:54:12 2023 +0000
>
>      gdb/testsuite: Update worker thread show assertion
>
> Tom
>
Yes, You approved that fix just a bit before I sent this one. Either 
way, I'm glad its fixed :)

(i didn't push this one because of that)

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

Re: [PATCH] gdb/testsuite: make gdb_get_worker_threads reads default number of threads

[Andrew Burgess]

Keith Seitz <keiths@redhat.com> writes:

> Hi,
>
> On 12/5/23 07:26, Guinevere Larsen wrote:
>
>> One such example is the current buildbot instance (at least the clang
>> version). This commit adds a new clause to the gdb_get_worker_threads
>> that detects the default worker thread message.
>> ---
>>   gdb/testsuite/lib/gdb.exp | 3 +++
>>   1 file changed, 3 insertions(+)
>> 
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index d0990dcfe0e..148476cf42d 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -10034,6 +10034,9 @@ proc gdb_get_worker_threads { {testname ""} } {
>>   	-wrap -re "^The number of worker threads GDB can use is unlimited \\(currently ($::decimal)\\)\\." {
>>   	    set worker_threads $expect_out(1,string)
>>   	}
>> +	-wrap -re "^The number of worker threads GDB can use is the default \\(currently ($::decimal)\\)\\." {
>> +	    set worker_threads $expect_out(1,string)
>> +	}
>>   	-wrap -re "^The number of worker threads GDB can use is ($::decimal)\\." {
>>   	    set worker_threads $expect_out(1,string)
>>   	}
>
> This is new:
>
> 33ae45434d0ab1f7de365b9140ad4e4ffc34b8a2
> Date:   Mon Dec 4 14:23:17 2023 +0000
>
>      gdb: Enable early init of thread pool size
>
> That commit changes the "is unlimited" text from 
> maintenance_show_worker_threads to "is the default":
>
>         gdb_printf (file, _("The number of worker threads GDB "
> -                         "can use is unlimited (currently %zu).\n"),
> +                         "can use is the default (currently %zu).\n"),
>                    gdb::thread_pool::g_thread_pool->thread_count ());
>
> So this patch can be further simplified. Otherwise LGTM.

Indeed.  This was my mistake for not retesting after the final rebase.
As Keith said s/unlimited/the default/ instead of adding a whole new
regexp, and this is good to go.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

Re: [PATCH] gdb/testsuite: make gdb_get_worker_threads reads default number of threads

[Richard Bunt]

On 06/12/2023 15:44, Guinevere Larsen wrote:
> On 06/12/2023 16:39, Tom Tromey wrote:
>>>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:
>> Guinevere> When a test in the testsuite attempts to collect the amount 
>> of worker
>> Guinevere> threads GDB is able to use, it calls the proc 
>> gdb_get_worker_threads,
>> Guinevere> which could understand if the number was unlimited or when 
>> it had been
>> Guinevere> explicitly defined to a number, but it was unable to 
>> understand the
>> Guinevere> default message. That lead to the following TCL error in some
>> Guinevere> situations:
>>
>> Thanks for the patch..  I think this was fixed yesterday by:
>>
>> commit 66e00622a89a0fd1c37539a72c6cdd7c6bd334e7
>> Author: Richard Bunt <richard.bunt@linaro.org>
>> Date:   Tue Dec 5 09:54:12 2023 +0000
>>
>>      gdb/testsuite: Update worker thread show assertion
>>
>> Tom
>>
> Yes, You approved that fix just a bit before I sent this one. Either 
> way, I'm glad its fixed :)
> 
> (i didn't push this one because of that)
> 

Sorry for the noise on this one. My internal CI marked this test as 
UNSUPPORTED so I missed it. I will improve my internal set up.

Re: [PATCH] gdb/testsuite: make gdb_get_worker_threads reads default number of threads

[Tom de Vries]

On 12/6/23 17:22, Andrew Burgess wrote:
> Keith Seitz <keiths@redhat.com> writes:
> 
>> Hi,
>>
>> On 12/5/23 07:26, Guinevere Larsen wrote:
>>
>>> One such example is the current buildbot instance (at least the clang
>>> version). This commit adds a new clause to the gdb_get_worker_threads
>>> that detects the default worker thread message.
>>> ---
>>>    gdb/testsuite/lib/gdb.exp | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>>> index d0990dcfe0e..148476cf42d 100644
>>> --- a/gdb/testsuite/lib/gdb.exp
>>> +++ b/gdb/testsuite/lib/gdb.exp
>>> @@ -10034,6 +10034,9 @@ proc gdb_get_worker_threads { {testname ""} } {
>>>    	-wrap -re "^The number of worker threads GDB can use is unlimited \\(currently ($::decimal)\\)\\." {
>>>    	    set worker_threads $expect_out(1,string)
>>>    	}
>>> +	-wrap -re "^The number of worker threads GDB can use is the default \\(currently ($::decimal)\\)\\." {
>>> +	    set worker_threads $expect_out(1,string)
>>> +	}
>>>    	-wrap -re "^The number of worker threads GDB can use is ($::decimal)\\." {
>>>    	    set worker_threads $expect_out(1,string)
>>>    	}
>>
>> This is new:
>>
>> 33ae45434d0ab1f7de365b9140ad4e4ffc34b8a2
>> Date:   Mon Dec 4 14:23:17 2023 +0000
>>
>>       gdb: Enable early init of thread pool size
>>
>> That commit changes the "is unlimited" text from
>> maintenance_show_worker_threads to "is the default":
>>
>>          gdb_printf (file, _("The number of worker threads GDB "
>> -                         "can use is unlimited (currently %zu).\n"),
>> +                         "can use is the default (currently %zu).\n"),
>>                     gdb::thread_pool::g_thread_pool->thread_count ());
>>
>> So this patch can be further simplified. Otherwise LGTM.
> 
> Indeed.  This was my mistake for not retesting after the final rebase.
> As Keith said s/unlimited/the default/ instead of adding a whole new
> regexp, and this is good to go.
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>

FTR, AFAICT this already has been fixed by commit 66e00622a89 
("gdb/testsuite: Update worker thread show assertion").

Thanks,
- Tom