All of lore.kernel.org
 help / color / mirror / Atom feed
* [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC]
@ 2026-05-28 22:36 Vineet Gupta
  2026-05-28 22:36 ` [PAHOLE Patch v2 2/2] Add support for DW_TAG_GNU_annotation Vineet Gupta
  2026-05-29 13:23 ` [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC] Alan Maguire
  0 siblings, 2 replies; 7+ messages in thread
From: Vineet Gupta @ 2026-05-28 22:36 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, Andrii Nakryiko, acme, Alan Maguire, jose.marchesi,
	David Faust, Vineet Gupta

NFC change preparing for DW_TAG_GNU_annotation support.
Extract the btf_type_tag annotation creation logic from into a helper
die__add_btf_type_tag().

Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
---
Changes since v1 [1]
 - NFC reinstate some original comments

[1] https://lore.kernel.org/bpf/20260526181818.4159927-1-vineet.gupta@linux.dev/
---
 dwarf_loader.c | 58 +++++++++++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 24 deletions(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index 16fb7becffee..8fc9a6794f2f 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -1600,14 +1600,45 @@ static struct btf_type_tag_type *die__create_new_btf_type_tag_type(Dwarf_Die *di
 	return tag;
 }
 
+static int die__add_btf_type_tag(struct btf_type_tag_ptr_type **tagp,
+				 Dwarf_Die *die, Dwarf_Die *adie,
+				 struct cu *cu, struct conf_load *conf)
+{
+	struct btf_type_tag_type *annot;
+	uint32_t id;
+
+	if (*tagp == NULL) {
+		/* Create a btf_type_tag_ptr type. */
+		*tagp = die__create_new_btf_type_tag_ptr_type(die, cu);
+		if (!*tagp)
+			return -ENOMEM;
+	}
+
+	/* Create a btf_type_tag type for this annotation. */
+	annot = die__create_new_btf_type_tag_type(adie, cu, conf);
+	if (annot == NULL)
+		return -ENOMEM;
+
+	if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
+		return -ENOMEM;
+
+	struct dwarf_tag *dtag = tag__dwarf(&annot->tag);
+	dtag->small_id = id;
+	cu__hash(cu, &annot->tag);
+
+	/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
+	 * the tag->tags contains tag3 -> tag2 -> tag1.
+	 */
+	list_add(&annot->node, &(*tagp)->tags);
+	return 0;
+}
+
 static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
 					       struct conf_load *conf)
 {
 	struct btf_type_tag_ptr_type *tag = NULL;
-	struct btf_type_tag_type *annot;
 	Dwarf_Die *cdie, child;
 	const char *name;
-	uint32_t id;
 
 	/* If no child tags or skipping btf_type_tag encoding, just create a new tag
 	 * and return
@@ -1627,29 +1658,8 @@ static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
 		if (strcmp(name, "btf_type_tag") != 0)
 			continue;
 
-		if (tag == NULL) {
-			/* Create a btf_type_tag_ptr type. */
-			tag = die__create_new_btf_type_tag_ptr_type(die, cu);
-			if (!tag)
-				return NULL;
-		}
-
-		/* Create a btf_type_tag type for this annotation. */
-		annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
-		if (annot == NULL)
+		if (die__add_btf_type_tag(&tag, die, cdie, cu, conf))
 			return NULL;
-
-		if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
-			return NULL;
-
-		struct dwarf_tag *dtag = tag__dwarf(&annot->tag);
-		dtag->small_id = id;
-		cu__hash(cu, &annot->tag);
-
-		/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
-		 * the tag->tags contains tag3 -> tag2 -> tag1.
-		 */
-		list_add(&annot->node, &tag->tags);
 	} while (dwarf_siblingof(cdie, cdie) == 0);
 
 	return tag ? &tag->tag : tag__new(die, cu);
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PAHOLE Patch v2 2/2] Add support for DW_TAG_GNU_annotation
  2026-05-28 22:36 [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC] Vineet Gupta
@ 2026-05-28 22:36 ` Vineet Gupta
  2026-05-29 13:37   ` Alan Maguire
  2026-05-29 13:23 ` [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC] Alan Maguire
  1 sibling, 1 reply; 7+ messages in thread
From: Vineet Gupta @ 2026-05-28 22:36 UTC (permalink / raw)
  To: dwarves
  Cc: bpf, Andrii Nakryiko, acme, Alan Maguire, jose.marchesi,
	David Faust, Vineet Gupta

gcc 16 was first release to support DW_TAG_GNU_annotation and this patch
enables the same in pahole. Bulk of changes are in dwarf_loader but
btf_encoder also gains support with minimal changes.

GCC encodes btf_type_tag and btf_decl_tag annotations differently from
LLVM. While LLVM uses DW_TAG_LLVM_annotation (0x6000) as child DIEs,
GCC uses DW_TAG_GNU_annotation (0x6001) as standalone sibling DIEs
referenced via DW_AT_GNU_annotation (0x2139) attributes, with chaining
through the same attribute on annotation DIEs themselves.

Handle both encoding styles:

For btf_type_tag (pointer annotations):
- Recognize DW_TAG_GNU_annotation alongside DW_TAG_LLVM_annotation in
  child annotation scanning.
- Follow DW_AT_GNU_annotation attribute chains on pointer types for
  GCC-style btf_type_tag resolution, with cycle detection.
- Normalize DW_TAG_GNU_annotation to DW_TAG_LLVM_annotation in the
  internal representation so downstream code works unchanged.

For btf_decl_tag (function/struct/member annotations):
- Add add_gnu_annotation_chain() to follow DW_AT_GNU_annotation
  attribute chains on function, struct, and member DIEs.
- GCC puts DW_AT_GNU_annotation on the function/struct DIE itself
  (not as child DIEs), referencing sibling annotation DIEs that chain
  via the same attribute.

Also:
- Silently skip standalone DW_TAG_GNU_annotation DIEs at CU level.
- Update pfunct-btf-decl-tags.sh test to use GCC 16+ when available and
  now passes.

Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
---
Changes since v1 [1]
 - NFC Reduce indentation with early exits (Alexei offlist)

[1] https://lore.kernel.org/bpf/20260526181818.4159927-2-vineet.gupta@linux.dev/
---
 btf_encoder.c                 |   1 +
 dutil.h                       |   8 +++
 dwarf_loader.c                | 115 +++++++++++++++++++++++++++++++---
 dwarves.h                     |   3 +-
 dwarves_fprintf.c             |   8 ++-
 tests/pfunct-btf-decl-tags.sh |  32 +++++++---
 6 files changed, 147 insertions(+), 20 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 633bc6162ce0..d5af706d7638 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -1831,6 +1831,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
 		name = namespace__name(tag__namespace(tag));
 		return btf_encoder__add_ref_type(encoder, BTF_KIND_TYPEDEF, ref_type_id, name, false);
 	case DW_TAG_LLVM_annotation:
+	case DW_TAG_GNU_annotation:
 		name = tag__btf_type_tag(tag)->value;
 		return btf_encoder__add_ref_type(encoder, BTF_KIND_TYPE_TAG, ref_type_id, name, false);
 	case DW_TAG_structure_type:
diff --git a/dutil.h b/dutil.h
index ff78aa6dfd10..abe0e62b412f 100644
--- a/dutil.h
+++ b/dutil.h
@@ -35,6 +35,14 @@
 #define DW_TAG_LLVM_annotation 0x6000
 #endif
 
+#ifndef DW_TAG_GNU_annotation
+#define DW_TAG_GNU_annotation 0x6001
+#endif
+
+#ifndef DW_AT_GNU_annotation
+#define DW_AT_GNU_annotation 0x2139
+#endif
+
 static inline __attribute__((const)) bool is_power_of_2(unsigned long n)
 {
         return (n != 0 && ((n & (n - 1)) == 0));
diff --git a/dwarf_loader.c b/dwarf_loader.c
index 8fc9a6794f2f..a3d51e30703c 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -908,6 +908,12 @@ static int tag__recode_dwarf_bitfield(struct tag *tag, struct cu *cu, uint16_t b
 	return -ENOMEM;
 }
 
+static bool die__tag_is_annotation(Dwarf_Die *die)
+{
+	unsigned int tag = dwarf_tag(die);
+	return tag == DW_TAG_LLVM_annotation || tag == DW_TAG_GNU_annotation;
+}
+
 static int add_llvm_annotation(Dwarf_Die *die, int component_idx, struct conf_load *conf,
 			       struct list_head *head)
 {
@@ -943,7 +949,7 @@ static int add_child_llvm_annotations(Dwarf_Die *die, int component_idx,
 
 	die = &child;
 	do {
-		if (dwarf_tag(die) == DW_TAG_LLVM_annotation) {
+		if (die__tag_is_annotation(die)) {
 			ret = add_llvm_annotation(die, component_idx, conf, head);
 			if (ret)
 				return ret;
@@ -953,6 +959,32 @@ static int add_child_llvm_annotations(Dwarf_Die *die, int component_idx,
 	return 0;
 }
 
+static int add_gnu_annotation_chain(Dwarf_Die *die, int component_idx,
+				    struct conf_load *conf, struct list_head *head)
+{
+	Dwarf_Attribute attr;
+	Dwarf_Die annot_die;
+
+	if (dwarf_attr(die, DW_AT_GNU_annotation, &attr) == NULL ||
+	    dwarf_formref_die(&attr, &annot_die) == NULL)
+		return 0;
+
+	for (;;) {
+		if (dwarf_tag(&annot_die) != DW_TAG_GNU_annotation)
+			break;
+
+		int ret = add_llvm_annotation(&annot_die, component_idx, conf, head);
+		if (ret)
+			return ret;
+
+		if (dwarf_attr(&annot_die, DW_AT_GNU_annotation, &attr) == NULL ||
+		    dwarf_formref_die(&attr, &annot_die) == NULL)
+			break;
+	}
+
+	return 0;
+}
+
 int class_member__dwarf_recode_bitfield(struct class_member *member,
 					struct cu *cu)
 {
@@ -1596,6 +1628,8 @@ static struct btf_type_tag_type *die__create_new_btf_type_tag_type(Dwarf_Die *di
 		return NULL;
 
 	tag__init(&tag->tag, cu, die);
+	/* Normalize DW_TAG_GNU_annotation to DW_TAG_LLVM_annotation internally */
+	tag->tag.tag = DW_TAG_LLVM_annotation;
 	tag->value = attr_string(die, DW_AT_const_value, conf);
 	return tag;
 }
@@ -1638,19 +1672,21 @@ static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
 {
 	struct btf_type_tag_ptr_type *tag = NULL;
 	Dwarf_Die *cdie, child;
+	Dwarf_Attribute attr;
+	Dwarf_Die annot_die;
 	const char *name;
 
-	/* If no child tags or skipping btf_type_tag encoding, just create a new tag
-	 * and return
-	 */
-	if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
-	    conf->skip_encoding_btf_type_tag)
+	/* If skipping btf_type_tag encoding, just create a new tag, return */
+	if (conf->skip_encoding_btf_type_tag)
 		return tag__new(die, cu);
 
-	/* Otherwise, check DW_TAG_LLVM_annotation child tags */
+	/* Handle LLVM style annotation tags if present */
+	if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0)
+		goto check_gnu_attr;
+
 	cdie = &child;
 	do {
-		if (dwarf_tag(cdie) != DW_TAG_LLVM_annotation)
+		if (!die__tag_is_annotation(cdie))
 			continue;
 
 		/* Only check btf_type_tag annotations */
@@ -1662,6 +1698,47 @@ static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
 			return NULL;
 	} while (dwarf_siblingof(cdie, cdie) == 0);
 
+check_gnu_attr:
+	/* Check for GCC-style DW_AT_GNU_annotation attribute */
+	if (tag != NULL ||
+	    dwarf_attr(die, DW_AT_GNU_annotation, &attr) == NULL ||
+	    dwarf_formref_die(&attr, &annot_die) == NULL)
+		goto out;
+
+	Dwarf_Off visited[256];
+	int nr_visited = 0;
+
+	for (;;) {
+		Dwarf_Off off = dwarf_dieoffset(&annot_die);
+		bool cycle = false;
+		int i;
+
+		for (i = 0; i < nr_visited; i++) {
+			if (visited[i] == off) {
+				cycle = true;
+				break;
+			}
+		}
+		if (cycle || nr_visited >= (int)ARRAY_SIZE(visited))
+			break;
+		visited[nr_visited++] = off;
+
+		if (dwarf_tag(&annot_die) != DW_TAG_GNU_annotation)
+			break;
+
+		name = attr_string(&annot_die, DW_AT_name, conf);
+		if (strcmp(name, "btf_type_tag") != 0)
+			break;
+
+		if (die__add_btf_type_tag(&tag, die, &annot_die, cu, conf))
+			return NULL;
+
+		if (dwarf_attr(&annot_die, DW_AT_GNU_annotation, &attr) == NULL ||
+		    dwarf_formref_die(&attr, &annot_die) == NULL)
+			break;
+	}
+
+out:
 	return tag ? &tag->tag : tag__new(die, cu);
 }
 
@@ -1690,6 +1767,12 @@ static struct tag *die__create_new_class(Dwarf_Die *die, struct cu *cu, struct c
 		}
 	}
 
+	if (class != NULL &&
+	    add_gnu_annotation_chain(die, -1, conf, &class->type.namespace.annots) != 0) {
+		class__delete(class, cu);
+		class = NULL;
+	}
+
 	return class ? &class->type.namespace.tag : NULL;
 }
 
@@ -2051,10 +2134,13 @@ static int die__process_class(Dwarf_Die *die, struct type *class,
 			cu__hash(cu, &member->tag);
 			if (add_child_llvm_annotations(die, member_idx, conf, &class->namespace.annots))
 				return -ENOMEM;
+			if (add_gnu_annotation_chain(die, member_idx, conf, &class->namespace.annots))
+				return -ENOMEM;
 			member_idx++;
 		}
 			continue;
 		case DW_TAG_LLVM_annotation:
+		case DW_TAG_GNU_annotation:
 			if (add_llvm_annotation(die, -1, conf, &class->namespace.annots))
 				return -ENOMEM;
 			continue;
@@ -2360,6 +2446,7 @@ static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
 				goto out_enomem;
 			continue;
 		case DW_TAG_LLVM_annotation:
+		case DW_TAG_GNU_annotation:
 			if (add_llvm_annotation(die, -1, conf, &(tag__function(&ftype->tag)->annots)))
 				goto out_enomem;
 			continue;
@@ -2408,6 +2495,12 @@ static struct tag *die__create_new_function(Dwarf_Die *die, struct cu *cu, struc
 		function = NULL;
 	}
 
+	if (function != NULL &&
+	    add_gnu_annotation_chain(die, -1, conf, &function->annots) != 0) {
+		function__delete(function, cu);
+		function = NULL;
+	}
+
 	return function ? &function->proto.tag : NULL;
 }
 
@@ -2469,6 +2562,9 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
 		 */
 		tag = &unsupported_tag;
 		break;
+	case DW_TAG_GNU_annotation:
+		tag = &unsupported_tag;
+		break;
 	case DW_TAG_label:
 		if (conf->ignore_labels)
 			tag = &unsupported_tag; // callers will assume conf->ignore_labels is true
@@ -2494,7 +2590,8 @@ static int die__process_unit(Dwarf_Die *die, struct cu *cu, struct conf_load *co
 			// XXX special case DW_TAG_dwarf_procedure, appears when looking at a recent ~/bin/perf
 			// Investigate later how to properly support this...
 			if (dwarf_tag(die) != DW_TAG_dwarf_procedure &&
-			    dwarf_tag(die) != DW_TAG_label) // conf->ignore_labels == true, see die__process_tag()
+			    dwarf_tag(die) != DW_TAG_label && // conf->ignore_labels == true, see die__process_tag()
+			    dwarf_tag(die) != DW_TAG_GNU_annotation)
 				tag__print_not_supported(die);
 			continue;
 		}
diff --git a/dwarves.h b/dwarves.h
index 5ec16e750e83..42b8e39aa2dd 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -670,7 +670,8 @@ static inline int tag__is_tag_type(const struct tag *tag)
 	       tag->tag == DW_TAG_volatile_type ||
 	       tag->tag == DW_TAG_atomic_type ||
 	       tag->tag == DW_TAG_unspecified_type ||
-	       tag->tag == DW_TAG_LLVM_annotation;
+	       tag->tag == DW_TAG_LLVM_annotation ||
+	       tag->tag == DW_TAG_GNU_annotation;
 }
 
 static inline const char *tag__decl_file(const struct tag *tag,
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index 1ec478c2a027..a514d7e98923 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -140,6 +140,8 @@ const char *dwarf_tag_name(const uint32_t tag)
 		return dwarf_gnu_tag_names[tag - DW_TAG_MIPS_loop];
 	else if (tag == DW_TAG_LLVM_annotation)
 		return "LLVM_annotation";
+	else if (tag == DW_TAG_GNU_annotation)
+		return "GNU_annotation";
 	return "INVALID";
 }
 
@@ -658,6 +660,7 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu,
 		snprintf(bf, len, "%s", variable__name(tag__variable(tag)));
 		break;
 	case DW_TAG_LLVM_annotation:
+	case DW_TAG_GNU_annotation:
 		type = cu__type(cu, tag->type);
 		if (type == NULL && tag->type != 0)
 			tag__id_not_found_snprintf(bf, len, tag->type);
@@ -731,7 +734,7 @@ static type_id_t skip_llvm_annotations(const struct cu *cu, type_id_t id)
 		if (id == 0)
 			break;
 		type = cu__type(cu, id);
-		if (type == NULL || type->tag != DW_TAG_LLVM_annotation || type->type == id)
+		if (type == NULL || (type->tag != DW_TAG_LLVM_annotation && type->tag != DW_TAG_GNU_annotation) || type->type == id)
 			break;
 		id = type->type;
 	}
@@ -936,7 +939,8 @@ print_modifier: {
 		else
 			printed += enumeration__fprintf(type, &tconf, fp);
 		break;
-	case DW_TAG_LLVM_annotation: {
+	case DW_TAG_LLVM_annotation:
+	case DW_TAG_GNU_annotation: {
 		struct tag *ttype = cu__type(cu, type->type);
 		if (ttype) {
 			type = ttype;
diff --git a/tests/pfunct-btf-decl-tags.sh b/tests/pfunct-btf-decl-tags.sh
index 35884b4e8687..5fdddc09f179 100755
--- a/tests/pfunct-btf-decl-tags.sh
+++ b/tests/pfunct-btf-decl-tags.sh
@@ -13,17 +13,21 @@ trap cleanup EXIT
 
 title_log "Check that pfunct can print btf_decl_tags read from BTF."
 
-# gcc now also supports decl tags as of gcc commit 43dcea48b8c,
-# in upstream version 16.
-# UPTODO: add a check here for that.
+# gcc 16+ supports decl tags via DW_TAG_GNU_annotation (gcc commit ac7027f180b).
+# Use gcc if available and version >= 16, otherwise fall back to clang.
 
+GCC=${GCC:-gcc}
 CLANG=${CLANG:-clang}
-if ! command -v $CLANG > /dev/null; then
-	error_log "Need clang for test $0"
-	test_fail
+
+use_gcc=0
+if command -v $GCC > /dev/null; then
+	gcc_ver=$($GCC -dumpversion 2>/dev/null | cut -d. -f1)
+	if [ "$gcc_ver" -ge 16 ] 2>/dev/null; then
+		use_gcc=1
+	fi
 fi
 
-(cat <<EOF
+src=$(cat <<EOF
 #define __tag(x) __attribute__((btf_decl_tag(#x)))
 
 __tag(a) __tag(b) __tag(c) void foo(void) {}
@@ -31,7 +35,19 @@ __tag(a) __tag(b)          void bar(void) {}
 __tag(a)                   void buz(void) {}
 
 EOF
-) | $CLANG --target=bpf -c -g -x c -o $tmpobj -
+)
+
+if [ "$use_gcc" -eq 1 ]; then
+	info_log "Using $GCC (version $gcc_ver) for btf_decl_tag test"
+	echo "$src" | $GCC -c -g -x c -o $tmpobj - 2>/dev/null
+	pahole -J $tmpobj 2>/dev/null
+elif command -v $CLANG > /dev/null; then
+	info_log "Using $CLANG for btf_decl_tag test"
+	echo "$src" | $CLANG --target=bpf -c -g -x c -o $tmpobj -
+else
+	error_log "Need gcc >= 16 or clang for test $0"
+	test_fail
+fi
 
 # tags order is not guaranteed
 sort_tags=$(cat <<EOF
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC]
  2026-05-28 22:36 [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC] Vineet Gupta
  2026-05-28 22:36 ` [PAHOLE Patch v2 2/2] Add support for DW_TAG_GNU_annotation Vineet Gupta
@ 2026-05-29 13:23 ` Alan Maguire
  2026-05-29 18:11   ` Vineet Gupta
  1 sibling, 1 reply; 7+ messages in thread
From: Alan Maguire @ 2026-05-29 13:23 UTC (permalink / raw)
  To: Vineet Gupta, dwarves
  Cc: bpf, Andrii Nakryiko, acme, jose.marchesi, David Faust

On 28/05/2026 23:36, Vineet Gupta wrote:
> NFC change preparing for DW_TAG_GNU_annotation support.
> Extract the btf_type_tag annotation creation logic from into a helper
> die__add_btf_type_tag().
> 
> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
> ---
> Changes since v1 [1]
>  - NFC reinstate some original comments
> 
> [1] https://lore.kernel.org/bpf/20260526181818.4159927-1-vineet.gupta@linux.dev/
> ---
>  dwarf_loader.c | 58 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 16fb7becffee..8fc9a6794f2f 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -1600,14 +1600,45 @@ static struct btf_type_tag_type *die__create_new_btf_type_tag_type(Dwarf_Die *di
>  	return tag;
>  }
>  

looks good Vineet! One small suggestion:

> +static int die__add_btf_type_tag(struct btf_type_tag_ptr_type **tagp,
> +				 Dwarf_Die *die, Dwarf_Die *adie,
> +				 struct cu *cu, struct conf_load *conf)

maybe just change the function to return the struct btf_type_tag_ptr_type *, 
since consumers just check for non-zero rather than doing anything with the
error code?


> +{
> +	struct btf_type_tag_type *annot;
> +	uint32_t id;
> +
> +	if (*tagp == NULL) {
> +		/* Create a btf_type_tag_ptr type. */
> +		*tagp = die__create_new_btf_type_tag_ptr_type(die, cu);
> +		if (!*tagp)
> +			return -ENOMEM;
> +	}
> +
> +	/* Create a btf_type_tag type for this annotation. */
> +	annot = die__create_new_btf_type_tag_type(adie, cu, conf);
> +	if (annot == NULL)
> +		return -ENOMEM;
> +
> +	if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
> +		return -ENOMEM;
> +
> +	struct dwarf_tag *dtag = tag__dwarf(&annot->tag);
> +	dtag->small_id = id;
> +	cu__hash(cu, &annot->tag);
> +
> +	/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
> +	 * the tag->tags contains tag3 -> tag2 -> tag1.
> +	 */
> +	list_add(&annot->node, &(*tagp)->tags);
> +	return 0;
> +}
> +
>  static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
>  					       struct conf_load *conf)
>  {
>  	struct btf_type_tag_ptr_type *tag = NULL;
> -	struct btf_type_tag_type *annot;
>  	Dwarf_Die *cdie, child;
>  	const char *name;
> -	uint32_t id;
>  
>  	/* If no child tags or skipping btf_type_tag encoding, just create a new tag
>  	 * and return
> @@ -1627,29 +1658,8 @@ static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
>  		if (strcmp(name, "btf_type_tag") != 0)
>  			continue;
>  
> -		if (tag == NULL) {
> -			/* Create a btf_type_tag_ptr type. */
> -			tag = die__create_new_btf_type_tag_ptr_type(die, cu);
> -			if (!tag)
> -				return NULL;
> -		}
> -
> -		/* Create a btf_type_tag type for this annotation. */
> -		annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
> -		if (annot == NULL)
> +		if (die__add_btf_type_tag(&tag, die, cdie, cu, conf))
>  			return NULL;
> -
> -		if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
> -			return NULL;
> -
> -		struct dwarf_tag *dtag = tag__dwarf(&annot->tag);
> -		dtag->small_id = id;
> -		cu__hash(cu, &annot->tag);
> -
> -		/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
> -		 * the tag->tags contains tag3 -> tag2 -> tag1.
> -		 */
> -		list_add(&annot->node, &tag->tags);
>  	} while (dwarf_siblingof(cdie, cdie) == 0);
>  
>  	return tag ? &tag->tag : tag__new(die, cu);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PAHOLE Patch v2 2/2] Add support for DW_TAG_GNU_annotation
  2026-05-28 22:36 ` [PAHOLE Patch v2 2/2] Add support for DW_TAG_GNU_annotation Vineet Gupta
@ 2026-05-29 13:37   ` Alan Maguire
  2026-05-29 23:32     ` Vineet Gupta
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Maguire @ 2026-05-29 13:37 UTC (permalink / raw)
  To: Vineet Gupta, dwarves
  Cc: bpf, Andrii Nakryiko, acme, jose.marchesi, David Faust

On 28/05/2026 23:36, Vineet Gupta wrote:
> gcc 16 was first release to support DW_TAG_GNU_annotation and this patch
> enables the same in pahole. Bulk of changes are in dwarf_loader but
> btf_encoder also gains support with minimal changes.
> 
> GCC encodes btf_type_tag and btf_decl_tag annotations differently from
> LLVM. While LLVM uses DW_TAG_LLVM_annotation (0x6000) as child DIEs,
> GCC uses DW_TAG_GNU_annotation (0x6001) as standalone sibling DIEs
> referenced via DW_AT_GNU_annotation (0x2139) attributes, with chaining
> through the same attribute on annotation DIEs themselves.
> 
> Handle both encoding styles:
> 
> For btf_type_tag (pointer annotations):
> - Recognize DW_TAG_GNU_annotation alongside DW_TAG_LLVM_annotation in
>   child annotation scanning.
> - Follow DW_AT_GNU_annotation attribute chains on pointer types for
>   GCC-style btf_type_tag resolution, with cycle detection.
> - Normalize DW_TAG_GNU_annotation to DW_TAG_LLVM_annotation in the
>   internal representation so downstream code works unchanged.
> 
> For btf_decl_tag (function/struct/member annotations):
> - Add add_gnu_annotation_chain() to follow DW_AT_GNU_annotation
>   attribute chains on function, struct, and member DIEs.
> - GCC puts DW_AT_GNU_annotation on the function/struct DIE itself
>   (not as child DIEs), referencing sibling annotation DIEs that chain
>   via the same attribute.
> 
> Also:
> - Silently skip standalone DW_TAG_GNU_annotation DIEs at CU level.
> - Update pfunct-btf-decl-tags.sh test to use GCC 16+ when available and
>   now passes.
> 
> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>

thanks for this Vineet! I'm building gcc-16 to manually test now but I have a few
suggestions below in the meantime. Changes look good in CI [1]

[1] https://github.com/alan-maguire/dwarves/actions/runs/26632028538

> ---
> Changes since v1 [1]
>  - NFC Reduce indentation with early exits (Alexei offlist)
> 
> [1] https://lore.kernel.org/bpf/20260526181818.4159927-2-vineet.gupta@linux.dev/
> ---
>  btf_encoder.c                 |   1 +
>  dutil.h                       |   8 +++
>  dwarf_loader.c                | 115 +++++++++++++++++++++++++++++++---
>  dwarves.h                     |   3 +-
>  dwarves_fprintf.c             |   8 ++-
>  tests/pfunct-btf-decl-tags.sh |  32 +++++++---
>  6 files changed, 147 insertions(+), 20 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 633bc6162ce0..d5af706d7638 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -1831,6 +1831,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
>  		name = namespace__name(tag__namespace(tag));
>  		return btf_encoder__add_ref_type(encoder, BTF_KIND_TYPEDEF, ref_type_id, name, false);
>  	case DW_TAG_LLVM_annotation:
> +	case DW_TAG_GNU_annotation:
>  		name = tag__btf_type_tag(tag)->value;
>  		return btf_encoder__add_ref_type(encoder, BTF_KIND_TYPE_TAG, ref_type_id, name, false);
>  	case DW_TAG_structure_type:
> diff --git a/dutil.h b/dutil.h
> index ff78aa6dfd10..abe0e62b412f 100644
> --- a/dutil.h
> +++ b/dutil.h
> @@ -35,6 +35,14 @@
>  #define DW_TAG_LLVM_annotation 0x6000
>  #endif
>  
> +#ifndef DW_TAG_GNU_annotation
> +#define DW_TAG_GNU_annotation 0x6001
> +#endif
> +
> +#ifndef DW_AT_GNU_annotation
> +#define DW_AT_GNU_annotation 0x2139
> +#endif
> +
>  static inline __attribute__((const)) bool is_power_of_2(unsigned long n)
>  {
>          return (n != 0 && ((n & (n - 1)) == 0));
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 8fc9a6794f2f..a3d51e30703c 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -908,6 +908,12 @@ static int tag__recode_dwarf_bitfield(struct tag *tag, struct cu *cu, uint16_t b
>  	return -ENOMEM;
>  }
>  
> +static bool die__tag_is_annotation(Dwarf_Die *die)
> +{
> +	unsigned int tag = dwarf_tag(die);
> +	return tag == DW_TAG_LLVM_annotation || tag == DW_TAG_GNU_annotation;
> +}
> +
>  static int add_llvm_annotation(Dwarf_Die *die, int component_idx, struct conf_load *conf,
>  			       struct list_head *head)
>  {
> @@ -943,7 +949,7 @@ static int add_child_llvm_annotations(Dwarf_Die *die, int component_idx,
>  
>  	die = &child;
>  	do {
> -		if (dwarf_tag(die) == DW_TAG_LLVM_annotation) {
> +		if (die__tag_is_annotation(die)) {
>  			ret = add_llvm_annotation(die, component_idx, conf, head);
>  			if (ret)
>  				return ret;
> @@ -953,6 +959,32 @@ static int add_child_llvm_annotations(Dwarf_Die *die, int component_idx,
>  	return 0;
>  }
>  
> +static int add_gnu_annotation_chain(Dwarf_Die *die, int component_idx,
> +				    struct conf_load *conf, struct list_head *head)
> +{
> +	Dwarf_Attribute attr;
> +	Dwarf_Die annot_die;
> +
> +	if (dwarf_attr(die, DW_AT_GNU_annotation, &attr) == NULL ||
> +	    dwarf_formref_die(&attr, &annot_die) == NULL)
> +		return 0;
> +
> +	for (;;) {
> +		if (dwarf_tag(&annot_die) != DW_TAG_GNU_annotation)
> +			break;
> +
> +		int ret = add_llvm_annotation(&annot_die, component_idx, conf, head);
> +		if (ret)
> +			return ret;
> +
> +		if (dwarf_attr(&annot_die, DW_AT_GNU_annotation, &attr) == NULL ||
> +		    dwarf_formref_die(&attr, &annot_die) == NULL)
> +			break;
> +	}

the pointer tag handling below uses bookkeeping to track visited dies; is there a need
for that here too? maybe a bit paranoid but perhaps we could haul the loop detection
logic out and reuse in both places?


> +
> +	return 0;
> +}
> +
>  int class_member__dwarf_recode_bitfield(struct class_member *member,
>  					struct cu *cu)
>  {
> @@ -1596,6 +1628,8 @@ static struct btf_type_tag_type *die__create_new_btf_type_tag_type(Dwarf_Die *di
>  		return NULL;
>  
>  	tag__init(&tag->tag, cu, die);
> +	/* Normalize DW_TAG_GNU_annotation to DW_TAG_LLVM_annotation internally */
> +	tag->tag.tag = DW_TAG_LLVM_annotation;
>  	tag->value = attr_string(die, DW_AT_const_value, conf);
>  	return tag;
>  }
> @@ -1638,19 +1672,21 @@ static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
>  {
>  	struct btf_type_tag_ptr_type *tag = NULL;
>  	Dwarf_Die *cdie, child;
> +	Dwarf_Attribute attr;
> +	Dwarf_Die annot_die;
>  	const char *name;
>  
> -	/* If no child tags or skipping btf_type_tag encoding, just create a new tag
> -	 * and return
> -	 */
> -	if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
> -	    conf->skip_encoding_btf_type_tag)
> +	/* If skipping btf_type_tag encoding, just create a new tag, return */
> +	if (conf->skip_encoding_btf_type_tag)
>  		return tag__new(die, cu);
>  
> -	/* Otherwise, check DW_TAG_LLVM_annotation child tags */
> +	/* Handle LLVM style annotation tags if present */
> +	if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0)
> +		goto check_gnu_attr;
> +
>  	cdie = &child;
>  	do {
> -		if (dwarf_tag(cdie) != DW_TAG_LLVM_annotation)
> +		if (!die__tag_is_annotation(cdie))
>  			continue;
>  
>  		/* Only check btf_type_tag annotations */
> @@ -1662,6 +1698,47 @@ static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
>  			return NULL;
>  	} while (dwarf_siblingof(cdie, cdie) == 0);
>  
> +check_gnu_attr:
> +	/* Check for GCC-style DW_AT_GNU_annotation attribute */
> +	if (tag != NULL ||
> +	    dwarf_attr(die, DW_AT_GNU_annotation, &attr) == NULL ||
> +	    dwarf_formref_die(&attr, &annot_die) == NULL)
> +		goto out;
> +
> +	Dwarf_Off visited[256];
> +	int nr_visited = 0;
> +
> +	for (;;) {
> +		Dwarf_Off off = dwarf_dieoffset(&annot_die);
> +		bool cycle = false;
> +		int i;
> +
> +		for (i = 0; i < nr_visited; i++) {
> +			if (visited[i] == off) {
> +				cycle = true;
> +				break;
> +			}
> +		}
> +		if (cycle || nr_visited >= (int)ARRAY_SIZE(visited))
> +			break;
> +		visited[nr_visited++] = off;
> +
> +		if (dwarf_tag(&annot_die) != DW_TAG_GNU_annotation)
> +			break;
> +
> +		name = attr_string(&annot_die, DW_AT_name, conf);
> +		if (strcmp(name, "btf_type_tag") != 0)
> +			break;

should we "continue;" here instead; if we encounter a non-type tag annotation
might we miss subsequent btf type tag annotations? might not be an issue today
but might be more future-proof to continue here maybe?
> +
> +		if (die__add_btf_type_tag(&tag, die, &annot_die, cu, conf))
> +			return NULL;
> +
> +		if (dwarf_attr(&annot_die, DW_AT_GNU_annotation, &attr) == NULL ||
> +		    dwarf_formref_die(&attr, &annot_die) == NULL)
> +			break;
> +	}
> +
> +out:
>  	return tag ? &tag->tag : tag__new(die, cu);
>  }
>  
> @@ -1690,6 +1767,12 @@ static struct tag *die__create_new_class(Dwarf_Die *die, struct cu *cu, struct c
>  		}
>  	}
>  
> +	if (class != NULL &&
> +	    add_gnu_annotation_chain(die, -1, conf, &class->type.namespace.annots) != 0) {
> +		class__delete(class, cu);
> +		class = NULL;
> +	}
> +
>  	return class ? &class->type.namespace.tag : NULL;
>  }
>  
> @@ -2051,10 +2134,13 @@ static int die__process_class(Dwarf_Die *die, struct type *class,
>  			cu__hash(cu, &member->tag);
>  			if (add_child_llvm_annotations(die, member_idx, conf, &class->namespace.annots))
>  				return -ENOMEM;
> +			if (add_gnu_annotation_chain(die, member_idx, conf, &class->namespace.annots))
> +				return -ENOMEM;
>  			member_idx++;
>  		}
>  			continue;
>  		case DW_TAG_LLVM_annotation:
> +		case DW_TAG_GNU_annotation:
>  			if (add_llvm_annotation(die, -1, conf, &class->namespace.annots))
>  				return -ENOMEM;
>  			continue;
> @@ -2360,6 +2446,7 @@ static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
>  				goto out_enomem;
>  			continue;
>  		case DW_TAG_LLVM_annotation:
> +		case DW_TAG_GNU_annotation:
>  			if (add_llvm_annotation(die, -1, conf, &(tag__function(&ftype->tag)->annots)))
>  				goto out_enomem;
>  			continue;
> @@ -2408,6 +2495,12 @@ static struct tag *die__create_new_function(Dwarf_Die *die, struct cu *cu, struc
>  		function = NULL;
>  	}
>  
> +	if (function != NULL &&
> +	    add_gnu_annotation_chain(die, -1, conf, &function->annots) != 0) {
> +		function__delete(function, cu);
> +		function = NULL;
> +	}
> +
>  	return function ? &function->proto.tag : NULL;
>  }
>  
> @@ -2469,6 +2562,9 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
>  		 */
>  		tag = &unsupported_tag;
>  		break;
> +	case DW_TAG_GNU_annotation:
> +		tag = &unsupported_tag;
> +		break;
>  	case DW_TAG_label:
>  		if (conf->ignore_labels)
>  			tag = &unsupported_tag; // callers will assume conf->ignore_labels is true
> @@ -2494,7 +2590,8 @@ static int die__process_unit(Dwarf_Die *die, struct cu *cu, struct conf_load *co
>  			// XXX special case DW_TAG_dwarf_procedure, appears when looking at a recent ~/bin/perf
>  			// Investigate later how to properly support this...
>  			if (dwarf_tag(die) != DW_TAG_dwarf_procedure &&
> -			    dwarf_tag(die) != DW_TAG_label) // conf->ignore_labels == true, see die__process_tag()
> +			    dwarf_tag(die) != DW_TAG_label && // conf->ignore_labels == true, see die__process_tag()
> +			    dwarf_tag(die) != DW_TAG_GNU_annotation)
>  				tag__print_not_supported(die);
>  			continue;
>  		}
> diff --git a/dwarves.h b/dwarves.h
> index 5ec16e750e83..42b8e39aa2dd 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -670,7 +670,8 @@ static inline int tag__is_tag_type(const struct tag *tag)
>  	       tag->tag == DW_TAG_volatile_type ||
>  	       tag->tag == DW_TAG_atomic_type ||
>  	       tag->tag == DW_TAG_unspecified_type ||
> -	       tag->tag == DW_TAG_LLVM_annotation;
> +	       tag->tag == DW_TAG_LLVM_annotation ||
> +	       tag->tag == DW_TAG_GNU_annotation;
>  }
>  
>  static inline const char *tag__decl_file(const struct tag *tag,
> diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
> index 1ec478c2a027..a514d7e98923 100644
> --- a/dwarves_fprintf.c
> +++ b/dwarves_fprintf.c
> @@ -140,6 +140,8 @@ const char *dwarf_tag_name(const uint32_t tag)
>  		return dwarf_gnu_tag_names[tag - DW_TAG_MIPS_loop];
>  	else if (tag == DW_TAG_LLVM_annotation)
>  		return "LLVM_annotation";
> +	else if (tag == DW_TAG_GNU_annotation)
> +		return "GNU_annotation";
>  	return "INVALID";
>  }
>  
> @@ -658,6 +660,7 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu,
>  		snprintf(bf, len, "%s", variable__name(tag__variable(tag)));
>  		break;
>  	case DW_TAG_LLVM_annotation:
> +	case DW_TAG_GNU_annotation:
>  		type = cu__type(cu, tag->type);
>  		if (type == NULL && tag->type != 0)
>  			tag__id_not_found_snprintf(bf, len, tag->type);
> @@ -731,7 +734,7 @@ static type_id_t skip_llvm_annotations(const struct cu *cu, type_id_t id)
>  		if (id == 0)
>  			break;
>  		type = cu__type(cu, id);
> -		if (type == NULL || type->tag != DW_TAG_LLVM_annotation || type->type == id)
> +		if (type == NULL || (type->tag != DW_TAG_LLVM_annotation && type->tag != DW_TAG_GNU_annotation) || type->type == id)
>  			break;
>  		id = type->type;
>  	}
> @@ -936,7 +939,8 @@ print_modifier: {
>  		else
>  			printed += enumeration__fprintf(type, &tconf, fp);
>  		break;
> -	case DW_TAG_LLVM_annotation: {
> +	case DW_TAG_LLVM_annotation:
> +	case DW_TAG_GNU_annotation: {
>  		struct tag *ttype = cu__type(cu, type->type);
>  		if (ttype) {
>  			type = ttype;
> diff --git a/tests/pfunct-btf-decl-tags.sh b/tests/pfunct-btf-decl-tags.sh
> index 35884b4e8687..5fdddc09f179 100755
> --- a/tests/pfunct-btf-decl-tags.sh
> +++ b/tests/pfunct-btf-decl-tags.sh
> @@ -13,17 +13,21 @@ trap cleanup EXIT
>  
>  title_log "Check that pfunct can print btf_decl_tags read from BTF."
>  
> -# gcc now also supports decl tags as of gcc commit 43dcea48b8c,
> -# in upstream version 16.
> -# UPTODO: add a check here for that.
> +# gcc 16+ supports decl tags via DW_TAG_GNU_annotation (gcc commit ac7027f180b).
> +# Use gcc if available and version >= 16, otherwise fall back to clang.
>  

Is there a reason why we couldn't do both? We'd ideally have CI test both clang
and gcc so wouldn't want to skip one or the other really. I'd suggest we lose
the elif logic below and test with whatever is available gcc, clang or both
ideally. We can fix up CI to install gcc 16 later.

Also I'd suggest move the test to a separate patch. Thanks!


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC]
  2026-05-29 13:23 ` [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC] Alan Maguire
@ 2026-05-29 18:11   ` Vineet Gupta
  0 siblings, 0 replies; 7+ messages in thread
From: Vineet Gupta @ 2026-05-29 18:11 UTC (permalink / raw)
  To: Alan Maguire, dwarves
  Cc: bpf, Andrii Nakryiko, acme, jose.marchesi, David Faust

Hi Alan,


On 5/29/26 06:23, Alan Maguire wrote:
> looks good Vineet! One small suggestion:
>
>> +static int die__add_btf_type_tag(struct btf_type_tag_ptr_type **tagp,
>> +				 Dwarf_Die *die, Dwarf_Die *adie,
>> +				 struct cu *cu, struct conf_load *conf)
> maybe just change the function to return the struct btf_type_tag_ptr_type *,
> since consumers just check for non-zero rather than doing anything with the
> error code?

OK will do.

Thx for taking a look.
-Vineet

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PAHOLE Patch v2 2/2] Add support for DW_TAG_GNU_annotation
  2026-05-29 13:37   ` Alan Maguire
@ 2026-05-29 23:32     ` Vineet Gupta
  2026-05-30  1:31       ` Vineet Gupta
  0 siblings, 1 reply; 7+ messages in thread
From: Vineet Gupta @ 2026-05-29 23:32 UTC (permalink / raw)
  To: Alan Maguire, dwarves
  Cc: bpf, Andrii Nakryiko, acme, jose.marchesi, David Faust

On 5/29/26 06:37, Alan Maguire wrote:
> thanks for this Vineet! I'm building gcc-16 to manually test now but I have a few
> suggestions below in the meantime. Changes look good in CI [1]
>
> [1] https://github.com/alan-maguire/dwarves/actions/runs/26632028538

Cool.
>> +static int add_gnu_annotation_chain(Dwarf_Die *die, int component_idx,
>> +				    struct conf_load *conf, struct list_head *head)
>> +{
>> +	Dwarf_Attribute attr;
>> +	Dwarf_Die annot_die;
>> +
>> +	if (dwarf_attr(die, DW_AT_GNU_annotation, &attr) == NULL ||
>> +	    dwarf_formref_die(&attr, &annot_die) == NULL)
>> +		return 0;
>> +
>> +	for (;;) {
>> +		if (dwarf_tag(&annot_die) != DW_TAG_GNU_annotation)
>> +			break;
>> +
>> +		int ret = add_llvm_annotation(&annot_die, component_idx, conf, head);
>> +		if (ret)
>> +			return ret;
>> +
>> +		if (dwarf_attr(&annot_die, DW_AT_GNU_annotation, &attr) == NULL ||
>> +		    dwarf_formref_die(&attr, &annot_die) == NULL)
>> +			break;
>> +	}
> the pointer tag handling below uses bookkeeping to track visited dies; is there a need
> for that here too? maybe a bit paranoid but perhaps we could haul the loop detection
> logic out and reuse in both places?

Looking back now, the loop detection seems excessive. It was from the 
initial attempt to triage what seemed like an infinite loop in pahole 
but just turned out to be excessive match failures and bails and retries 
due to the gcc bug [1] which was generating variants of same core 
struct, causing failures pretty much everywhere due to the core struct 
being embedded all over. If we want to add loop detection that should be 
addon as it unconditional additional processing. I agree that even with 
that out of picture this function and the opencoded check_gnu_attr code 
seem to share a lot of structure - but they have their own *thing* to do 
and it would be unwieldy and awkward to factor out into a shared loop 
body with a call back or an additional arg to call add_llvm_annotation 
or die__add_btf_type_tag for either of those cases.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125421 
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125421>

>> +check_gnu_attr:
>> +	/* Check for GCC-style DW_AT_GNU_annotation attribute */
>> +	if (tag != NULL ||
>> +	    dwarf_attr(die, DW_AT_GNU_annotation, &attr) == NULL ||
>> +	    dwarf_formref_die(&attr, &annot_die) == NULL)
>> +		goto out;
>> +
>> +	Dwarf_Off visited[256];
>> +	int nr_visited = 0;
>> +
>> +	for (;;) {
>> +		Dwarf_Off off = dwarf_dieoffset(&annot_die);
>> +		bool cycle = false;
>> +		int i;
>> +
>> +		for (i = 0; i < nr_visited; i++) {
>> +			if (visited[i] == off) {
>> +				cycle = true;
>> +				break;
>> +			}
>> +		}
>> +		if (cycle || nr_visited >= (int)ARRAY_SIZE(visited))
>> +			break;
>> +		visited[nr_visited++] = off;

I'm thinking of ripping this all out. Rest of dwarf machinery feels 
robust enough.

>> +
>> +		if (dwarf_tag(&annot_die) != DW_TAG_GNU_annotation)
>> +			break;
>> +
>> +		name = attr_string(&annot_die, DW_AT_name, conf);
>> +		if (strcmp(name, "btf_type_tag") != 0)
>> +			break;
> should we "continue;" here instead; if we encounter a non-type tag annotation
> might we miss subsequent btf type tag annotations? might not be an issue today
> but might be more future-proof to continue here maybe?

Good point. Will do.

>
>> +
>> +		if (die__add_btf_type_tag(&tag, die, &annot_die, cu, conf))
>> +			return NULL;
>> +
>> +		if (dwarf_attr(&annot_die, DW_AT_GNU_annotation, &attr) == NULL ||
>> +		    dwarf_formref_die(&attr, &annot_die) == NULL)
>> +			break;
>> +	}
>> +
>> +out:
>>   	return tag ? &tag->tag : tag__new(die, cu);
>>   }

[snip]

>>   
>>
>> diff --git a/tests/pfunct-btf-decl-tags.sh b/tests/pfunct-btf-decl-tags.sh
>> index 35884b4e8687..5fdddc09f179 100755
>> --- a/tests/pfunct-btf-decl-tags.sh
>> +++ b/tests/pfunct-btf-decl-tags.sh
>> @@ -13,17 +13,21 @@ trap cleanup EXIT
>>   
>>   title_log "Check that pfunct can print btf_decl_tags read from BTF."
>>   
>> -# gcc now also supports decl tags as of gcc commit 43dcea48b8c,
>> -# in upstream version 16.
>> -# UPTODO: add a check here for that.
>> +# gcc 16+ supports decl tags via DW_TAG_GNU_annotation (gcc commit ac7027f180b).
>> +# Use gcc if available and version >= 16, otherwise fall back to clang.
>>   
> Is there a reason why we couldn't do both? We'd ideally have CI test both clang
> and gcc so wouldn't want to skip one or the other really. I'd suggest we lose
> the elif logic below and test with whatever is available gcc, clang or both
> ideally. We can fix up CI to install gcc 16 later.

OK.

> Also I'd suggest move the test to a separate patch. Thanks!

The test was already there, just not wired up for gcc. So it seems the 
ask is to leave it disabled here do a separate patch for enabling. Sure 
that works too.

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PAHOLE Patch v2 2/2] Add support for DW_TAG_GNU_annotation
  2026-05-29 23:32     ` Vineet Gupta
@ 2026-05-30  1:31       ` Vineet Gupta
  0 siblings, 0 replies; 7+ messages in thread
From: Vineet Gupta @ 2026-05-30  1:31 UTC (permalink / raw)
  To: Alan Maguire, dwarves
  Cc: bpf, Andrii Nakryiko, acme, jose.marchesi, David Faust

On 5/29/26 16:32, Vineet Gupta wrote:
> On 5/29/26 06:37, Alan Maguire wrote:
>> thanks for this Vineet! I'm building gcc-16 to manually test now but 
>> I have a few
>> suggestions below in the meantime. Changes look good in CI [1]
>>
>> [1] https://github.com/alan-maguire/dwarves/actions/runs/26632028538
>
> Cool.
>>> +static int add_gnu_annotation_chain(Dwarf_Die *die, int component_idx,
>>> +                    struct conf_load *conf, struct list_head *head)
>>> +{
>>> +    Dwarf_Attribute attr;
>>> +    Dwarf_Die annot_die;
>>> +
>>> +    if (dwarf_attr(die, DW_AT_GNU_annotation, &attr) == NULL ||
>>> +        dwarf_formref_die(&attr, &annot_die) == NULL)
>>> +        return 0;
>>> +
>>> +    for (;;) {
>>> +        if (dwarf_tag(&annot_die) != DW_TAG_GNU_annotation)
>>> +            break;
>>> +
>>> +        int ret = add_llvm_annotation(&annot_die, component_idx, 
>>> conf, head);
>>> +        if (ret)
>>> +            return ret;
>>> +
>>> +        if (dwarf_attr(&annot_die, DW_AT_GNU_annotation, &attr) == 
>>> NULL ||
>>> +            dwarf_formref_die(&attr, &annot_die) == NULL)
>>> +            break;
>>> +    }
>> the pointer tag handling below uses bookkeeping to track visited 
>> dies; is there a need
>> for that here too? maybe a bit paranoid but perhaps we could haul the 
>> loop detection
>> logic out and reuse in both places?
>
> Looking back now, the loop detection seems excessive. It was from the 
> initial attempt to triage what seemed like an infinite loop in pahole 
> but just turned out to be excessive match failures and bails and 
> retries due to the gcc bug [1] which was generating variants of same 
> core struct, causing failures pretty much everywhere due to the core 
> struct being embedded all over. If we want to add loop detection that 
> should be addon as it unconditional additional processing. I agree 
> that even with that out of picture this function and the opencoded 
> check_gnu_attr code seem to share a lot of structure - but they have 
> their own *thing* to do and it would be unwieldy and awkward to factor 
> out into a shared loop body with a call back or an additional arg to 
> call add_llvm_annotation or die__add_btf_type_tag for either of those 
> cases.
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125421 
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125421>
>
>>> +check_gnu_attr:
>>> +    /* Check for GCC-style DW_AT_GNU_annotation attribute */
>>> +    if (tag != NULL ||
>>> +        dwarf_attr(die, DW_AT_GNU_annotation, &attr) == NULL ||
>>> +        dwarf_formref_die(&attr, &annot_die) == NULL)
>>> +        goto out;
>>> +
>>> +    Dwarf_Off visited[256];
>>> +    int nr_visited = 0;
>>> +
>>> +    for (;;) {
>>> +        Dwarf_Off off = dwarf_dieoffset(&annot_die);
>>> +        bool cycle = false;
>>> +        int i;
>>> +
>>> +        for (i = 0; i < nr_visited; i++) {
>>> +            if (visited[i] == off) {
>>> +                cycle = true;
>>> +                break;
>>> +            }
>>> +        }
>>> +        if (cycle || nr_visited >= (int)ARRAY_SIZE(visited))
>>> +            break;
>>> +        visited[nr_visited++] = off;
>
> I'm thinking of ripping this all out. Rest of dwarf machinery feels 
> robust enough.
>
>>> +
>>> +        if (dwarf_tag(&annot_die) != DW_TAG_GNU_annotation)
>>> +            break;
>>> +
>>> +        name = attr_string(&annot_die, DW_AT_name, conf);
>>> +        if (strcmp(name, "btf_type_tag") != 0)
>>> +            break;
>> should we "continue;" here instead; if we encounter a non-type tag 
>> annotation
>> might we miss subsequent btf type tag annotations? might not be an 
>> issue today
>> but might be more future-proof to continue here maybe?
>
> Good point. Will do.

Claude suggests break is the right thing to do for precisely the reason 
you mention.
This code only handles DW_TAG_pointer_type DIEs which currently don't 
expect non-type tag, and this pertains to a single chain it could point 
to a potential issue as of now.
But up to you what you feel is better.

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-05-30  1:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-28 22:36 [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC] Vineet Gupta
2026-05-28 22:36 ` [PAHOLE Patch v2 2/2] Add support for DW_TAG_GNU_annotation Vineet Gupta
2026-05-29 13:37   ` Alan Maguire
2026-05-29 23:32     ` Vineet Gupta
2026-05-30  1:31       ` Vineet Gupta
2026-05-29 13:23 ` [PAHOLE Patch v2 1/2] dwarf_loader: Extract die__add_btf_type_tag() helper [NFC] Alan Maguire
2026-05-29 18:11   ` Vineet Gupta

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.