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

[Bruno Larsen <blarsen@redhat.com>]

Good catch. I'll drop this patch.

Cheers!
Bruno Larsen

On 6/14/22 04:23, George, Jini Susan wrote:
> [Public]
> 
> I missed mentioning that Kavitha's patch takes care of making symbol-alias.exp xfail for clang versions less than 15.
> 
> Thanks,
> Jini.
> 
>>> -----Original Message-----
>>> From: Gdb-patches <gdb-patches-
>>> bounces+jigeorge=amd.com@sourceware.org> On Behalf Of George, Jini Susan
>>> via Gdb-patches
>>> Sent: Tuesday, June 14, 2022 12:44 PM
>>> To: Andrew Burgess <aburgess@redhat.com>; Bruno Larsen
>>> <blarsen@redhat.com>; gdb-patches@sourceware.org
>>> Cc: Natarajan, Kavitha <Kavitha.Natarajan@amd.com>
>>> Subject: RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang
>>>
>>> [Public]
>>>
>>> [CAUTION: External Email]
>>>
>>> [Public]
>>>
>>> Now that Kavitha's patch just got committed (details below), guess we might not
>>> require this.
>>>
>>> commit 6df97c56ea0f3086c96743ec47148ee69fd8cf71 (HEAD -> master,
>>> origin/master, origin/HEAD)
>>> Author: Kavitha Natarajan <kavitha.natarajan@amd.com>
>>> Date:   Tue Jun 14 10:37:46 2022 +0530
>>>
>>>     Debug support for global alias variable
>>>
>>>     Starting with (future) Clang 15 (since
>>>
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.
>>> llvm.org%2FD120989&amp;data=05%7C01%7CJiniSusan.George%40amd.com%
>>> 7Ce7ea873468214efcf9d908da4dd648a2%7C3dd8961fe4884e608e11a82d994e
>>> 183d%7C0%7C0%7C637907879987330246%7CUnknown%7CTWFpbGZsb3d8eyJ
>>> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
>>> 3000%7C%7C%7C&amp;sdata=bNOSusnr0cUBWttQ1PM8JrnijdjPJl2ee7lBpPsStY
>>> A%3D&amp;reserved=0), Clang emits the DWARF information
>>>     of global alias variables as DW_TAG_imported_declaration.  However,
>>>     GDB does not handle it.  It incorrectly always reads this tag as
>>>     C++/Fortran imported declaration (type alias, namespace alias and
>>>     Fortran module).  This commit adds support to handle this tag as an
>>>     alias variable.
>>>
>>>     This change fixes the failures in the gdb.base/symbol-alias.exp
>>>     testcase with current git Clang.  This testcase is also updated to
>>>     test nested (recursive) aliases.
>>>
>>> Thanks,
>>> Jini.
>>>
>>>>> -----Original Message-----
>>>>> From: Gdb-patches <gdb-patches-
>>>>> bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Andrew Burgess
>>>>> via Gdb-patches
>>>>> Sent: Friday, June 10, 2022 4:31 PM
>>>>> To: Bruno Larsen <blarsen@redhat.com>; gdb-patches@sourceware.org
>>>>> Subject: RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail
>>>>> with clang
>>>>>
>>>>> [CAUTION: External Email]
>>>>>
>>>>> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
>>>>> ce
>>>>> ware.org%2Fpipermail%2Fgdb-patches%2F2022-
>>>>> April%2F188354.html&amp;data=05%7C01%7CJiniSusan.George%40amd.com
>>> %
>>>>> 7Ca2aaecea2df0476d781808da4ad0988b%7C3dd8961fe4884e608e11a82d99
>>> 4e
>>>>> 183d%7C0%7C0%7C637904557029019597%7CUnknown%7CTWFpbGZsb3d8e
>>> yJ
>>>>> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
>>> 7C
>>>>> 3000%7C%7C%7C&amp;sdata=8%2FUKHMbpyAKXN4HOPHRJ2vP62yKK5i5Qe
>>> au
>>>>> v0GQtWHo%3D&amp;reserved=0
>>>>>
>>>>> So, if I had clang 15 right now, this test would still fail, right?
>>>>>
>>>>> 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.
>>>>>
>>>>> 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?
>>>>>
>>>>> Thannks,
>>>>> Andrew
>>>>>
>>>>>
>>>>>> +    setup_xfail "clang/52664" *-*-*
>>>>>>   }
>>>>>> +gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
>>>>>> --
>>>>>> 2.31.1
>