Re: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[Bruno Larsen <blarsen@redhat.com>]

On 6/10/22 08:01, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> Hi Jini,
>>
>> Great to hear that Kavitha's changes have landed on clang! I do think
>> that it is still important to have xfails, however, since only new
>> clangs would add the information, and GDB is tested in all manner of
>> systems.
>>
>> I have changed the patch to assume that clang 15 has Kavitha's patches,
>> and changed the clang compiler test.  Does this look acceptable?
>>
>> [PATCH v4 03/14] gdb/testsuite: Change gdb.base/symbol-alias to xfail with old clang
>>
>> Clang didn't use to add alias information, even when using -gfull.  This
>> commit checks if the clang version being used is already providing alias
>> information (15 or newer), otherwise it adds an XFAIL.
> 
> My understanding from Jini's email was that for this test to pass we
> would also need this gdb patch:
> 
>    https://sourceware.org/pipermail/gdb-patches/2022-April/188354.html
> 
> So, if I had clang 15 right now, this test would still fail, right?

Yes. My thought was that for clang < 15, it is an XFAIL, because there is nothing we can do about it, and I'd leave all other outputs the same.

> 
> I guess you either need to hold this patch back until the above is
> merged, or put this in with a generic "all clang" pattern (like you
> originally had) and then assume someone will spot the KPASS and fix up
> the test later.

If I'm honest, I thought that patch would land before clang-15 was released. I can hold off on this until then.

> 
> Also...
> 
> 
>> ---
>>   gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp b/gdb/testsuite/gdb.base/symbol-alias.exp
>> index 289f49bbc3f..078158dc101 100644
>> --- a/gdb/testsuite/gdb.base/symbol-alias.exp
>> +++ b/gdb/testsuite/gdb.base/symbol-alias.exp
>> @@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {
>>   }
>>   
>>   # Variables.
>> -foreach v {"g_var_s" "g_var_s_alias"} {
>> -    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
>> +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
>> +
>> +# Clang didn't include alias information until version 15.
>> +if {[test_compiler_info {clang-[1-9]*}]
>> +     || [test_compiler_info {clang-1[0-4]*}]} {
> 
> Wouldn't clang-15 match the first of these patterns?

Now that you mention it, it should, but doesn't. My guess is that TCL implicitly adds [^1-9] for the first character of '*', assuming that if you meant to have it, you'd have written [1-9]+ instead. However, I will change it to "clang-[1-9]-*" to not rely on that unexplained behavior.

Cheers!
Bruno Larsen

> 
> Thannks,
> Andrew
> 
> 
>> +    setup_xfail "clang/52664" *-*-*
>>   }
>> +gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
>> -- 
>> 2.31.1
>