Re: [PATCH v3] gdb tests: Allow for "LWP" in thread IDs from info threads.

[John Baldwin <jhb@FreeBSD.org>]

On 5/12/23 7:15 AM, John Baldwin wrote:
> On 5/9/23 1:56 PM, John Baldwin wrote:
>> On 5/9/23 12:01 PM, Simon Marchi wrote:
>>> On 4/18/23 16:13, John Baldwin wrote:
>>>> Several tests assume that the first word after a thread ID in 'info
>>>> threads' output is "Thread".  However, several targets use "LWP"
>>>> instead such as the FreeBSD and NetBSD native targets.  The Linux
>>>> native target also uses "LWP" if libthread_db is not being used.
>>>>
>>>> Add a tdlabel_re global variable as a regular-expression for a thread
>>>> label in `info threads' that matches either "thread" or "LWP".
>>>>
>>>> Some other tests in the tree don't require a specific word, and
>>>> some targets may use other first words (e.g. OpenBSD uses "thread"
>>>> and Ravenscar threads use "Ravenscar Thread").
>>>
>>> That LGTM, I don't really see another way:
>>>
>>> Approved-By: Simon Marchi <simon.marchi@efficios.com>
>>>
>>> One nit is that I prefer to use the ${::tdlabel_re} notation instead of
>>> the global keyword, but it's not a dealbreaker.
>>>
>>> And just one question:
>>>
>>>> diff --git a/gdb/testsuite/gdb.trace/report.exp b/gdb/testsuite/gdb.trace/report.exp
>>>> index 14db2511d40..a5b7ccf4ba5 100644
>>>> --- a/gdb/testsuite/gdb.trace/report.exp
>>>> +++ b/gdb/testsuite/gdb.trace/report.exp
>>>> @@ -380,7 +380,7 @@ proc use_collected_data { data_source } {
>>>>     
>>>>     	# There is always a thread of an inferior, either a live one or
>>>>     	# a faked one.
>>>> -	gdb_test "info threads" "\\* ${decimal}    (process|Thread) \[0-9\.\]+\[ \t\].*"
>>>> +	gdb_test "info threads" "\\* ${decimal}    ${tdlabel_re} \[0-9\.\]+\[ \t\].*"
>>>
>>> This one said "process" as well, I guess it's not important?
>>
>> Hummm, it didn't regress on Linux, but on some other native target that
>> doesn't support threads it might regress I guess.  normal_pid_to_str
>> uses "process <PID>" and is the defalt for target::pid_to_str.
>>
>> Probably tdlabel_re should permit "process" as a word as well:
>>
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 527c84599ca..6ded65e31c4 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -283,7 +283,7 @@ set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(\[^\n\r\]*\\) exited)"
>>     
>>     # A regular expression that matches the first word of a thread
>>     # description after the thread number info 'info threads'
>> -set tdlabel_re "(Thread|LWP)"
>> +set tdlabel_re "(process|Thread|LWP)"
>>     
>>     # A regular expression that matches a value history number.
>>     # E.g., $1, $2, etc.
>>
>> (I'd also have to reword the commit log a bit to mention this case if so.)
> 
> Just to be sure, are you still ok with this patch with this tweak?

Ping, here's the updated log message to match:

     gdb tests: Allow for "LWP" in thread IDs from info threads.
     
     Several tests assume that the first word after a thread ID in 'info
     threads' output is "Thread".  However, several targets use "LWP"
     instead such as the FreeBSD and NetBSD native targets.  The Linux
     native target also uses "LWP" if libthread_db is not being used.
     Targets that do not support threads use "process" as the first word
     via normal_pid_to_str.
     
     Add a tdlabel_re global variable as a regular-expression for a thread
     label in `info threads' that matches either "process", "Thread", or
     "LWP".
     
     Some other tests in the tree don't require a specific word, and
     some targets may use other first words (e.g. OpenBSD uses "thread"
     and Ravenscar threads use "Ravenscar Thread").


-- 
John Baldwin