> "Jose E. Marchesi" <[email protected]> writes:
>
>> ping
>
> richi already approved it some time ago:
> https://inbox.sourceware.org/gcc-patches/[email protected]/
Oh missed that. Thanks for pointing that out.
Doing the backport then.
>
>>
>>> This BPF related patch involved dwarf2out.
>>> Is it OK for GCC 16?
>>>
>>> ---
>>>
>>> typedef chain traversal could sometimes skip an intermediate type.
>>> This could happen when two DIEs have a common underlying type but diverge
>>> due to presence of additional attribute in one case and a qualifier in
>>> other.
>>> In the example from testcase, variable "perm" typechain misses interim u16.
>>>
>>> typedef unsigned short __u16;
>>> typedef __u16 u16;
>>> __attribute__((btf_type_tag(""))) u16 a;
>>> const u16 perm;
>>>
>>> generates
>>>
>>> .uleb128 0x1 # (DIE (0x66) DW_TAG_variable)
>>> .long .LASF3 # DW_AT_name: "perm"
>>> # DW_AT_decl_file (1, pr125421.c)
>>> .byte 0xa # DW_AT_decl_line
>>> .byte 0xb # DW_AT_decl_column
>>> .long 0x2f # DW_AT_type
>>>
>>> .uleb128 0x4 # (DIE (0x2f) DW_TAG_const_type)
>>> .long 0x23 # DW_AT_type <-- BUG
>>>
>>> .uleb128 0x3 # (DIE (0x23) DW_TAG_typedef)
>>> .long .LASF5 # DW_AT_name: "__u16"
>>> .byte 0x1 # DW_AT_decl_file (pr125421.c)
>>> .byte 0x7 # DW_AT_decl_line
>>> .byte 0xf # DW_AT_decl_column
>>> .long 0x34 # DW_AT_type
>>>
>>> .uleb128 0x5 # (DIE (0x34) DW_TAG_base_type)
>>> .byte 0x2 # DW_AT_byte_size
>>> .byte 0x5 # DW_AT_encoding
>>> .long .LASF6 # DW_AT_name: "short int"
>>>
>>> What makes this issue worse is depending on the order in which the types
>>> are specified in source, and processed, the problem may or maynot show up.
>>> So in the test code above if lines 3 and 4 are swppaed, "perm" gets the
>>> missing type u16 as expected.
>>>
>>> The issue is in modified_type_die (), get_qualified_type (type) returned
>>> pointer may not be identical to dtype: TREE_TYPE (TYPE_NAME
>>> (qualified_type))
>>> while having the same underlying base type. And due to the failed
>>> pointer identity test, subsequent usage of DECL_ORIGINAL_TYPE () for
>>> recursive
>>> chain processing can peel away the needed type.
>>>
>>> The implications are for a multi CU build, structurally similar but
>>> non identical variants of types (due to an embedded member getting a
>>> different chained typdef) can be generated and accumulate at link time
>>> in the final .debug_info.
>>>
>>> This would be OK / unnoticed for usual dwarf debugging purposes.
>>> However in BFP workflow: kernel binary linking invokes pahole to process
>>> the dwarf and dedup it for btf generation (gcc can emit btf directly but
>>> thats not been done currently for other reasons). The slightly different
>>> variations of same structure due to thi issue cause combinatiorial explosion
>>> in pahole dedup processing trying to match and failing repeatedly in what
>>> seems like infinite recursion. The problem showed up and was excerbated
>>> when trying to enable attr btf_type_tag in kernel with gcc for the first
>>> time as that is the key ingredient to trigger the latent issue.
>>>
>>> Fix is to handle qualified_type != dtype as if they were identical when
>>> they are different variant nodes of the same underlying typedef.
>>>
>>> Bootstrapped and regtested on x86 and aarch64.
>>>
>>> PR debug/125421
>>>
>>> gcc/ChangeLog:
>>>
>>> * dwarf2out.cc (modified_type_die): Handle qualified_type being
>>> a variant of dtype as if they were identical to avoid peeling a
>>> typdef.
>>> For named types, use dentry unconditionally as that is valid for
>>> both cases of qualified_type == and != dentry.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.dg/debug/dwarf2/pr125421.c: New Test.
>>>
>>> Signed-off-by: Vineet Gupta <[email protected]>
>>> (cherry picked from commit 3e2885588fe0e5273a50c471d41c61ee9aefc269)
>>> ---
>>> gcc/dwarf2out.cc | 15 ++++++++++++---
>>> gcc/testsuite/gcc.dg/debug/dwarf2/pr125421.c | 18 ++++++++++++++++++
>>> 2 files changed, 30 insertions(+), 3 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/pr125421.c
>>>
>>> diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc
>>> index bf342e468bf..9d6ca06067f 100644
>>> --- a/gcc/dwarf2out.cc
>>> +++ b/gcc/dwarf2out.cc
>>> @@ -13997,7 +13997,16 @@ modified_type_die (tree type, int cv_quals, tree
>>> type_attrs, bool reverse,
>>> tree dtype = TREE_TYPE (name);
>>>
>>> /* Skip the typedef for base types with DW_AT_endianity, no big
>>> deal. */
>>> - if (qualified_type == dtype && !reverse_type)
>>> + if (!reverse_type
>>> + && (qualified_type == dtype
>>> + /* Pointer identity check above might fail when qualified_type
>>> + is a different variant node of the same typedef yet requires
>>> + the same handling as if they matched (see PR/125421).
>>> + Skip this when btf_type_tag attributes are present, as those
>>> + need to be handled in the else branch below. */
>>> + || (TYPE_NAME (qualified_type) == name
>>> + && TYPE_QUALS (qualified_type) == TYPE_QUALS (dtype)
>>> + && !lookup_attribute ("btf_type_tag", type_attrs))))
>>> {
>>> tree origin = decl_ultimate_origin (name);
>>>
>>> @@ -14009,8 +14018,8 @@ modified_type_die (tree type, int cv_quals, tree
>>> type_attrs, bool reverse,
>>> reverse, context_die);
>>>
>>> /* For a named type, use the typedef. */
>>> - gen_type_die (qualified_type, context_die);
>>> - return lookup_type_die (qualified_type);
>>> + gen_type_die (dtype, context_die);
>>> + return lookup_type_die (dtype);
>>> }
>>> else
>>> {
>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr125421.c
>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/pr125421.c
>>> new file mode 100644
>>> index 00000000000..810afeae739
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr125421.c
>>> @@ -0,0 +1,18 @@
>>> +/* Verify the typedef chains are preserved.
>>> + DW_TAG_const_type for "const u16" chains to u16 typedef and doesnot
>>> + skip to underlying __u16 typedef */
>>> +
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-gdwarf -dA" } */
>>> +
>>> +typedef short __u16;
>>> +typedef __u16 u16;
>>> +__attribute__((btf_type_tag(""))) u16 __softirq_pending;
>>> +const u16 perm;
>>> +
>>> +/* The exact failing pattern is hard to encode with TCL regex machinery,
>>> + so resort to an indirect way: in the buggy output, const_type points
>>> + directly to __u16, and only one u16 typedef DIE is emitted (for the
>>> + btf_type_tag use). In the fixed output, const_type points to a u16
>>> + typedef, creating a second u16 typedef DIE.
>>> +/* { dg-final { scan-assembler-times "(DW_AT_name:
>>> \"u16\"|\"u16..\"\[^\\r\\n\]*DW_AT_name)" 2 } } */
