gdb/testsuite: make gdb_get_worker_threads reads default number of threads
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-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
Commit Message
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(+)
Comments
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
>>>>> "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
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)
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
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.
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
@@ -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)
}