[PATCH dwarves 0/3] btf_encoder: emit type tags for bpf_arena pointers

[PATCH dwarves 0/3] btf_encoder: emit type tags for bpf_arena pointers

[Ihor Solodrai]

This patch series implements emitting appropriate BTF type tags for
argument and return types of kfuncs marked with KF_ARENA_* flags.

For additional context see the description of BPF patch
"bpf: define KF_ARENA_* flags for bpf_arena kfuncs" [1].

The feature depends on recent changes in libbpf [2].

[1] https://lore.kernel.org/bpf/20250206003148.2308659-1-ihor.solodrai@linux.dev/
[2] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/

Ihor Solodrai (3):
  btf_encoder: collect kfuncs info in btf_encoder__new
  btf_encoder: emit type tags for bpf_arena pointers
  pahole: introduce --btf_feature=attributes

 btf_encoder.c | 190 ++++++++++++++++++++++++++++++++++++++++++++------
 dwarves.h     |   1 +
 pahole.c      |  11 +++
 3 files changed, 179 insertions(+), 23 deletions(-)

-- 
2.48.1

[PATCH dwarves 1/3] btf_encoder: collect kfuncs info in btf_encoder__new

[Ihor Solodrai]

From: Ihor Solodrai <ihor.solodrai@pm.me>

btf_encoder__tag_kfuncs() is a post-processing step of BTF encoding,
executed right before BTF is deduped and dumped to the output.

Split btf_encoder__tag_kfuncs() routine in two parts:
  * btf_encoder__collect_kfuncs()
  * btf_encoder__tag_kfuncs()

btf_encoder__collect_kfuncs() reads the .BTF_ids section of the ELF,
collecting kfunc information into a list of kfunc_info structs in the
btf_encoder. It is executed in btf_encoder__new() when tag_kfuncs flag
is set. This way kfunc information is available during entire lifetime
of the btf_encoder.

btf_encoder__tag_kfuncs() is basically the same: collect BTF
functions, and then for each kfunc find and tag correspoding BTF
func. Except now kfunc information is not collected in-place, but is
simply read from the btf_encoder.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c | 89 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 68 insertions(+), 21 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 511c1ea..e9f4baf 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -89,6 +89,7 @@ struct elf_function {
 	const char	*name;
 	char		*alias;
 	size_t		prefixlen;
+	bool		kfunc;
 };
 
 struct elf_secinfo {
@@ -100,6 +101,13 @@ struct elf_secinfo {
 	struct gobuffer secinfo;
 };
 
+struct kfunc_info {
+	struct list_head node;
+	uint32_t id;
+	uint32_t flags;
+	const char* name;
+};
+
 struct elf_functions {
 	struct list_head node; /* for elf_functions_list */
 	Elf *elf; /* source ELF */
@@ -143,6 +151,7 @@ struct btf_encoder {
 	 * so we have to store elf_functions tables per ELF.
 	 */
 	struct list_head elf_functions_list;
+	struct list_head kfuncs; /* list of kfunc_info */
 };
 
 struct btf_func {
@@ -1842,9 +1851,9 @@ static int btf__add_kfunc_decl_tag(struct btf *btf, const char *tag, __u32 id, c
 	return 0;
 }
 
-static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *funcs, const char *kfunc, __u32 flags)
+static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *funcs, const struct kfunc_info *kfunc)
 {
-	struct btf_func key = { .name = kfunc };
+	struct btf_func key = { .name = kfunc->name };
 	struct btf *btf = encoder->btf;
 	struct btf_func *target;
 	const void *base;
@@ -1855,7 +1864,7 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
 	cnt = gobuffer__nr_entries(funcs);
 	target = bsearch(&key, base, cnt, sizeof(key), btf_func_cmp);
 	if (!target) {
-		fprintf(stderr, "%s: failed to find kfunc '%s' in BTF\n", __func__, kfunc);
+		fprintf(stderr, "%s: failed to find kfunc '%s' in BTF\n", __func__, kfunc->name);
 		return -1;
 	}
 
@@ -1864,11 +1873,12 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
 	 * We are ok to do this b/c we will later btf__dedup() to remove
 	 * any duplicates.
 	 */
-	err = btf__add_kfunc_decl_tag(btf, BTF_KFUNC_TYPE_TAG, target->type_id, kfunc);
+	err = btf__add_kfunc_decl_tag(btf, BTF_KFUNC_TYPE_TAG, target->type_id, kfunc->name);
 	if (err < 0)
 		return err;
-	if (flags & KF_FASTCALL) {
-		err = btf__add_kfunc_decl_tag(btf, BTF_FASTCALL_TAG, target->type_id, kfunc);
+
+	if (kfunc->flags & KF_FASTCALL) {
+		err = btf__add_kfunc_decl_tag(btf, BTF_FASTCALL_TAG, target->type_id, kfunc->name);
 		if (err < 0)
 			return err;
 	}
@@ -1876,11 +1886,10 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
 	return 0;
 }
 
-static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
+static int btf_encoder__collect_kfuncs(struct btf_encoder *encoder)
 {
 	const char *filename = encoder->source_filename;
 	struct gobuffer btf_kfunc_ranges = {};
-	struct gobuffer btf_funcs = {};
 	Elf_Data *symbols = NULL;
 	Elf_Data *idlist = NULL;
 	Elf_Scn *symscn = NULL;
@@ -1897,6 +1906,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 	int nr_syms;
 	int i = 0;
 
+	INIT_LIST_HEAD(&encoder->kfuncs);
+
 	fd = open(filename, O_RDONLY);
 	if (fd < 0) {
 		fprintf(stderr, "Cannot open %s\n", filename);
@@ -1977,12 +1988,6 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 	}
 	nr_syms = shdr.sh_size / shdr.sh_entsize;
 
-	err = btf_encoder__collect_btf_funcs(encoder, &btf_funcs);
-	if (err) {
-		fprintf(stderr, "%s: failed to collect BTF funcs\n", __func__);
-		goto out;
-	}
-
 	/* First collect all kfunc set ranges.
 	 *
 	 * Note we choose not to sort these ranges and accept a linear
@@ -2015,12 +2020,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 	for (i = 0; i < nr_syms; i++) {
 		const struct btf_kfunc_set_range *ranges;
 		const struct btf_id_and_flag *pair;
+		struct elf_function *elf_fn;
+		struct kfunc_info *kfunc;
 		unsigned int ranges_cnt;
 		char *func, *name;
 		ptrdiff_t off;
 		GElf_Sym sym;
 		bool found;
-		int err;
 		int j;
 
 		if (!gelf_getsym(symbols, i, &sym)) {
@@ -2061,18 +2067,26 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 			continue;
 		}
 
-		err = btf_encoder__tag_kfunc(encoder, &btf_funcs, func, pair->flags);
-		if (err) {
-			fprintf(stderr, "%s: failed to tag kfunc '%s'\n", __func__, func);
-			free(func);
+		elf_fn = btf_encoder__find_function(encoder, func, 0);
+		free(func);
+		if (!elf_fn)
+			continue;
+		elf_fn->kfunc = true;
+
+		kfunc = calloc(1, sizeof(*kfunc));
+		if (!kfunc) {
+			fprintf(stderr, "%s: failed to allocate memory for kfunc info\n", __func__);
+			err = -ENOMEM;
 			goto out;
 		}
-		free(func);
+		kfunc->id = pair->id;
+		kfunc->flags = pair->flags;
+		kfunc->name = elf_fn->name;
+		list_add(&kfunc->node, &encoder->kfuncs);
 	}
 
 	err = 0;
 out:
-	__gobuffer__delete(&btf_funcs);
 	__gobuffer__delete(&btf_kfunc_ranges);
 	if (elf)
 		elf_end(elf);
@@ -2081,6 +2095,34 @@ out:
 	return err;
 }
 
+static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
+{
+	struct gobuffer btf_funcs = {};
+	int err;
+
+	err = btf_encoder__collect_btf_funcs(encoder, &btf_funcs);
+	if (err) {
+		fprintf(stderr, "%s: failed to collect BTF funcs\n", __func__);
+		goto out;
+	}
+
+	struct kfunc_info *kfunc, *tmp;
+	list_for_each_entry_safe(kfunc, tmp, &encoder->kfuncs, node) {
+		err = btf_encoder__tag_kfunc(encoder, &btf_funcs, kfunc);
+		if (err) {
+			fprintf(stderr, "%s: failed to tag kfunc '%s'\n", __func__, kfunc->name);
+			goto out;
+		}
+		list_del(&kfunc->node);
+		free(kfunc);
+	}
+
+	err = 0;
+out:
+	__gobuffer__delete(&btf_funcs);
+	return err;
+}
+
 int btf_encoder__encode(struct btf_encoder *encoder, struct conf_load *conf)
 {
 	bool should_tag_kfuncs;
@@ -2496,6 +2538,11 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 		if (!found_percpu && encoder->verbose)
 			printf("%s: '%s' doesn't have '%s' section\n", __func__, cu->filename, PERCPU_SECTION);
 
+		if (encoder->tag_kfuncs) {
+			if (btf_encoder__collect_kfuncs(encoder))
+				goto out_delete;
+		}
+
 		if (encoder->verbose)
 			printf("File %s:\n", cu->filename);
 	}
-- 
2.48.1

[PATCH dwarves 2/3] btf_encoder: emit type tags for bpf_arena pointers

[Ihor Solodrai]

When adding a kfunc prototype to BTF, check for the flags indicating
bpf_arena pointers and emit a type tag encoding
__attribute__((address_space(1))) for them. This also requires
updating BTF type ids in the btf_encoder_func_state, which is done as
a side effect in the tagging functions.

This feature depends on recent update in libbpf, supporting arbitrarty
attribute encoding [1].

[1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index e9f4baf..d7837c2 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -40,7 +40,13 @@
 #define BTF_SET8_KFUNCS		(1 << 0)
 #define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
 #define BTF_FASTCALL_TAG       "bpf_fastcall"
-#define KF_FASTCALL            (1 << 12)
+#define BPF_ARENA_ATTR         "address_space(1)"
+
+/* kfunc flags, see include/linux/btf.h in the kernel source */
+#define KF_FASTCALL   (1 << 12)
+#define KF_ARENA_RET  (1 << 13)
+#define KF_ARENA_ARG1 (1 << 14)
+#define KF_ARENA_ARG2 (1 << 15)
 
 struct btf_id_and_flag {
 	uint32_t id;
@@ -743,6 +749,91 @@ static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t tag_t
 	return encoder->type_id_off + tag_type;
 }
 
+static inline struct kfunc_info* btf_encoder__kfunc_by_name(struct btf_encoder *encoder, const char *name) {
+	struct kfunc_info *kfunc;
+
+	list_for_each_entry(kfunc, &encoder->kfuncs, node) {
+		if (strcmp(kfunc->name, name) == 0)
+			return kfunc;
+	}
+	return NULL;
+}
+
+#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
+static int btf_encoder__tag_bpf_arena_ptr(struct btf *btf, int ptr_id)
+{
+	const struct btf_type *ptr;
+	int tagged_type_id;
+
+	ptr = btf__type_by_id(btf, ptr_id);
+	if (!btf_is_ptr(ptr))
+		return -EINVAL;
+
+	tagged_type_id = btf__add_type_attr(btf, BPF_ARENA_ATTR, ptr->type);
+	if (tagged_type_id < 0)
+		return tagged_type_id;
+
+	return btf__add_ptr(btf, tagged_type_id);
+}
+
+static int btf_encoder__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state *state, int idx)
+{
+	int id;
+
+	if (state->nr_parms <= idx)
+		return -EINVAL;
+
+	id = btf_encoder__tag_bpf_arena_ptr(btf, state->parms[idx].type_id);
+	if (id < 0) {
+		btf__log_err(btf, BTF_KIND_TYPE_TAG, BPF_ARENA_ATTR, true, id,
+			"Error adding BPF_ARENA_ATTR for an argument of kfunc '%s'", state->elf->name);
+		return id;
+	}
+	state->parms[idx].type_id = id;
+
+	return id;
+}
+
+static int btf_encoder__add_bpf_arena_type_tags(struct btf_encoder *encoder, struct btf_encoder_func_state *state)
+{
+	struct kfunc_info *kfunc = NULL;
+	int ret_type_id;
+	int err = 0;
+
+	if (!state || !state->elf || !state->elf->kfunc)
+		goto out;
+
+	kfunc = btf_encoder__kfunc_by_name(encoder, state->elf->name);
+	if (!kfunc)
+		goto out;
+
+	if (KF_ARENA_RET & kfunc->flags) {
+		ret_type_id = btf_encoder__tag_bpf_arena_ptr(encoder->btf, state->ret_type_id);
+		if (ret_type_id < 0) {
+			btf__log_err(encoder->btf, BTF_KIND_TYPE_TAG, BPF_ARENA_ATTR, true, ret_type_id,
+				"Error adding BPF_ARENA_ATTR for return type of kfunc '%s'", state->elf->name);
+			err = ret_type_id;
+			goto out;
+		}
+		state->ret_type_id = ret_type_id;
+	}
+
+	if (KF_ARENA_ARG1 & kfunc->flags) {
+		err = btf_encoder__tag_bpf_arena_arg(encoder->btf, state, 0);
+		if (err < 0)
+			goto out;
+	}
+
+	if (KF_ARENA_ARG2 & kfunc->flags) {
+		err = btf_encoder__tag_bpf_arena_arg(encoder->btf, state, 1);
+		if (err < 0)
+			goto out;
+	}
+out:
+	return err;
+}
+#endif // LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
+
 static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype,
 					   struct btf_encoder_func_state *state)
 {
@@ -762,6 +853,10 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f
 		nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
 		type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
 	} else if (state) {
+#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
+		if (btf_encoder__add_bpf_arena_type_tags(encoder, state) < 0)
+			return -1;
+#endif
 		encoder = state->encoder;
 		btf = state->encoder->btf;
 		nr_params = state->nr_parms;
-- 
2.48.1

[PATCH dwarves 3/3] pahole: introduce --btf_feature=attributes

[Ihor Solodrai]

Add a feature flag "attributes" (default: false) controlling whether
pahole is allowed to generate BTF attributes: type tags and decl tags
with kind_flag = 1.

This is necessary for backward compatibility, as BPF verifier does not
recognize tags with kind_flag = 1 prior to (at least) 6.14-rc1 [1].

[1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c |  6 ++++--
 dwarves.h     |  1 +
 pahole.c      | 11 +++++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index d7837c2..0a734d4 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -142,7 +142,8 @@ struct btf_encoder {
 			  gen_floats,
 			  skip_encoding_decl_tag,
 			  tag_kfuncs,
-			  gen_distilled_base;
+			  gen_distilled_base,
+			  encode_attributes;
 	uint32_t	  array_index_id;
 	struct elf_secinfo *secinfo;
 	size_t             seccnt;
@@ -800,7 +801,7 @@ static int btf_encoder__add_bpf_arena_type_tags(struct btf_encoder *encoder, str
 	int ret_type_id;
 	int err = 0;
 
-	if (!state || !state->elf || !state->elf->kfunc)
+	if (!encoder->encode_attributes || !state || !state->elf || !state->elf->kfunc)
 		goto out;
 
 	kfunc = btf_encoder__kfunc_by_name(encoder, state->elf->name);
@@ -2553,6 +2554,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 		encoder->skip_encoding_decl_tag	 = conf_load->skip_encoding_btf_decl_tag;
 		encoder->tag_kfuncs	 = conf_load->btf_decl_tag_kfuncs;
 		encoder->gen_distilled_base = conf_load->btf_gen_distilled_base;
+		encoder->encode_attributes = conf_load->btf_attributes;
 		encoder->verbose	 = verbose;
 		encoder->has_index_type  = false;
 		encoder->need_index_type = false;
diff --git a/dwarves.h b/dwarves.h
index 8234e1a..99ed783 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -89,6 +89,7 @@ struct conf_load {
 	bool			reproducible_build;
 	bool			btf_decl_tag_kfuncs;
 	bool			btf_gen_distilled_base;
+	bool			btf_attributes;
 	uint8_t			hashtable_bits;
 	uint8_t			max_hashtable_bits;
 	uint16_t		kabi_prefix_len;
diff --git a/pahole.c b/pahole.c
index af3e1cf..0bda249 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1152,6 +1152,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
 #define ARG_padding_ge		   347
 #define ARG_padding		   348
 #define ARGP_with_embedded_flexible_array 349
+#define ARGP_btf_attributes	   350
 
 /* --btf_features=feature1[,feature2,..] allows us to specify
  * a list of requested BTF features or "default" to enable all default
@@ -1210,6 +1211,9 @@ struct btf_feature {
 	BTF_NON_DEFAULT_FEATURE(distilled_base, btf_gen_distilled_base, false),
 #endif
 	BTF_NON_DEFAULT_FEATURE(global_var, encode_btf_global_vars, false),
+#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
+	BTF_NON_DEFAULT_FEATURE(attributes, btf_attributes, false),
+#endif
 };
 
 #define BTF_MAX_FEATURE_STR	1024
@@ -1785,6 +1789,11 @@ static const struct argp_option pahole__options[] = {
 		.key = ARGP_running_kernel_vmlinux,
 		.doc = "Search for, possibly getting from a debuginfo server, a vmlinux matching the running kernel build-id (from /sys/kernel/notes)"
 	},
+	{
+		.name = "btf_attributes",
+		.key  = ARGP_btf_attributes,
+		.doc  = "Allow generation of attributes in BTF. Attributes are the type tags and decl tags with the kind_flag set to 1.",
+	},
 	{
 		.name = NULL,
 	}
@@ -1979,6 +1988,8 @@ static error_t pahole__options_parser(int key, char *arg,
 		show_supported_btf_features(stdout);	exit(0);
 	case ARGP_btf_features_strict:
 		parse_btf_features(arg, true);		break;
+	case ARGP_btf_attributes:
+		conf_load.btf_attributes = true;	break;
 	default:
 		return ARGP_ERR_UNKNOWN;
 	}
-- 
2.48.1

Re: [PATCH dwarves 1/3] btf_encoder: collect kfuncs info in btf_encoder__new

[Eduard Zingerman]

On Thu, 2025-02-06 at 18:14 -0800, Ihor Solodrai wrote:
> From: Ihor Solodrai <ihor.solodrai@pm.me>
> 
> btf_encoder__tag_kfuncs() is a post-processing step of BTF encoding,
> executed right before BTF is deduped and dumped to the output.
> 
> Split btf_encoder__tag_kfuncs() routine in two parts:
>   * btf_encoder__collect_kfuncs()
>   * btf_encoder__tag_kfuncs()
> 
> btf_encoder__collect_kfuncs() reads the .BTF_ids section of the ELF,
> collecting kfunc information into a list of kfunc_info structs in the
> btf_encoder. It is executed in btf_encoder__new() when tag_kfuncs flag
> is set. This way kfunc information is available during entire lifetime
> of the btf_encoder.
> 
> btf_encoder__tag_kfuncs() is basically the same: collect BTF
> functions, and then for each kfunc find and tag correspoding BTF
> func. Except now kfunc information is not collected in-place, but is
> simply read from the btf_encoder.
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---

Tbh, I don't think this split is necessary, modifying btf_type
in-place should be fine (and libbpf does it at-least in one place).
E.g. like here:
https://github.com/acmel/dwarves/compare/master...eddyz87:dwarves:arena-attrs-no-split
I like it because it keeps the change a bit more contained,
but I do not insist.

[...]

> @@ -1876,11 +1886,10 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
>  	return 0;
>  }
>  
> -static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
> +static int btf_encoder__collect_kfuncs(struct btf_encoder *encoder)
>  {
>  	const char *filename = encoder->source_filename;
>  	struct gobuffer btf_kfunc_ranges = {};
> -	struct gobuffer btf_funcs = {};
>  	Elf_Data *symbols = NULL;
>  	Elf_Data *idlist = NULL;
>  	Elf_Scn *symscn = NULL;
> @@ -1897,6 +1906,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  	int nr_syms;
>  	int i = 0;
>  
> +	INIT_LIST_HEAD(&encoder->kfuncs);
> +

Nit: do this in the btf_encoder__new?

>  	fd = open(filename, O_RDONLY);
>  	if (fd < 0) {
>  		fprintf(stderr, "Cannot open %s\n", filename);
> @@ -1977,12 +1988,6 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  	}
>  	nr_syms = shdr.sh_size / shdr.sh_entsize;
>  
> -	err = btf_encoder__collect_btf_funcs(encoder, &btf_funcs);
> -	if (err) {
> -		fprintf(stderr, "%s: failed to collect BTF funcs\n", __func__);
> -		goto out;
> -	}
> -
>  	/* First collect all kfunc set ranges.
>  	 *
>  	 * Note we choose not to sort these ranges and accept a linear
> @@ -2015,12 +2020,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  	for (i = 0; i < nr_syms; i++) {
>  		const struct btf_kfunc_set_range *ranges;
>  		const struct btf_id_and_flag *pair;
> +		struct elf_function *elf_fn;
> +		struct kfunc_info *kfunc;
>  		unsigned int ranges_cnt;
>  		char *func, *name;
>  		ptrdiff_t off;
>  		GElf_Sym sym;
>  		bool found;
> -		int err;
>  		int j;
>  
>  		if (!gelf_getsym(symbols, i, &sym)) {
> @@ -2061,18 +2067,26 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  			continue;
>  		}
>  
> -		err = btf_encoder__tag_kfunc(encoder, &btf_funcs, func, pair->flags);
> -		if (err) {
> -			fprintf(stderr, "%s: failed to tag kfunc '%s'\n", __func__, func);
> -			free(func);
> +		elf_fn = btf_encoder__find_function(encoder, func, 0);
> +		free(func);
> +		if (!elf_fn)
> +			continue;
> +		elf_fn->kfunc = true;
> +
> +		kfunc = calloc(1, sizeof(*kfunc));
> +		if (!kfunc) {
> +			fprintf(stderr, "%s: failed to allocate memory for kfunc info\n", __func__);
> +			err = -ENOMEM;
>  			goto out;
>  		}
> -		free(func);
> +		kfunc->id = pair->id;
> +		kfunc->flags = pair->flags;
> +		kfunc->name = elf_fn->name;

If we do go with split, maybe make refactoring a bit more drastic and
merge kfunc_info with elf_function?
This would make maintaining a separate encoder->kfuncs list unnecessary.
Also, can get rid of separate 'struct gobuffer *funcs'.
E.g. see my commit on top of yours:
https://github.com/acmel/dwarves/compare/master...eddyz87:dwarves:arena-attrs-merge-kfunc-info

> +		list_add(&kfunc->node, &encoder->kfuncs);
>  	}
>  
>  	err = 0;
>  out:
> -	__gobuffer__delete(&btf_funcs);
>  	__gobuffer__delete(&btf_kfunc_ranges);
>  	if (elf)
>  		elf_end(elf);
> @@ -2081,6 +2095,34 @@ out:
>  	return err;
>  }
>  

[...]

Re: [PATCH dwarves 2/3] btf_encoder: emit type tags for bpf_arena pointers

[Alan Maguire]

On 07/02/2025 02:14, Ihor Solodrai wrote:
> When adding a kfunc prototype to BTF, check for the flags indicating
> bpf_arena pointers and emit a type tag encoding
> __attribute__((address_space(1))) for them. This also requires
> updating BTF type ids in the btf_encoder_func_state, which is done as
> a side effect in the tagging functions.
> 
> This feature depends on recent update in libbpf, supporting arbitrarty
> attribute encoding [1].
> 
> [1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

a few minor issues below, but

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>

> ---
>  btf_encoder.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index e9f4baf..d7837c2 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -40,7 +40,13 @@
>  #define BTF_SET8_KFUNCS		(1 << 0)
>  #define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
>  #define BTF_FASTCALL_TAG       "bpf_fastcall"
> -#define KF_FASTCALL            (1 << 12)
> +#define BPF_ARENA_ATTR         "address_space(1)"
> +
> +/* kfunc flags, see include/linux/btf.h in the kernel source */
> +#define KF_FASTCALL   (1 << 12)
> +#define KF_ARENA_RET  (1 << 13)
> +#define KF_ARENA_ARG1 (1 << 14)
> +#define KF_ARENA_ARG2 (1 << 15)
>  
>  struct btf_id_and_flag {
>  	uint32_t id;
> @@ -743,6 +749,91 @@ static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t tag_t
>  	return encoder->type_id_off + tag_type;
>  }
>  
> +static inline struct kfunc_info* btf_encoder__kfunc_by_name(struct btf_encoder *encoder, const char *name) {
> +	struct kfunc_info *kfunc;
> +
> +	list_for_each_entry(kfunc, &encoder->kfuncs, node) {
> +		if (strcmp(kfunc->name, name) == 0)
> +			return kfunc;
> +	}
> +	return NULL;
> +}
> +

above function is only used within #if statement below, right? Should
probably move it there to avoid warnings.

> +#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
> +static int btf_encoder__tag_bpf_arena_ptr(struct btf *btf, int ptr_id)
> +{
> +	const struct btf_type *ptr;
> +	int tagged_type_id;
> +
> +	ptr = btf__type_by_id(btf, ptr_id);
> +	if (!btf_is_ptr(ptr))
> +		return -EINVAL;
> +
> +	tagged_type_id = btf__add_type_attr(btf, BPF_ARENA_ATTR, ptr->type);
> +	if (tagged_type_id < 0)
> +		return tagged_type_id;
> +
> +	return btf__add_ptr(btf, tagged_type_id);
> +}
> +
> +static int btf_encoder__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state *state, int idx)
> +{
> +	int id;
> +
> +	if (state->nr_parms <= idx)
> +		return -EINVAL;
> +
> +	id = btf_encoder__tag_bpf_arena_ptr(btf, state->parms[idx].type_id);
> +	if (id < 0) {
> +		btf__log_err(btf, BTF_KIND_TYPE_TAG, BPF_ARENA_ATTR, true, id,
> +			"Error adding BPF_ARENA_ATTR for an argument of kfunc '%s'", state->elf->name);

nit: since we call this for arguments + return value, should we reflect
that in the function name/error message? maybe pass in the KF_ARENA_*
flag or something?

> +		return id;
> +	}
> +	state->parms[idx].type_id = id;
> +
> +	return id;
> +}
> +
> +static int btf_encoder__add_bpf_arena_type_tags(struct btf_encoder *encoder, struct btf_encoder_func_state *state)
> +{
> +	struct kfunc_info *kfunc = NULL;
> +	int ret_type_id;
> +	int err = 0;
> +
> +	if (!state || !state->elf || !state->elf->kfunc)
> +		goto out;
> +
> +	kfunc = btf_encoder__kfunc_by_name(encoder, state->elf->name);
> +	if (!kfunc)
> +		goto out;
> +
> +	if (KF_ARENA_RET & kfunc->flags) {
> +		ret_type_id = btf_encoder__tag_bpf_arena_ptr(encoder->btf, state->ret_type_id);
> +		if (ret_type_id < 0) {
> +			btf__log_err(encoder->btf, BTF_KIND_TYPE_TAG, BPF_ARENA_ATTR, true, ret_type_id,
> +				"Error adding BPF_ARENA_ATTR for return type of kfunc '%s'", state->elf->name);
> +			err = ret_type_id;
> +			goto out;
> +		}
> +		state->ret_type_id = ret_type_id;
> +	}
> +
> +	if (KF_ARENA_ARG1 & kfunc->flags) {
> +		err = btf_encoder__tag_bpf_arena_arg(encoder->btf, state, 0);
> +		if (err < 0)
> +			goto out;
> +	}
> +
> +	if (KF_ARENA_ARG2 & kfunc->flags) {
> +		err = btf_encoder__tag_bpf_arena_arg(encoder->btf, state, 1);
> +		if (err < 0)
> +			goto out;
> +	}
> +out:
> +	return err;

not sure we need goto outs here; there are no resources to free etc so
we can just return err/return 0 where appropriate.

> +}
> +#endif // LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
> +
>  static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype,
>  					   struct btf_encoder_func_state *state)
>  {
> @@ -762,6 +853,10 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f
>  		nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
>  		type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
>  	} else if (state) {
> +#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
> +		if (btf_encoder__add_bpf_arena_type_tags(encoder, state) < 0)

kind of a nit I guess, but I think it might be clearer to make explicit
the work we only have to do for kfuncs, i.e.

		if (state->elf && state->elf->kfunc) {
			/* do kfunc-specific work like arena ptr tag */

		}

I know the function has checks for this internally but I think it makes
it a bit clearer that it's only needed for a small subset of functions,
what do you think?


> +			return -1;
> +#endif
>  		encoder = state->encoder;
>  		btf = state->encoder->btf;
>  		nr_params = state->nr_parms;

Re: [PATCH dwarves 3/3] pahole: introduce --btf_feature=attributes

[Alan Maguire]

On 07/02/2025 02:14, Ihor Solodrai wrote:
> Add a feature flag "attributes" (default: false) controlling whether
> pahole is allowed to generate BTF attributes: type tags and decl tags
> with kind_flag = 1.
> 
> This is necessary for backward compatibility, as BPF verifier does not
> recognize tags with kind_flag = 1 prior to (at least) 6.14-rc1 [1].
> 
> [1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

Needs update to pahole man page describing the new "attributes"
btf_feature, but aside from that LGTM.

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>

> ---
>  btf_encoder.c |  6 ++++--
>  dwarves.h     |  1 +
>  pahole.c      | 11 +++++++++++
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index d7837c2..0a734d4 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -142,7 +142,8 @@ struct btf_encoder {
>  			  gen_floats,
>  			  skip_encoding_decl_tag,
>  			  tag_kfuncs,
> -			  gen_distilled_base;
> +			  gen_distilled_base,
> +			  encode_attributes;
>  	uint32_t	  array_index_id;
>  	struct elf_secinfo *secinfo;
>  	size_t             seccnt;
> @@ -800,7 +801,7 @@ static int btf_encoder__add_bpf_arena_type_tags(struct btf_encoder *encoder, str
>  	int ret_type_id;
>  	int err = 0;
>  
> -	if (!state || !state->elf || !state->elf->kfunc)
> +	if (!encoder->encode_attributes || !state || !state->elf || !state->elf->kfunc)
>  		goto out;
>  
>  	kfunc = btf_encoder__kfunc_by_name(encoder, state->elf->name);
> @@ -2553,6 +2554,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
>  		encoder->skip_encoding_decl_tag	 = conf_load->skip_encoding_btf_decl_tag;
>  		encoder->tag_kfuncs	 = conf_load->btf_decl_tag_kfuncs;
>  		encoder->gen_distilled_base = conf_load->btf_gen_distilled_base;
> +		encoder->encode_attributes = conf_load->btf_attributes;
>  		encoder->verbose	 = verbose;
>  		encoder->has_index_type  = false;
>  		encoder->need_index_type = false;
> diff --git a/dwarves.h b/dwarves.h
> index 8234e1a..99ed783 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -89,6 +89,7 @@ struct conf_load {
>  	bool			reproducible_build;
>  	bool			btf_decl_tag_kfuncs;
>  	bool			btf_gen_distilled_base;
> +	bool			btf_attributes;
>  	uint8_t			hashtable_bits;
>  	uint8_t			max_hashtable_bits;
>  	uint16_t		kabi_prefix_len;
> diff --git a/pahole.c b/pahole.c
> index af3e1cf..0bda249 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -1152,6 +1152,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
>  #define ARG_padding_ge		   347
>  #define ARG_padding		   348
>  #define ARGP_with_embedded_flexible_array 349
> +#define ARGP_btf_attributes	   350
>  
>  /* --btf_features=feature1[,feature2,..] allows us to specify
>   * a list of requested BTF features or "default" to enable all default
> @@ -1210,6 +1211,9 @@ struct btf_feature {
>  	BTF_NON_DEFAULT_FEATURE(distilled_base, btf_gen_distilled_base, false),
>  #endif
>  	BTF_NON_DEFAULT_FEATURE(global_var, encode_btf_global_vars, false),
> +#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
> +	BTF_NON_DEFAULT_FEATURE(attributes, btf_attributes, false),
> +#endif
>  };
>  
>  #define BTF_MAX_FEATURE_STR	1024
> @@ -1785,6 +1789,11 @@ static const struct argp_option pahole__options[] = {
>  		.key = ARGP_running_kernel_vmlinux,
>  		.doc = "Search for, possibly getting from a debuginfo server, a vmlinux matching the running kernel build-id (from /sys/kernel/notes)"
>  	},
> +	{
> +		.name = "btf_attributes",
> +		.key  = ARGP_btf_attributes,
> +		.doc  = "Allow generation of attributes in BTF. Attributes are the type tags and decl tags with the kind_flag set to 1.",
> +	},
>  	{
>  		.name = NULL,
>  	}
> @@ -1979,6 +1988,8 @@ static error_t pahole__options_parser(int key, char *arg,
>  		show_supported_btf_features(stdout);	exit(0);
>  	case ARGP_btf_features_strict:
>  		parse_btf_features(arg, true);		break;
> +	case ARGP_btf_attributes:
> +		conf_load.btf_attributes = true;	break;
>  	default:
>  		return ARGP_ERR_UNKNOWN;
>  	}

Re: [PATCH dwarves 1/3] btf_encoder: collect kfuncs info in btf_encoder__new

[Ihor Solodrai]

On 2/10/25 12:57 PM, Eduard Zingerman wrote:
> On Thu, 2025-02-06 at 18:14 -0800, Ihor Solodrai wrote:
>> From: Ihor Solodrai <ihor.solodrai@pm.me>
>>
>> btf_encoder__tag_kfuncs() is a post-processing step of BTF encoding,
>> executed right before BTF is deduped and dumped to the output.
>>
>> Split btf_encoder__tag_kfuncs() routine in two parts:
>>   * btf_encoder__collect_kfuncs()
>>   * btf_encoder__tag_kfuncs()
>>
>> [...]
>
> Tbh, I don't think this split is necessary, modifying btf_type
> in-place should be fine (and libbpf does it at-least in one place).
> E.g. like here:
> https://github.com/acmel/dwarves/compare/master...eddyz87:dwarves:arena-attrs-no-split
> I like it because it keeps the change a bit more contained,
> but I do not insist.

There are a couple of reasons this split makes sense to me.

First, I wanted to avoid modifying BTF. Having btf_encoder only
appending things to BTF is easy to reason about. But you're saying
modification does happen somewhere already?

The second reason is that the input for kfunc tagging is ELF, and so
it can be read at around the same time other ELF data is read (such as
for fucntions table). This has an additional benefit of running in
parallel to dwarf encoders (because one of the dwarf workers is
creating btf_encoder struct), as opposed to a sequential
post-processing step.

Finally I think it is generally useful to have kfunc info available at
any point of btf encoding, which becomes possible if the BTF_ids
section is read at the beginning of the encoding process.

>
> [...]
>>  
>> -		err = btf_encoder__tag_kfunc(encoder, &btf_funcs, func, pair->flags);
>> -		if (err) {
>> -			fprintf(stderr, "%s: failed to tag kfunc '%s'\n", __func__, func);
>> -			free(func);
>> +		elf_fn = btf_encoder__find_function(encoder, func, 0);
>> +		free(func);
>> +		if (!elf_fn)
>> +			continue;
>> +		elf_fn->kfunc = true;
>> +
>> +		kfunc = calloc(1, sizeof(*kfunc));
>> +		if (!kfunc) {
>> +			fprintf(stderr, "%s: failed to allocate memory for kfunc info\n", __func__);
>> +			err = -ENOMEM;
>>  			goto out;
>>  		}
>> -		free(func);
>> +		kfunc->id = pair->id;
>> +		kfunc->flags = pair->flags;
>> +		kfunc->name = elf_fn->name;
>
> If we do go with split, maybe make refactoring a bit more drastic and
> merge kfunc_info with elf_function?
> This would make maintaining a separate encoder->kfuncs list unnecessary.
> Also, can get rid of separate 'struct gobuffer *funcs'.
> E.g. see my commit on top of yours:
> https://github.com/acmel/dwarves/compare/master...eddyz87:dwarves:arena-attrs-merge-kfunc-info

Yeah, I like it. I'll play with this for v2, thanks.

>
>> +		list_add(&kfunc->node, &encoder->kfuncs);
>>  	}
>>  
>>  	err = 0;
>>  out:
>> -	__gobuffer__delete(&btf_funcs);
>>  	__gobuffer__delete(&btf_kfunc_ranges);
>>  	if (elf)
>>  		elf_end(elf);
>> @@ -2081,6 +2095,34 @@ out:
>>  	return err;
>>  }
>>  
>
> [...]
>

Re: [PATCH dwarves 2/3] btf_encoder: emit type tags for bpf_arena pointers

[Ihor Solodrai]

On 2/10/25 2:11 PM, Alan Maguire wrote:
> On 07/02/2025 02:14, Ihor Solodrai wrote:
>> When adding a kfunc prototype to BTF, check for the flags indicating
>> bpf_arena pointers and emit a type tag encoding
>> __attribute__((address_space(1))) for them. This also requires
>> updating BTF type ids in the btf_encoder_func_state, which is done as
>> a side effect in the tagging functions.
>>
>> This feature depends on recent update in libbpf, supporting arbitrarty
>> attribute encoding [1].
>>
>> [1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
>>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
>
> a few minor issues below, but
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>
>> ---
>>  btf_encoder.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/btf_encoder.c b/btf_encoder.c
>> index e9f4baf..d7837c2 100644
>> --- a/btf_encoder.c
>> +++ b/btf_encoder.c
>> @@ -40,7 +40,13 @@
>>  #define BTF_SET8_KFUNCS		(1 << 0)
>>  #define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
>>  #define BTF_FASTCALL_TAG       "bpf_fastcall"
>> -#define KF_FASTCALL            (1 << 12)
>> +#define BPF_ARENA_ATTR         "address_space(1)"
>> +
>> +/* kfunc flags, see include/linux/btf.h in the kernel source */
>> +#define KF_FASTCALL   (1 << 12)
>> +#define KF_ARENA_RET  (1 << 13)
>> +#define KF_ARENA_ARG1 (1 << 14)
>> +#define KF_ARENA_ARG2 (1 << 15)
>>  
>>  struct btf_id_and_flag {
>>  	uint32_t id;
>> @@ -743,6 +749,91 @@ static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t tag_t
>>  	return encoder->type_id_off + tag_type;
>>  }
>>  
>> +static inline struct kfunc_info* btf_encoder__kfunc_by_name(struct btf_encoder *encoder, const char *name) {
>> +	struct kfunc_info *kfunc;
>> +
>> +	list_for_each_entry(kfunc, &encoder->kfuncs, node) {
>> +		if (strcmp(kfunc->name, name) == 0)
>> +			return kfunc;
>> +	}
>> +	return NULL;
>> +}
>> +
>
> above function is only used within #if statement below, right? Should
> probably move it there to avoid warnings.

Right. I think some of these functions may go away, given Eduard's
suggestions.

>
>> +#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
>> +static int btf_encoder__tag_bpf_arena_ptr(struct btf *btf, int ptr_id)
>> +{
>> +	const struct btf_type *ptr;
>> +	int tagged_type_id;
>> +
>> +	ptr = btf__type_by_id(btf, ptr_id);
>> +	if (!btf_is_ptr(ptr))
>> +		return -EINVAL;
>> +
>> +	tagged_type_id = btf__add_type_attr(btf, BPF_ARENA_ATTR, ptr->type);
>> +	if (tagged_type_id < 0)
>> +		return tagged_type_id;
>> +
>> +	return btf__add_ptr(btf, tagged_type_id);
>> +}
>> +
>> +static int btf_encoder__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state *state, int idx)
>> +{
>> +	int id;
>> +
>> +	if (state->nr_parms <= idx)
>> +		return -EINVAL;
>> +
>> +	id = btf_encoder__tag_bpf_arena_ptr(btf, state->parms[idx].type_id);
>> +	if (id < 0) {
>> +		btf__log_err(btf, BTF_KIND_TYPE_TAG, BPF_ARENA_ATTR, true, id,
>> +			"Error adding BPF_ARENA_ATTR for an argument of kfunc '%s'", state->elf->name);
>
> nit: since we call this for arguments + return value, should we reflect
> that in the function name/error message? maybe pass in the KF_ARENA_*
> flag or something?

It is what's happening. btf_encoder__tag_bpf_arena_ptr is called from
btf_encoder__tag_bpf_arena_arg and separately for a return type:

	if (KF_ARENA_RET & kfunc->flags) {
		ret_type_id = btf_encoder__tag_bpf_arena_ptr(encoder->btf, state->ret_type_id);

In both cases the return value is checked and the error message is
different.

I factored out *_arg version of this operation because it has to be
done twice: for _ARG1 and _ARG2.

Maybe I misunderstood you question? lmk

>
>> +		return id;
>> +	}
>> +	state->parms[idx].type_id = id;
>> +
>> +	return id;
>> +}
>> +
>> +static int btf_encoder__add_bpf_arena_type_tags(struct btf_encoder *encoder, struct btf_encoder_func_state *state)
>> +{
>> +	struct kfunc_info *kfunc = NULL;
>> +	int ret_type_id;
>> +	int err = 0;
>> +
>> +	if (!state || !state->elf || !state->elf->kfunc)
>> +		goto out;
>> +
>> +	kfunc = btf_encoder__kfunc_by_name(encoder, state->elf->name);
>> +	if (!kfunc)
>> +		goto out;
>> +
>> +	if (KF_ARENA_RET & kfunc->flags) {
>> +		ret_type_id = btf_encoder__tag_bpf_arena_ptr(encoder->btf, state->ret_type_id);
>> +		if (ret_type_id < 0) {
>> +			btf__log_err(encoder->btf, BTF_KIND_TYPE_TAG, BPF_ARENA_ATTR, true, ret_type_id,
>> +				"Error adding BPF_ARENA_ATTR for return type of kfunc '%s'", state->elf->name);
>> +			err = ret_type_id;
>> +			goto out;
>> +		}
>> +		state->ret_type_id = ret_type_id;
>> +	}
>> +
>> +	if (KF_ARENA_ARG1 & kfunc->flags) {
>> +		err = btf_encoder__tag_bpf_arena_arg(encoder->btf, state, 0);
>> +		if (err < 0)
>> +			goto out;
>> +	}
>> +
>> +	if (KF_ARENA_ARG2 & kfunc->flags) {
>> +		err = btf_encoder__tag_bpf_arena_arg(encoder->btf, state, 1);
>> +		if (err < 0)
>> +			goto out;
>> +	}
>> +out:
>> +	return err;
>
> not sure we need goto outs here; there are no resources to free etc so
> we can just return err/return 0 where appropriate.

ack

>
>> +}
>> +#endif // LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
>> +
>>  static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype,
>>  					   struct btf_encoder_func_state *state)
>>  {
>> @@ -762,6 +853,10 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f
>>  		nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
>>  		type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
>>  	} else if (state) {
>> +#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 6
>> +		if (btf_encoder__add_bpf_arena_type_tags(encoder, state) < 0)
>
> kind of a nit I guess, but I think it might be clearer to make explicit
> the work we only have to do for kfuncs, i.e.
>
> 		if (state->elf && state->elf->kfunc) {
> 			/* do kfunc-specific work like arena ptr tag */
>
> 		}
>
> I know the function has checks for this internally but I think it makes
> it a bit clearer that it's only needed for a small subset of functions,
> what do you think?

Actually I did write it like that first. IIRC tagging has to be done
before `type_id = state->ret_type_id;` and only when `state` is
passed.

Anyways, I agree it should be clearer that this happens just for some
functions.

>
>
>> +			return -1;
>> +#endif
>>  		encoder = state->encoder;
>>  		btf = state->encoder->btf;
>>  		nr_params = state->nr_parms;

Re: [PATCH dwarves 3/3] pahole: introduce --btf_feature=attributes

[Ihor Solodrai]

On 2/10/25 2:13 PM, Alan Maguire wrote:
> On 07/02/2025 02:14, Ihor Solodrai wrote:
>> Add a feature flag "attributes" (default: false) controlling whether
>> pahole is allowed to generate BTF attributes: type tags and decl tags
>> with kind_flag = 1.
>>
>> This is necessary for backward compatibility, as BPF verifier does not
>> recognize tags with kind_flag = 1 prior to (at least) 6.14-rc1 [1].
>>
>> [1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
>>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> Needs update to pahole man page describing the new "attributes"
> btf_feature, but aside from that LGTM.

I was wondering what have I missed. Thanks.

>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>
> [...]

Re: [PATCH dwarves 1/3] btf_encoder: collect kfuncs info in btf_encoder__new

[Eduard Zingerman]

On Mon, 2025-02-10 at 22:42 +0000, Ihor Solodrai wrote:
> On 2/10/25 12:57 PM, Eduard Zingerman wrote:
> > On Thu, 2025-02-06 at 18:14 -0800, Ihor Solodrai wrote:
> > > From: Ihor Solodrai <ihor.solodrai@pm.me>
> > > 
> > > btf_encoder__tag_kfuncs() is a post-processing step of BTF encoding,
> > > executed right before BTF is deduped and dumped to the output.
> > > 
> > > Split btf_encoder__tag_kfuncs() routine in two parts:
> > >   * btf_encoder__collect_kfuncs()
> > >   * btf_encoder__tag_kfuncs()
> > > 
> > > [...]
> > 
> > Tbh, I don't think this split is necessary, modifying btf_type
> > in-place should be fine (and libbpf does it at-least in one place).
> > E.g. like here:
> > https://github.com/acmel/dwarves/compare/master...eddyz87:dwarves:arena-attrs-no-split
> > I like it because it keeps the change a bit more contained,
> > but I do not insist.
> 
> There are a couple of reasons this split makes sense to me.
> 
> First, I wanted to avoid modifying BTF. Having btf_encoder only
> appending things to BTF is easy to reason about. But you're saying
> modification does happen somewhere already?

See tools/lib/bpf/libbpf.c:bpf_object__sanitize_btf().
A set of small in-place rewrites for compatibility with old kernels.

> The second reason is that the input for kfunc tagging is ELF, and so
> it can be read at around the same time other ELF data is read (such as
> for fucntions table). This has an additional benefit of running in
> parallel to dwarf encoders (because one of the dwarf workers is
> creating btf_encoder struct), as opposed to a sequential
> post-processing step.

Makes sense.

[...]