On 5/6/25 01:24, Richard Biener wrote:
> On Mon, May 5, 2025 at 10:40 PM David Faust <david.fa...@oracle.com> wrote:
>>
>>
>>
>> On 5/2/25 01:26, Richard Biener wrote:
>>> On Wed, Apr 30, 2025 at 7:26 PM David Faust <david.fa...@oracle.com> wrote:
>>>>
>>>> The btf_decl_tag and btf_type_tag attributes provide a means to annotate
>>>> declarations and types respectively with arbitrary user provided
>>>> strings. These strings are recorded in debug information for
>>>> post-compilation uses, and despite the name they are meant to be
>>>> recorded in DWARF as well as BTF. New DWARF extensions
>>>> DW_TAG_GNU_annotation and DW_AT_GNU_annotation are used to represent
>>>> these user annotations in DWARF.
>>>>
>>>> This patch introduces the new DWARF extension DIE and attribute, and
>>>> generates them as necessary to represent user annotations from
>>>> btf_decl_tag and btf_type_tag.
>>>>
>>>> The format of the new DIE is as follows:
>>>>
>>>> DW_TAG_GNU_annotation
>>>> DW_AT_name: "btf_decl_tag" or "btf_type_tag"
>>>> DW_AT_const_value: <arbitrary user-supplied string>
>>>> DW_AT_GNU_annotation: <reference to another TAG_GNU_annotation DIE>
>>>>
>>>> DW_AT_GNU_annotation is a new attribute extension used to refer to these
>>>> new annotation DIEs. If non-null in any given declaration or type DIE,
>>>> it is a reference to a DW_TAG_GNU_annotation DIE holding an annotation
>>>> for that declaration or type. In addition, the DW_TAG_GNU_annotation
>>>> DIEs may also have a non-null DW_AT_GNU_annotation, referring to another
>>>> annotation DIE. This allows chains of annotation DIEs to be formed,
>>>> such as in the case where a single declaration has multiple instances of
>>>> btf_decl_tag with different string annotations.
>>>>
>>>> gcc/
>>>> * dwarf2out.cc (struct annotation_node, struct
>>>> annotation_node_hasher)
>>>> (btf_tag_htab): New ancillary structures and hash table.
>>>> (annotation_node_hasher::hash, annotation_node_hasher::equal): New.
>>>> (hash_btf_tag, gen_btf_tag_dies, gen_btf_type_tag_dies)
>>>> (maybe_gen_btf_type_tag_dies, gen_btf_decl_tag_dies): New
>>>> functions.
>>>> (modified_type_die): Handle btf_type_tag attribute.
>>>> (gen_array_type_die): Call maybe_gen_btf_type_tags for the type.
>>>> (gen_formal_parameter_die): Call gen_btf_decl_tags for the
>>>> parameter.
>>>> (gen_decl_die): Call gen_btf_decl_tags for the decl.
>>>> (gen_tagged_type_die): Call maybe_gen_btf_type_tag_dies for the
>>>> type.
>>>> (dwarf2out_early_finish): Empty btf_tag_htab hash table.
>>>> (dwarf2out_cc_finalize): Delete btf_tag_htab hash table.
>>>>
>>>> include/
>>>> * dwarf2.def (DW_TAG_GNU_annotation): New DWARF extension.
>>>> (DW_AT_GNU_annotation): Likewise.
>>>>
>>>> gcc/testsuite/
>>>> * gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-1.c: New test.
>>>> * gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-2.c: New test.
>>>> * gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-3.c: New test.
>>>> * gcc.dg/debug/dwarf2/dwarf-btf-type-tag-1.c: New test.
>>>> * gcc.dg/debug/dwarf2/dwarf-btf-type-tag-2.c: New test.
>>>> * gcc.dg/debug/dwarf2/dwarf-btf-type-tag-3.c: New test.
>>>> * gcc.dg/debug/dwarf2/dwarf-btf-type-tag-4.c: New test.
>>>> * gcc.dg/debug/dwarf2/dwarf-btf-type-tag-5.c: New test.
>>>> ---
>>>> gcc/dwarf2out.cc | 270 +++++++++++++++++-
>>>> .../debug/dwarf2/dwarf-btf-decl-tag-1.c | 11 +
>>>> .../debug/dwarf2/dwarf-btf-decl-tag-2.c | 25 ++
>>>> .../debug/dwarf2/dwarf-btf-decl-tag-3.c | 21 ++
>>>> .../debug/dwarf2/dwarf-btf-type-tag-1.c | 10 +
>>>> .../debug/dwarf2/dwarf-btf-type-tag-2.c | 31 ++
>>>> .../debug/dwarf2/dwarf-btf-type-tag-3.c | 15 +
>>>> .../debug/dwarf2/dwarf-btf-type-tag-4.c | 33 +++
>>>> .../debug/dwarf2/dwarf-btf-type-tag-5.c | 10 +
>>>> include/dwarf2.def | 4 +
>>>> 10 files changed, 426 insertions(+), 4 deletions(-)
>>>> create mode 100644
>>>> gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-1.c
>>>> create mode 100644
>>>> gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-2.c
>>>> create mode 100644
>>>> gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-3.c
>>>> create mode 100644
>>>> gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-1.c
>>>> create mode 100644
>>>> gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-2.c
>>>> create mode 100644
>>>> gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-3.c
>>>> create mode 100644
>>>> gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-4.c
>>>> create mode 100644
>>>> gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-5.c
>>>>
>>>> diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc
>>>> index 34ffeed86ff..1ec3b24e773 100644
>>>> --- a/gcc/dwarf2out.cc
>>>> +++ b/gcc/dwarf2out.cc
>>>> @@ -3696,6 +3696,32 @@ static bool frame_pointer_fb_offset_valid;
>>>>
>>>> static vec<dw_die_ref> base_types;
>>>>
>>>> +/* A cached btf_type_tag or btf_decl_tag user annotation. */
>>>> +struct GTY ((for_user)) annotation_node
>>>> +{
>>>> + const char *name;
>>>> + const char *value;
>>>> + hashval_t hash;
>>>> + dw_die_ref die;
>>>> + struct annotation_node *next;
>>>> +};
>>>> +
>>>> +struct annotation_node_hasher : ggc_ptr_hash<annotation_node>
>>>> +{
>>>> + typedef const struct annotation_node *compare_type;
>>>> +
>>>> + static hashval_t hash (struct annotation_node *);
>>>> + static bool equal (const struct annotation_node *,
>>>> + const struct annotation_node *);
>>>> +};
>>>> +
>>>> +/* A hash table of tag annotation nodes for btf_type_tag and btf_decl_tag
>>>> C
>>>> + attributes. DIEs for these user annotations may be reused if they are
>>>> + structurally equivalent; this hash table is used to ensure the DIEs are
>>>> + reused wherever possible. */
>>>> +static GTY (()) hash_table<annotation_node_hasher> *btf_tag_htab;
>>>> +
>>>> +
>>>> /* Flags to represent a set of attribute classes for attributes that
>>>> represent
>>>> a scalar value (bounds, pointers, ...). */
>>>> enum dw_scalar_form
>>>> @@ -13659,6 +13685,180 @@ long_double_as_float128 (tree type)
>>>> return NULL_TREE;
>>>> }
>>>>
>>>> +
>>>> +hashval_t
>>>> +annotation_node_hasher::hash (struct annotation_node *node)
>>>> +{
>>>> + return node->hash;
>>>> +}
>>>> +
>>>> +bool
>>>> +annotation_node_hasher::equal (const struct annotation_node *node1,
>>>> + const struct annotation_node *node2)
>>>> +{
>>>> + return (node1->hash == node2->hash);
>>>
>>> never any hash collision? I'd have expected name/value to be compared here,
>>> and also 'next' (the "chain")?
>>
>> Thanks, you're right. name/value/chain go into the hash but I need to
>> check for collisions here.
>>
>>>
>>> Are we ever able to share partial chains this way? LTO streaming does
>>> for example "share" list tails. I think attribute list generation
>>> does not perform
>>> any attempt to share.
>>
>> Yes, we are able to share common list tails of the attribute. But it does
>> depend on the ordering of the attribute list that we get.
>>
>> For example, if __tagN are all btf_type_tag("tagN") attributes:
>>
>> int * __tag1 __tag2 __tag3 foo;
>> char * __tag1 __tag2 __tag4 bar;
>>
>> In this case the sub-chain for (__tag1 __tag2) is shared, because the
>> attribute lists for the two types have them in the same order
>> (the actual tree_list nodes are not shared despite same contents).
>> Then __tag3 or __tag4 can be tacked on to the head of that common tail,
>> and the result is only one TAG_annotation DIE for each tag is produced.
>>
>> But if we add:
>>
>> float __tag2 __tag1 __tag3 baz;
>>
>> Currently this will not share the above sub-chain, because the attribute
>> list for this type has a different ordering than above.
>>
>> This could be improved by enforcing our own ordering when processing the
>> attribute list for tags, or maybe better by enforcing an ordering when
>> the attribute list is constructed. That would require looking also at
>> the attribute value, which I think we would need to do anyway as a step
>> to making attribute lists sharable. I may look into that in the future,
>> but IMO a little out of scope of this series.
>>
>>>
>>>> +}
>>>> +
>>>> +/* Return an appropriate entry in the btf tag hash table for a given btf
>>>> tag.
>>>> + If a structurally equivalent tag (one with the same name, value, and
>>>> + subsequent chain of further tags) has already been processed, then the
>>>> + existing entry for that tag is returned and should be reused.
>>>> + Otherwise, a new entry is added to the hash table and returned. */
>>>> +
>>>> +static struct annotation_node *
>>>> +hash_btf_tag (tree attr)
>>>> +{
>>>> + if (attr == NULL_TREE || TREE_CODE (attr) != TREE_LIST)
>>>> + return NULL;
>>>> +
>>>> + if (!btf_tag_htab)
>>>> + btf_tag_htab = hash_table<annotation_node_hasher>::create_ggc (10);
>>>> +
>>>> + const char * name = IDENTIFIER_POINTER (get_attribute_name (attr));
>>>> + const char * value = TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE
>>>> (attr)));
>>>> + tree chain = lookup_attribute (name, TREE_CHAIN (attr));
>>>> +
>>>> + /* Hash for one tag depends on hash of next tag in the chain, because
>>>> + the chain is part of structural equivalence. */
>>>> + struct annotation_node *chain_node = hash_btf_tag (chain);
>>>> + gcc_checking_assert (chain == NULL_TREE || chain_node != NULL);
>>>> +
>>>> + /* Skip any non-btf-tag attributes that might be in the chain. */
>>>> + if (strcmp (name, "btf_type_tag") != 0 && strcmp (name, "btf_decl_tag")
>>>> != 0)
>>>> + return chain_node;
>>>> +
>>>> + /* Hash for a given tag is determined by the name, value, and chain of
>>>> + further tags. */
>>>> + inchash::hash h;
>>>> + h.merge_hash (htab_hash_string (name));
>>>> + h.merge_hash (htab_hash_string (value));
>>>> + h.merge_hash (chain_node ? chain_node->hash : 0);
>>>> +
>>>> + struct annotation_node node;
>>>> + node.name = name;
>>>> + node.value = value;
>>>> + node.hash = h.end ();
>>>> + node.next = chain_node;
>>>> +
>>>> + struct annotation_node **slot = btf_tag_htab->find_slot (&node, INSERT);
>>>> + if (*slot == NULL)
>>>> + {
>>>> + /* Create new htab entry for this annotation. */
>>>> + struct annotation_node *new_slot
>>>> + = ggc_cleared_alloc<struct annotation_node> ();
>>>> + new_slot->name = name;
>>>> + new_slot->value = value;
>>>> + new_slot->hash = node.hash;
>>>> + new_slot->next = chain_node;
>>>> +
>>>> + *slot = new_slot;
>>>> + return new_slot;
>>>> + }
>>>> + else
>>>> + {
>>>> + /* This node is already in the hash table. */
>>>> + return *slot;
>>>> + }
>>>> +}
>>>> +
>>>> +/* Generate (or reuse) DW_TAG_annotation DIEs representing the
>>>> btf_type_tag or
>>>> + btf_decl_tag user annotations in ATTR, and update DIE to refer to them
>>>> + via DW_AT_annotation. If there are multiple type_tag or decl_tag
>>>> + annotations in ATTR, they are all processed recursively by this
>>>> function
>>>> + to build a chain of annotation DIEs.
>>>> + Return the first annotation DIE in the created (or reused) chain. */
>>>> +
>>>> +static dw_die_ref
>>>> +gen_btf_tag_dies (tree attr, dw_die_ref die, dw_die_ref context_die)
>>>> +{
>>>> + if (attr == NULL_TREE)
>>>> + return die;
>>>> +
>>>> + while (dw_get_die_tag (context_die) != DW_TAG_compile_unit)
>>>> + context_die = context_die->die_parent;
>>>> +
>>>> + const char * name = IDENTIFIER_POINTER (get_attribute_name (attr));
>>>> + const char * value = TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE
>>>> (attr)));
>>>> +
>>>> + dw_die_ref tag_die, prev = NULL;
>>>> +
>>>> + /* Multiple annotations on the same item form a singly-linked list of
>>>> + annotation DIEs; generate recursively backward from the end so we can
>>>> + chain each created DIE to the next, which has already been created.
>>>> */
>>>> + tree rest = lookup_attribute (name, TREE_CHAIN (attr));
>>>> + if (rest)
>>>> + prev = gen_btf_tag_dies (rest, NULL, context_die);
>>>> +
>>>> + /* Calculate a hash value for the tag based on its structure, find the
>>>> + existing entry for it (if any) in the hash table, or create a new
>>>> entry
>>>> + which can be reused by structurally-equivalent tags. */
>>>> + struct annotation_node *entry = hash_btf_tag (attr);
>>>> + if (!entry)
>>>> + return die;
>>>> +
>>>> + /* If the node already has an associated DIE, reuse it.
>>>> + Otherwise, create the new annotation DIE, and associate it with
>>>> + the hash table entry for future reuse. Any structurally-equivalent
>>>> + tag we process later will find and share the same DIE. */
>>>> + if (entry->die)
>>>> + tag_die = entry->die;
>>>> + else
>>>> + {
>>>> + tag_die = new_die (DW_TAG_GNU_annotation, context_die, NULL);
>>>> + add_name_attribute (tag_die, name);
>>>> + add_AT_string (tag_die, DW_AT_const_value, value);
>>>> + if (prev)
>>>> + add_AT_die_ref (tag_die, DW_AT_GNU_annotation, prev);
>>>
>>> So I can see this saves DWARF space compared to adding multiple
>>> DW_AT_GNU_annotation to the annotated object. It does feel a bit
>>> un-DWARFish, I suppose using DIE children (with the same tag?)
>>> would have been more DWARFish here. That would of course
>>> remove the ability to share sub-lists (but as said, I'm unsure this
>>> works now). So for a three-item list you'd have
>>>
>>> DW_TAG_GNU_annotation
>>> name
>>> value
>>> DW_TAG_GNU_annotation (first child)
>>> name
>>> value
>>> DW_TAG_GNU_annotation (second child)
>>> name
>>> value
>>>
>>> or alternatively, make the outer one "empty" (a container)
>>> and add a third child with the above outers name/value.
>>>
>>> It's a bit of bike-shedding of course.
>>>
>>
>> Hmm, interesting. This is a little similar to what clang does now,
>> except that they attach the annotation DIEs as children directly to
>> the type DIE, which eliminates sharing altogether and was the main
>> driver to look into a different format.
>>
>> So iiuc this is a sort of hybrid, to use the new AT_annotation
>> along with the simpler child-DIE approach. That way we can still
>> share since all types with the same set of tags can use AT_annotation
>> to point to the same TAG_annotation DIE.
>>
>> I agree this is probably a little more simple and a little more
>> DWARFish. Not sure whether or not the tradeoff with sharing sub-chains
>> is worth it.
>>
>> The way type_tag is currently used in the kernel, the common case is
>> that there is a small set of distinct values (e.g. "user", "kernel",
>> "rcu", a few others) and there are many occurrences types which have
>> one or maybe two of these tags. I do think type_tags are an interesting
>> concept which have several potential applications beyond the kernel
>> use case though.
>>
>>>> +
>>>> + entry->die = tag_die;
>>>> + }
>>>> +
>>>> + if (die)
>>>> + add_AT_die_ref (die, DW_AT_GNU_annotation, tag_die);
>>>> +
>>>> + return tag_die;
>>>> +}
>>>> +
>>>> +static void
>>>> +gen_btf_type_tag_dies (tree t, tree attr, dw_die_ref target,
>>>> + dw_die_ref context_die)
>>>> +{
>>>> + if (t == NULL_TREE || !TYPE_P (t) || !target)
>>>> + return;
>>>> +
>>>> + gen_btf_tag_dies (attr, target, context_die);
>>>> +}
>>>> +
>>>> +static void
>>>> +maybe_gen_btf_type_tag_dies (tree t, dw_die_ref die, dw_die_ref
>>>> context_die)
>>>> +{
>>>> + tree tags = lookup_attribute ("btf_type_tag", TYPE_ATTRIBUTES (t));
>>>> + if (tags)
>>>> + {
>>>> + TYPE_ATTRIBUTES (t)
>>>> + = remove_attribute ("btf_type_tag", TYPE_ATTRIBUTES (t));
>>>> + gen_btf_type_tag_dies (t, tags, die, context_die);
>>>> + }
>>>> +}
>>>> +
>>>
>>> Most of the functions miss a function-level comment.
>>
>> Oops, thanks.
>>
>>>
>>>> +static void
>>>> +gen_btf_decl_tag_dies (tree t, dw_die_ref target, dw_die_ref context_die)
>>>> +{
>>>> + if (t == NULL_TREE || !DECL_P (t) || !target)
>>>> + return;
>>>> +
>>>> + tree attr = lookup_attribute ("btf_decl_tag", DECL_ATTRIBUTES (t));
>>>> + if (attr == NULL_TREE)
>>>> + return;
>>>> +
>>>> + gen_btf_tag_dies (attr, target, context_die);
>>>> +
>>>> + /* Strip the decl tag attribute once we have created the annotation DIEs
>>>> + to avoid attempting process it multiple times. */
>>>> + DECL_ATTRIBUTES (t)
>>>> + = remove_attribute ("btf_decl_tag", DECL_ATTRIBUTES (t));
>>>
>>> Hmm, you should get a type/decl DIE created only once, so why would you
>>> run into this multiple times?
>>
>> For decls, I see that we go through gen_decl_die twice for global variables;
>> first from c_parser_declaration_or_fndef via finish_decl, and then again
>> later from c_write_global_declarations_1.
>>
>> For types, it is due to the recursion in modified_type_die.
>>
>>>
>>>> +}
>>>> +
>>>> /* Given a pointer to an arbitrary ..._TYPE tree node, return a debugging
>>>> entry that chains the modifiers specified by CV_QUALS in front of the
>>>> given type. REVERSE is true if the type is to be interpreted in the
>>>> @@ -13783,10 +13983,44 @@ modified_type_die (tree type, int cv_quals, bool
>>>> reverse,
>>>> dquals &= cv_qual_mask;
>>>> if ((dquals & ~cv_quals) != TYPE_UNQUALIFIED
>>>> || (cv_quals == dquals && DECL_ORIGINAL_TYPE (name) != type))
>>>> - /* cv-unqualified version of named type. Just use
>>>> - the unnamed type to which it refers. */
>>>> - return modified_type_die (DECL_ORIGINAL_TYPE (name), cv_quals,
>>>> - reverse, context_die);
>>>> + {
>>>> + if (lookup_attribute ("btf_type_tag", TYPE_ATTRIBUTES
>>>> (type)))
>>>> + {
>>>> + /* Use of a typedef with additional btf_type_tags.
>>>> Create
>>>> + a distinct typedef DIE for this version of the named
>>>> type
>>>> + so that the btf_type_tag annotations may be attached
>>>> to
>>>> + it without affecting other users of the plain
>>>> typedef. */
>>>> + tree tags = lookup_attribute ("btf_type_tag",
>>>> + TYPE_ATTRIBUTES (type));
>>>
>>> you did this in the if () check already. Note I think this is only executed
>>> when the type was qualified or typedef-ed?
>>
>> OK, will fix/reuse the lookup_attribute in the if() here and elsewhere.
>>
>> Yes, this block is for dealing with type_tag on typedefs, since it is
>> possible to add a type_tag at the use of a typedef'd type like
>>
>> typedef const int __tag1 my_int;
>> my_int __tag2 a;
>> my_int __tag3 b;
>> my_int c;
>>
>> We need to construct a few distinct typedef DIEs here so that the different
>> tags can be attached to their respective versions without disrupting other
>> users of the underlying typedef.
>>
>>>
>>>> + tree dtags = lookup_attribute ("btf_type_tag",
>>>> + TYPE_ATTRIBUTES (dtype));
>>>> +
>>>> + /* Remove type tags before the recursive call to avoid
>>>> + processing the same attribute multiple times. */
>>>> + TYPE_ATTRIBUTES (type)
>>>> + = remove_attribute ("btf_type_tag", TYPE_ATTRIBUTES
>>>> (type));
>>>> +
>>>> + dw_die_ref mod_die = modified_type_die (dtype, cv_quals,
>>>> + reverse,
>>>> context_die);
>>>> +
>>>> + /* Create a new typedef DIE since the btf_type_tag-ed
>>>> use of
>>>> + the typedef has really created a distinct type. */
>>>> + if (!attribute_list_equal (tags, dtags))
>>>> + {
>>>> + mod_die = clone_die (mod_die);
>>>> + add_child_die (comp_unit_die (), mod_die);
>>>> + if (!lookup_type_die (type))
>>>> + equate_type_number_to_die (type, mod_die);
>>>> + }
>>>> +
>>>> + gen_btf_type_tag_dies (type, tags, mod_die, context_die);
>>>> + return mod_die;
>>>> + }
>>>> + /* cv-unqualified version of named type. Just use
>>>> + the unnamed type to which it refers. */
>>>> + return modified_type_die (DECL_ORIGINAL_TYPE (name),
>>>> cv_quals,
>>>> + reverse, context_die);
>>>> + }
>>>> /* Else cv-qualified version of named type; fall through. */
>>>> }
>>>> }
>>>> @@ -13887,6 +14121,18 @@ modified_type_die (tree type, int cv_quals, bool
>>>> reverse,
>>>> first_quals |= dwarf_qual_info[i].q;
>>>> }
>>>> }
>>>> + else if (lookup_attribute ("btf_type_tag", TYPE_ATTRIBUTES (type)))
>>>> + {
>>>> + /* Remove type tags before the recursive call to avoid processing
>>>> the
>>>> + same attribute multiple times. */
>>>> + tree attr = lookup_attribute ("btf_type_tag", TYPE_ATTRIBUTES
>>>> (type));
>>>
>>> twice again - if (tree attr = lookup_...) is a thing.
>>>
>>>> + TYPE_ATTRIBUTES (type) = remove_attribute ("btf_type_tag",
>>>> + TYPE_ATTRIBUTES (type));
>>>> + dw_die_ref mod_die = modified_type_die (type, cv_quals, reverse,
>>>> + context_die);
>>>
>>> While recusing might look "convenient" here I think given
>>> modified_type_die's
>>> complexity a more obvious handling should be prefered ;)
>>
>> Yes.. I have had quite some trouble with modified_type_die and doing the
>> right
>> thing in all various constructs with typedefs, cv-quals, and now type_tags as
>> another axis of complexity.
>>
>> Did you a particular approach in mind?
>> I have tried a few non-recursive alternatives, but I didn't land on anything
>> that was actually easier to reason about than the recursive call, without
>> duplicating a lot of logic already written elsewhere.
>>
>> It seems to me that in general we need the recursion, following the same
>> approach as is done for cv-qual'd types above: process the qualifier(s)
>> (here type_tags) and then recurse with a less-qualified type.
>>
>>>
>>> I think you want the tags to work like qualifiers, but I'm not sure
>>> how this works
>>> with your patch. Consider
>>>
>>> struct S { int i ; };
>>>
>>> struct S __attribute__((btf_type_tag("foo"))) a;
>>> const struct S b;
>>> struct S c;
>>> const struct S __attribute__((btf_type_tag("foo"))) d;
>>>
>>> how does the DWARF for 'a' look like? In particular the iffy thing is
>>> dwarf_qual_info and the way we check we already have a qualified
>>> type.
>>
>> This is iffy for me as well.
>>
>> You are right that basically the desire is to have these tags work like
>> qualifiers. But of course, attributes are not qualifiers...
>
> Note TYPE_ADDR_SPACE has the very same issue (and bugs). There is
> currently no nice way to stuff in extra "dwarf" qualifiers that are
> not qualifiers
> on tree (or the other way around, like we have for TYPE_ADDR_SPACE).
Oh, interesting. I'm not that familiar with TYPE_ADDR_SPACE.
Will look into it.
>
>> Currently, the DWARF for 'a' above does not have any annotation DIE at all.
>> Because the attribute is ignored during parsing (also for 'd'):
>>
>> warning: ignoring attributes applied to 'struct S' after definition
>>
>> (This happens also if we try to "hide" the struct behind a typedef).
>>
>> I am undecided so far whether this is really what we want long term.
>> Probably not? For now it seems this is not a concern for kernel users;
>> what they are really concerned about is being able to tag pointer types.
>> But I do think these type tags are useful beyond the kernel needs, so I'd
>> like to figure out a better answer long term.
>>
>> I have experimented some with the attribute parsing and creating a variant
>> type of struct S for the attributed version, but not to a working impl.
>> It's not totally clear to me whether that is something that would be "OK"
>> to do or would cause breakage elsewhere.
>>
>>>
>>> How do you represent having both the type S and the attributed type S?
>>> And how do you go between having a const qualified variant of both?
>>>
>>
>> We would need to have two DIEs for the 'struct S'; one with AT_annotation
>> and one without, as we do for base types like int. Obviously this is not
>> good if S has many members and/or there are many uses of S with different
>> tags.
>>
>> As for const (or otherwise qualified) versions, it is relatively
>> straightforward: the DW_TAG_const refers via AT_type either to the
>> type die for S with AT_annotation or to the one without AT_annotation,
>> as appropriate.
>
> I think what would be good to have is a set of testcases with those
> corner cases and the expectation what happens.
Yep, makes sense. I will add some tests.
>
> modified_type_die is unfortunately a twisted maze and I hate making
> it even more mazier. But I honestly have no energy to try refactoring
> it into something more maintainable...
I can take a stab at it. I know it will not be easy, but maybe we can
get some step in the right direction at least. If it's ok with you, I'll
work on the refactor as a separate/followup patch(es) to this series.
The thing I will need to study more is C++ and other languages'
interactions with modified_type_die and dwarf2out more generally.
Some changes I tried for this patch and a while back when originally
looking at PR110439 (which I still intend to find a solution for,
maybe as a part of refactoring) seemed to work ok for C but cause
explosions in C++ tests.
If you have already any ideas or suggestions about the refactor,
I'm all ears :)
Thanks,
David
>
> Richard.
>
>>
>> Thanks for all your comments :)
>> David
>>
>>>> + gen_btf_type_tag_dies (type, attr, mod_die, context_die);
>>>> + return mod_die;
>>>> + }
>>>> else if (code == POINTER_TYPE || code == REFERENCE_TYPE)
>>>> {
>>>> dwarf_tag tag = DW_TAG_pointer_type;
>>>> @@ -22729,6 +22975,7 @@ gen_array_type_die (tree type, dw_die_ref
>>>> context_die)
>>>> add_pubtype (type, array_die);
>>>>
>>>> add_alignment_attribute (array_die, type);
>>>> + maybe_gen_btf_type_tag_dies (type, array_die, context_die);
>>>> }
>>>>
>>>> /* This routine generates DIE for array with hidden descriptor, details
>>>> @@ -23099,6 +23346,8 @@ gen_formal_parameter_die (tree node, tree origin,
>>>> bool emit_name_p,
>>>> else
>>>> {
>>>> add_child_die (context_die, parm_die);
>>>> +
>>>> + gen_btf_decl_tag_dies (node_or_origin, parm_die,
>>>> context_die);
>>>> return parm_die;
>>>> }
>>>> }
>>>> @@ -23167,6 +23416,8 @@ gen_formal_parameter_die (tree node, tree origin,
>>>> bool emit_name_p,
>>>> gcc_unreachable ();
>>>> }
>>>>
>>>> + gen_btf_decl_tag_dies (node_or_origin, parm_die, context_die);
>>>> +
>>>> return parm_die;
>>>> }
>>>>
>>>> @@ -26391,6 +26642,10 @@ gen_tagged_type_die (tree type,
>>>> else
>>>> gen_struct_or_union_type_die (type, context_die, usage);
>>>>
>>>> + dw_die_ref die = lookup_type_die (type);
>>>> + if (die)
>>>> + maybe_gen_btf_type_tag_dies (type, die, context_die);
>>>> +
>>>> /* Don't set TREE_ASM_WRITTEN on an incomplete struct; we want to fix
>>>> it up if it is ever completed. gen_*_type_die will set it for us
>>>> when appropriate. */
>>>> @@ -27401,6 +27656,9 @@ gen_decl_die (tree decl, tree origin, struct
>>>> vlr_context *ctx,
>>>> break;
>>>> }
>>>>
>>>> + gen_btf_decl_tag_dies (decl_or_origin, lookup_decl_die (decl_or_origin),
>>>> + context_die);
>>>> +
>>>> return NULL;
>>>> }
>>>>
>>>> @@ -33321,6 +33579,9 @@ dwarf2out_early_finish (const char *filename)
>>>> print_die (comp_unit_die (), dump_file);
>>>> }
>>>>
>>>> + if (btf_tag_htab)
>>>> + btf_tag_htab->empty ();
>>>> +
>>>> /* Generate CTF/BTF debug info. */
>>>> if ((ctf_debug_info_level > CTFINFO_LEVEL_NONE
>>>> || btf_debuginfo_p ()) && lang_GNU_C ())
>>>> @@ -33516,6 +33777,7 @@ dwarf2out_cc_finalize (void)
>>>> switch_text_ranges = NULL;
>>>> switch_cold_ranges = NULL;
>>>> current_unit_personality = NULL;
>>>> + btf_tag_htab = NULL;
>>>>
>>>> early_dwarf = false;
>>>> early_dwarf_finished = false;
>>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-1.c
>>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-1.c
>>>> new file mode 100644
>>>> index 00000000000..a1c1676a7ba
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-1.c
>>>> @@ -0,0 +1,11 @@
>>>> +/* Test simple generation of DW_TAG_GNU_annotation DIE for
>>>> + btf_decl_tag attribute. */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-gdwarf -dA" } */
>>>> +
>>>> +int *foo __attribute__((btf_decl_tag ("my_foo")));
>>>> +
>>>> +/* { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\)
>>>> DW_TAG_GNU_annotation" 1 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_name: \"btf_decl_tag\"" 1 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_const_value: \"my_foo\"" 1 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_GNU_annotation" 1 } } */
>>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-2.c
>>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-2.c
>>>> new file mode 100644
>>>> index 00000000000..76583840439
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-2.c
>>>> @@ -0,0 +1,25 @@
>>>> +/* Test dwarf generation for btf_decl_tag on struct and union members. */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-gdwarf -dA" } */
>>>> +
>>>> +#define __tag1 __attribute__((btf_decl_tag ("decl1")))
>>>> +#define __tag2 __attribute__((btf_decl_tag ("decl2")))
>>>> +
>>>> +union U {
>>>> + int i __tag1;
>>>> + unsigned char ub[4];
>>>> +};
>>>> +
>>>> +struct S {
>>>> + union U u;
>>>> + int b __tag2;
>>>> + char *z __tag1;
>>>> +};
>>>> +
>>>> +struct S my_s __tag1 __tag2;
>>>> +
>>>> +/* We must have two occurrances of one of the two annotation DIEs due to
>>>> + the different attribute sets between declarations above. */
>>>> +/* { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\)
>>>> DW_TAG_GNU_annotation" 3 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_name: \"btf_decl_tag\"" 3 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_GNU_annotation" 5 } } */
>>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-3.c
>>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-3.c
>>>> new file mode 100644
>>>> index 00000000000..f3fad8fe3d2
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-decl-tag-3.c
>>>> @@ -0,0 +1,21 @@
>>>> +/* Test dwarf generation for btf_decl_tag on functions and function args.
>>>> */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-gdwarf -dA" } */
>>>> +
>>>> +#define __tag1 __attribute__((btf_decl_tag ("decl1")))
>>>> +#define __tag2 __attribute__((btf_decl_tag ("decl2")))
>>>> +
>>>> +int __tag1 __tag2 func (int arg_a __tag1, int arg_b __tag2)
>>>> +{
>>>> + return arg_a * arg_b;
>>>> +}
>>>> +
>>>> +int foo (int x) {
>>>> + return func (x, x + 1);
>>>> +}
>>>> +
>>>> +/* In this case one of the decl tag DIEs must be duplicated due to
>>>> differing
>>>> + DW_AT_GNU_annotation chain between the three uses. */
>>>> +/* { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\)
>>>> DW_TAG_GNU_annotation" 3 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_name: \"btf_decl_tag\"" 3 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_GNU_annotation" 4 } } */
>>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-1.c
>>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-1.c
>>>> new file mode 100644
>>>> index 00000000000..772aab09cfb
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-1.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* Test simple generation for btf_type_tag attribute. */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-gdwarf -dA" } */
>>>> +
>>>> +int * __attribute__((btf_type_tag("__user"))) ptr;
>>>> +
>>>> +/* { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\)
>>>> DW_TAG_GNU_annotation" 1 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_name: \"btf_type_tag\"" 1 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_const_value: \"__user\"" 1 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_GNU_annotation" 1 } } */
>>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-2.c
>>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-2.c
>>>> new file mode 100644
>>>> index 00000000000..9c44e0ee0b9
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-2.c
>>>> @@ -0,0 +1,31 @@
>>>> +/* Test that DW_TAG_GNU_annotation DIEs for attribute btf_type_tag are
>>>> shared
>>>> + where possible. */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-gdwarf -dA" } */
>>>> +
>>>> +#define __tag1 __attribute__((btf_type_tag("tag1")))
>>>> +#define __tag2 __attribute__((btf_type_tag("tag2")))
>>>> +
>>>> +int __tag1 foo;
>>>> +char * __tag1 __tag2 bar;
>>>> +
>>>> +struct S
>>>> +{
>>>> + unsigned char bytes[8];
>>>> + unsigned long __tag1 t;
>>>> + void *ptr;
>>>> +};
>>>> +
>>>> +struct S * __tag1 __tag2 my_S;
>>>> +
>>>> +/* Only 2 DW_TAG_GNU_annotation DIEs should be generated, one each for
>>>> "tag1"
>>>> + and "tag2", and they should be reused. */
>>>> +/* { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\)
>>>> DW_TAG_GNU_annotation" 2 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_name: \"btf_type_tag\"" 2 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_const_value: \"tag1\"" 1 } }
>>>> */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_const_value: \"tag2\"" 1 } }
>>>> */
>>>> +
>>>> +/* Each attribute-ed type shall refer via DW_AT_GNU_annotation to the
>>>> + appropriate annotation DIE, including the annotation DIE for "tag2"
>>>> which
>>>> + is always chained to the DIE for "tag1" in this construction. */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_GNU_annotation" 5 } } */
>>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-3.c
>>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-3.c
>>>> new file mode 100644
>>>> index 00000000000..d02144c8004
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-3.c
>>>> @@ -0,0 +1,15 @@
>>>> +/* Test dwarf generation for btf_type_tag with cv-quals and typedefs. */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-gdwarf -dA" } */
>>>> +
>>>> +#define __tag1 __attribute__((btf_type_tag ("tag1")))
>>>> +#define __tag2 __attribute__((btf_type_tag ("tag2")))
>>>> +
>>>> +typedef const int foo;
>>>> +typedef int __tag1 bar;
>>>> +
>>>> +foo __tag2 x;
>>>> +const bar y;
>>>> +
>>>> +/* { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\)
>>>> DW_TAG_GNU_annotation" 2 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_name: \"btf_type_tag\"" 2 }
>>>> } */
>>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-4.c
>>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-4.c
>>>> new file mode 100644
>>>> index 00000000000..2e916e8b443
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-4.c
>>>> @@ -0,0 +1,33 @@
>>>> +/* Test generating annotation DIEs for struct/union/enum types. */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-gdwarf -dA" } */
>>>> +
>>>> +enum E
>>>> +{
>>>> + ONE,
>>>> + TWO
>>>> +} __attribute__((btf_type_tag("foo")));
>>>> +
>>>> +enum E some_e;
>>>> +
>>>> +struct S
>>>> +{
>>>> + int i;
>>>> + char c;
>>>> +} __attribute__((btf_type_tag("foo")));
>>>> +
>>>> +typedef struct S S1;
>>>> +const S1 some_s1;
>>>> +
>>>> +union U
>>>> +{
>>>> + int x;
>>>> + char y;
>>>> +} __attribute__((btf_type_tag("foo")));
>>>> +
>>>> +volatile union U volatile_u;
>>>> +
>>>> +/* One annotation DIE may be shared by all three annotated types. */
>>>> +/* { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\)
>>>> DW_TAG_GNU_annotation" 1 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_name: \"btf_type_tag\"" 1 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_GNU_annotation" 3 } } */
>>>> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-5.c
>>>> b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-5.c
>>>> new file mode 100644
>>>> index 00000000000..1a6b29f99a1
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/dwarf-btf-type-tag-5.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* Test generation for btf_type_tag attribute on array type. */
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-gdwarf -dA" } */
>>>> +
>>>> +int arr[8] __attribute__((btf_type_tag("tagged_arr")));
>>>> +
>>>> +/* { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\)
>>>> DW_TAG_GNU_annotation" 1 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_name: \"btf_type_tag\"" 1 }
>>>> } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_const_value: \"tagged_arr\""
>>>> 1 } } */
>>>> +/* { dg-final { scan-assembler-times " DW_AT_GNU_annotation" 1 } } */
>>>> diff --git a/include/dwarf2.def b/include/dwarf2.def
>>>> index 989f078041d..37b8d6b99d0 100644
>>>> --- a/include/dwarf2.def
>>>> +++ b/include/dwarf2.def
>>>> @@ -174,6 +174,9 @@ DW_TAG (DW_TAG_GNU_formal_parameter_pack, 0x4108)
>>>> are properly part of DWARF 5. */
>>>> DW_TAG (DW_TAG_GNU_call_site, 0x4109)
>>>> DW_TAG (DW_TAG_GNU_call_site_parameter, 0x410a)
>>>> +
>>>> +DW_TAG (DW_TAG_GNU_annotation, 0x6001)
>>>> +
>>>> /* Extensions for UPC. See: http://dwarfstd.org/doc/DWARF4.pdf. */
>>>> DW_TAG (DW_TAG_upc_shared_type, 0x8765)
>>>> DW_TAG (DW_TAG_upc_strict_type, 0x8766)
>>>> @@ -456,6 +459,7 @@ DW_AT (DW_AT_GNU_pubtypes, 0x2135)
>>>> DW_AT (DW_AT_GNU_discriminator, 0x2136)
>>>> DW_AT (DW_AT_GNU_locviews, 0x2137)
>>>> DW_AT (DW_AT_GNU_entry_view, 0x2138)
>>>> +DW_AT (DW_AT_GNU_annotation, 0x2139)
>>>> /* VMS extensions. */
>>>> DW_AT (DW_AT_VMS_rtnbeg_pd_address, 0x2201)
>>>> /* GNAT extensions. */
>>>> --
>>>> 2.47.2
>>>>
>>
