[PATCH v2 0/3] btf_encoder: do not encode functions consuming packed structs on stack

[PATCH v2 0/3] btf_encoder: do not encode functions consuming packed structs on stack

[Alexis Lothoré (eBPF Foundation)]

Hello,
this series is a follow-up to the RFC sent in [1], which aimed to make
sure that functions consuming struct by value on stack were not encoded
in BTF.

This new series comes with a less "aggressive" strategy, and only skips
function encoding if:
- the function consumes a struct by value
- the struct is passed on the stack
- it is detected as packed (see class__infer_packed_attributes)

The detection is not done anymore in parameter__new, as it may be done
too early to have all the relevant info to properly infer the
attributes; it is now done in btf_encoder__encode_cu. The series also
comes with a simple test, but as the kernel don't have such case
exposed, this new test comes with a small out-of-tree module. The logic
is kept simple and the test is by default skipped (it needs to be
provided with a path to some kernel sources), as I am not sure how it
should integrate with the current CI effort done by Alan.

[1] https://lore.kernel.org/dwarves/20250618-btf_skip_structs_on_stack-v1-1-e70be639cc53@bootlin.com/

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Changes in v2:
- infer structs attributes
- skip function encoded if some consumed struct (passed on stack) is
  marked as packed
- add some tests in btf_functions.sh
- drop RFC prefix
- Link to v1: https://lore.kernel.org/r/20250618-btf_skip_structs_on_stack-v1-1-e70be639cc53@bootlin.com

---
Alexis Lothoré (eBPF Foundation) (3):
      btf_encoder: skip functions consuming packed structs passed by value on stack
      tests: add some tests validating skipped functions due to uncertain arg location
      gitignore: ignore all the test kmod build-related files

 .gitignore             |  3 ++
 btf_encoder.c          | 50 ++++++++++++++++++++++++-
 dwarves.h              |  1 +
 tests/btf_functions.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/kmod/Makefile    |  1 +
 tests/kmod/kmod.c      | 69 +++++++++++++++++++++++++++++++++++
 6 files changed, 221 insertions(+), 2 deletions(-)
---
base-commit: 042d73962d35fdd1466e056f1ea14590b1cdbb9b
change-id: 20250617-btf_skip_structs_on_stack-006adf457d50

Best regards,
-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack

[Alexis Lothoré (eBPF Foundation)]

Most ABIs allow functions to receive structs passed by value, if they
fit in a register or a pair of registers, depending on the exact ABI.
However, when there is a struct passed by value but all registers are
already used for parameters passing, the struct is still passed by value
but on the stack. This becomes an issue if the passed struct is defined
with some attributes like __attribute__((packed)) or
__attribute__((aligned(X)), as its location on the stack is altered, but
this change is not reflected in dwarf information. The corresponding BTF
data generated from this can lead to incorrect BPF trampolines
generation (eg to attach bpf tracing programs to kernel functions) in
the Linux kernel.

Prevent those wrong cases by not encoding functions consuming structs
passed by value on stack, when those structs do not have the expected
alignment due to some attribute usage.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Changes in v2:
- do not deny any struct passed by value, only those passed on stack AND
  with some attribute alteration
- use the existing class__infer_packed_attributes to deduce is a struct
  is "altered". As a consequence, move the function filtering from
  parameter__new to btf_encoder__encode_cu, to make sure that all the
  needed data has been parsed from debug info
---
 btf_encoder.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 dwarves.h     |  1 +
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 0bc23349b5d740c3ddab8208b2e15cdbdd139b9d..16739066caae808aea77175e6c221afbe37b7c70 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -87,6 +87,7 @@ struct btf_encoder_func_state {
 	uint8_t optimized_parms:1;
 	uint8_t unexpected_reg:1;
 	uint8_t inconsistent_proto:1;
+	uint8_t uncertain_parm_loc:1;
 	int ret_type_id;
 	struct btf_encoder_func_parm *parms;
 	struct btf_encoder_func_annot *annots;
@@ -1203,6 +1204,7 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
 	state->inconsistent_proto = ftype->inconsistent_proto;
 	state->unexpected_reg = ftype->unexpected_reg;
 	state->optimized_parms = ftype->optimized_parms;
+	state->uncertain_parm_loc = ftype->uncertain_parm_loc;
 	ftype__for_each_parameter(ftype, param) {
 		const char *name = parameter__name(param) ?: "";
 
@@ -1430,9 +1432,15 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
 		/* do not exclude functions with optimized-out parameters; they
 		 * may still be _called_ with the right parameter values, they
 		 * just do not _use_ them.  Only exclude functions with
-		 * unexpected register use or multiple inconsistent prototypes.
+		 * unexpected register use, multiple inconsistent prototypes or
+		 * uncertain parameters location
 		 */
-		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto;
+		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc;
+
+		if (state->uncertain_parm_loc)
+			btf_encoder__log_func_skip(encoder, saved_fns[i].elf,
+					"uncertain parameter location\n",
+					0, 0);
 
 		if (add_to_btf) {
 			err = btf_encoder__add_func(state->encoder, state);
@@ -2553,6 +2561,39 @@ void btf_encoder__delete(struct btf_encoder *encoder)
 	free(encoder);
 }
 
+static bool ftype__has_uncertain_arg_loc(struct cu *cu, struct ftype *ftype)
+{
+	struct parameter *param;
+	int param_idx = 0;
+
+	if (ftype->nr_parms < cu->nr_register_params)
+		return false;
+
+	ftype__for_each_parameter(ftype, param) {
+		if (param_idx++ < cu->nr_register_params)
+			continue;
+
+		struct tag *type = cu__type(cu, param->tag.type);
+
+		if (type == NULL || !tag__is_struct(type))
+			continue;
+
+		struct type *ctype = tag__type(type);
+		if (ctype->namespace.name == 0)
+			continue;
+
+		struct class *class = tag__class(type);
+
+		class__find_holes(class);
+		class__infer_packed_attributes(class, cu);
+
+		if (class->is_packed)
+			return true;
+	}
+
+	return false;
+}
+
 int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct conf_load *conf_load)
 {
 	struct llvm_annotation *annot;
@@ -2647,6 +2688,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
 		 * Skip functions that:
 		 *   - are marked as declarations
 		 *   - do not have full argument names
+		 *   - have arguments with uncertain locations, e.g packed
+		 *   structs passed by value on stack
 		 *   - are not in ftrace list (if it's available)
 		 *   - are not external (in case ftrace filter is not available)
 		 */
@@ -2693,6 +2736,9 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
 		if (!func)
 			continue;
 
+		if (ftype__has_uncertain_arg_loc(cu, &fn->proto))
+			fn->proto.uncertain_parm_loc = 1;
+
 		err = btf_encoder__save_func(encoder, fn, func);
 		if (err)
 			goto out;
diff --git a/dwarves.h b/dwarves.h
index 36c689847ebf29a1ab9936f9d0f928dd46514547..d689aee5910f4b40dc13b3e9dc596dfbe6a2c3d0 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -1021,6 +1021,7 @@ struct ftype {
 	uint8_t		 unexpected_reg:1;
 	uint8_t		 processed:1;
 	uint8_t		 inconsistent_proto:1;
+	uint8_t		 uncertain_parm_loc:1;
 	struct list_head template_type_params;
 	struct list_head template_value_params;
 	struct template_parameter_pack *template_parameter_pack;

-- 
2.50.0

[PATCH v2 2/3] tests: add some tests validating skipped functions due to uncertain arg location

[Alexis Lothoré (eBPF Foundation)]

Add a small kernel module representing specific cases likely absent from
standard vmlinux files. As a starter, the introduced module exposes a
few functions consuming structs passed by value, some passed by
register, some passed on the stack:

  int kmod_test_init(void);
  int test_kmod_func_ok(int, void *, char, short int);
  int test_kmod_func_struct_ok(int, void *, char, struct kmod_struct);
  int test_kmod_func_struct_on_stack_ok(int, void *, char, short int, int, \
    void *, char, short int, struct kmod_struct);
  int test_kmod_func_struct_on_stack_ko(int, void *, char, short int, int, \
    void *, char, short int, struct kmod_struct_packed);

Then enrich btf_functions.sh to make it perform the following steps:
- build the module
- generate BTF info and pfunct listing, both with dwarf and the
  generated BTF
- check that any function encoded in BTF is found in DWARF
- check that any function announced as skipped is indeed absent from BTF
- check that any skipped function has been skipped due to uncertain
  parameter location

Those new tests are executed only if a kernel directory is provided as
script's second argument, they are otherwise skipped.

Example of the new test execution:
  Encoding...Matched 4 functions exactly.
  Ok
  Validation of skipped function logic...
  Skipped encoding 1 functions in BTF.
  Ok
  Validating skipped functions have uncertain parameter location...
  Found 1 legitimately skipped function due to uncertain loc
  Ok

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Changes in v2:
- new patch
---
 tests/btf_functions.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/kmod/Makefile    |  1 +
 tests/kmod/kmod.c      | 69 +++++++++++++++++++++++++++++++++++
 3 files changed, 169 insertions(+)

diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
index c92e5ae906f90badfede86eb530108894fbc8c93..64810b7eb51e7f2693929fbf66e0641a9d4e0277 100755
--- a/tests/btf_functions.sh
+++ b/tests/btf_functions.sh
@@ -193,4 +193,103 @@ if [[ -n "$VERBOSE" ]]; then
 fi
 echo "Ok"
 
+# Some specific cases can not  be tested directly with a standard kernel.
+# We can use the kernel module in kmod/ to test those cases, like packed
+# structs passed on the stack. Run this test only if we have the needed
+# dependencies (eg: some kernel sources directory passed as argument; those
+# must match the used vmlinux file)
+
+KDIR=${KDIR:-$2}
+if [ -z "$KDIR" ] ; then
+	echo "Skipping kmod tests"
+	exit 0
+fi
+
+echo -n "Validation of BTF encoding corner cases with kmod functions; this may take some time: "
+
+test -n "$VERBOSE" && printf "\nBuilding kmod..."
+tests_dir=$(dirname $0)
+make -C ${KDIR} M=${tests_dir}/kmod
+
+test -n "$VERBOSE" && printf "\nEncoding..."
+pahole --btf_features=default --lang_exclude=rust --btf_encode_detached=$outdir/kmod.btf \
+	--verbose ${tests_dir}/kmod/kmod.ko | grep "skipping BTF encoding of function" \
+	> ${outdir}/kmod_skipped_fns
+
+funcs=$(pfunct --format_path=btf $outdir/kmod.btf 2>/dev/null|sort)
+pfunct --all --no_parm_names --format_path=dwarf kmod/kmod.ko | \
+	sort|uniq > $outdir/kmod_dwarf.funcs
+pfunct --all --no_parm_names --format_path=btf $outdir/kmod.btf 2>/dev/null|\
+	awk '{ gsub("^(bpf_kfunc |bpf_fastcall )+",""); print $0}'|sort|uniq > $outdir/kmod_btf.funcs
+
+exact=0
+while IFS= read -r btf ; do
+	# Matching process can be kept simpler as the tested binary is
+	# specifically tailored for tests
+	dwarf=$(grep -F "$btf" $outdir/kmod_dwarf.funcs)
+	if [[ "$btf" != "$dwarf" ]]; then
+		echo "ERROR: mismatch : BTF '$btf' not found; DWARF '$dwarf'"
+		fail
+	else
+		exact=$((exact+1))
+	fi
+done < $outdir/kmod_btf.funcs
+
+if [[ -n "$VERBOSE" ]]; then
+	echo "Matched $exact functions exactly."
+	echo "Ok"
+	echo "Validation of skipped function logic..."
+fi
+
+skipped_cnt=$(wc -l ${outdir}/kmod_skipped_fns | awk '{ print $1}')
+if [[ "$skipped_cnt" == "0" ]]; then
+	echo "No skipped functions.  Done."
+	exit 0
+fi
+
+skipped_fns=$(awk '{print $1}' $outdir/kmod_skipped_fns)
+for s in $skipped_fns ; do
+	# Ensure the skipped function are not in BTF
+	inbtf=$(grep " $s(" $outdir/kmod_btf.funcs)
+	if [[ -n "$inbtf" ]]; then
+		echo "ERROR: '${s}()' was added incorrectly to BTF: '$inbtf'"
+		fail
+	fi
+done
+
+if [[ -n "$VERBOSE" ]]; then
+	echo "Skipped encoding $skipped_cnt functions in BTF."
+	echo "Ok"
+	echo "Validating skipped functions have uncertain parameter location..."
+fi
+
+uncertain_loc=$(awk '/due to uncertain parameter location/ { print $1 }' $outdir/kmod_skipped_fns)
+legitimate_skip=0
+
+for f in $uncertain_loc ; do
+	# Extract parameters types
+	raw_params=$(grep ${f} $outdir/kmod_dwarf.funcs|sed -n 's/^[^(]*(\([^)]*\)).*/\1/p')
+	IFS=',' read -ra params <<< "${raw_params}"
+	for param in "${params[@]}"
+	do
+		# Search any param that could be a struct
+		struct_type=$(echo ${param}|grep -E '^struct [^*]' | sed -E 's/^struct //')
+		if [ -n "${struct_type}" ]; then
+			# Check with pahole if the struct is detected as
+			# packed
+			if pahole -C "${struct_type}" kmod/kmod.ko|grep -q __packed__
+			then
+				legitimate_skip=$((legitimate_skip+1))
+				continue 2
+			fi
+		fi
+	done
+	echo "ERROR: '${f}()' should not have been skipped; it has no parameter with uncertain location"
+	fail
+done
+
+if [[ -n "$VERBOSE" ]]; then
+	echo "Found ${legitimate_skip} legitimately skipped function due to uncertain loc"
+fi
+echo "Ok"
 exit 0
diff --git a/tests/kmod/Makefile b/tests/kmod/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..e7c2ed929eaf81e91429f744c3778156ed2be2d2
--- /dev/null
+++ b/tests/kmod/Makefile
@@ -0,0 +1 @@
+obj-m += kmod.o
diff --git a/tests/kmod/kmod.c b/tests/kmod/kmod.c
new file mode 100644
index 0000000000000000000000000000000000000000..5b93614b6156b05925a6cb48809ad63533ccba3e
--- /dev/null
+++ b/tests/kmod/kmod.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/module.h>
+#include <linux/printk.h>
+
+struct kmod_struct {
+	char a;
+	short b;
+	int c;
+	unsigned long long d;
+};
+
+struct kmod_struct_packed {
+	char a;
+	short b;
+	int c;
+	unsigned long long d;
+}__packed;
+
+int test_kmod_func_ok(int a, void *b, char c, short d);
+int test_kmod_func_struct_ok(int a, void *b, char c, struct kmod_struct d);
+int test_kmod_func_struct_on_stack_ok(int a, void *b, char c, short d, int e,
+                                      void *f, char g, short h,
+                                      struct kmod_struct i);
+int test_kmod_func_struct_on_stack_ko(int a, void *b, char c, short d, int e,
+                                      void *f, char g, short h,
+                                      struct kmod_struct_packed i);
+
+noinline int test_kmod_func_ok(int a, void *b, char c, short d)
+{
+	return a + (long)b + c + d;
+}
+
+noinline int test_kmod_func_struct_ok(int a, void *b, char c,
+                                      struct kmod_struct d)
+{
+	return a + (long)b + c + d.a + d.b + d.c + d.d;
+}
+
+noinline int test_kmod_func_struct_on_stack_ok(int a, void *b, char c, short d,
+                                               int e, void *f, char g, short h,
+                                               struct kmod_struct i)
+{
+	return a + (long)b + c + d + e + (long)f + g + h + i.a + i.b + i.c + i.d;
+}
+
+noinline int test_kmod_func_struct_on_stack_ko(int a, void *b, char c, short d,
+                                               int e, void *f, char g, short h,
+                                               struct kmod_struct_packed i)
+{
+	return a + (long)b + c + d + e + (long)f + g + h + i.a + i.b + i.c + i.d;
+}
+
+static int kmod_test_init(void)
+{
+	struct kmod_struct test;
+	struct kmod_struct_packed test_bis;
+
+	test_kmod_func_ok(0, NULL, 0, 0);
+	test_kmod_func_struct_ok(0, NULL, 0, test);
+	test_kmod_func_struct_on_stack_ok(0, NULL, 0, 0, 0, NULL, 0, 0, test);
+	test_kmod_func_struct_on_stack_ko(0, NULL, 0, 0, 0, NULL, 0, 0, test_bis);
+	return 0;
+}
+
+module_init(kmod_test_init);
+
+MODULE_AUTHOR("Alexis Lothoré");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Pahole testing module");

-- 
2.50.0

[PATCH v2 3/3] gitignore: ignore all the test kmod build-related files

[Alexis Lothoré (eBPF Foundation)]

The kmod module generates quite a lot of intermediate build files, so
ignore those in git.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Changes in v2:
- new patch
---
 .gitignore | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.gitignore b/.gitignore
index 96e05c7842624067ed5571bccbaae76122a66567..b82205d4ee2d0a83ac736c3c879d46d44e0212d6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,5 @@
 /build
 /config.h
+tests/kmod/*
+!tests/kmod/kmod.c
+!tests/kmod/Makefile

-- 
2.50.0

Re: [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack

[Ihor Solodrai]

On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:
> Most ABIs allow functions to receive structs passed by value, if they
> fit in a register or a pair of registers, depending on the exact ABI.
> However, when there is a struct passed by value but all registers are
> already used for parameters passing, the struct is still passed by value
> but on the stack. This becomes an issue if the passed struct is defined
> with some attributes like __attribute__((packed)) or
> __attribute__((aligned(X)), as its location on the stack is altered, but
> this change is not reflected in dwarf information. The corresponding BTF
> data generated from this can lead to incorrect BPF trampolines
> generation (eg to attach bpf tracing programs to kernel functions) in
> the Linux kernel.
> 
> Prevent those wrong cases by not encoding functions consuming structs
> passed by value on stack, when those structs do not have the expected
> alignment due to some attribute usage.
> 
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
> Changes in v2:
> - do not deny any struct passed by value, only those passed on stack AND
>    with some attribute alteration
> - use the existing class__infer_packed_attributes to deduce is a struct
>    is "altered". As a consequence, move the function filtering from
>    parameter__new to btf_encoder__encode_cu, to make sure that all the
>    needed data has been parsed from debug info
> ---
>   btf_encoder.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
>   dwarves.h     |  1 +
>   2 files changed, 49 insertions(+), 2 deletions(-)
> 

Hi Alexis. A couple of comments below.


> diff --git a/btf_encoder.c b/btf_encoder.c
> index 0bc23349b5d740c3ddab8208b2e15cdbdd139b9d..16739066caae808aea77175e6c221afbe37b7c70 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -87,6 +87,7 @@ struct btf_encoder_func_state {
>   	uint8_t optimized_parms:1;
>   	uint8_t unexpected_reg:1;
>   	uint8_t inconsistent_proto:1;
> +	uint8_t uncertain_parm_loc:1;
>   	int ret_type_id;
>   	struct btf_encoder_func_parm *parms;
>   	struct btf_encoder_func_annot *annots;
> @@ -1203,6 +1204,7 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
>   	state->inconsistent_proto = ftype->inconsistent_proto;
>   	state->unexpected_reg = ftype->unexpected_reg;
>   	state->optimized_parms = ftype->optimized_parms;
> +	state->uncertain_parm_loc = ftype->uncertain_parm_loc;
>   	ftype__for_each_parameter(ftype, param) {
>   		const char *name = parameter__name(param) ?: "";
>   
> @@ -1430,9 +1432,15 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
>   		/* do not exclude functions with optimized-out parameters; they
>   		 * may still be _called_ with the right parameter values, they
>   		 * just do not _use_ them.  Only exclude functions with
> -		 * unexpected register use or multiple inconsistent prototypes.
> +		 * unexpected register use, multiple inconsistent prototypes or
> +		 * uncertain parameters location
>   		 */
> -		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto;
> +		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc;


Is it possible for a function to have uncertain_parm_loc in one CU,
but not in another?

If yes, we still don't want the function in BTF, right?

> +
> +		if (state->uncertain_parm_loc)
> +			btf_encoder__log_func_skip(encoder, saved_fns[i].elf,
> +					"uncertain parameter location\n",
> +					0, 0);
>   
>   		if (add_to_btf) {
>   			err = btf_encoder__add_func(state->encoder, state);
> @@ -2553,6 +2561,39 @@ void btf_encoder__delete(struct btf_encoder *encoder)
>   	free(encoder);
>   }
>   
> +static bool ftype__has_uncertain_arg_loc(struct cu *cu, struct ftype *ftype)
> +{
> +	struct parameter *param;
> +	int param_idx = 0;
> +
> +	if (ftype->nr_parms < cu->nr_register_params)
> +		return false;
> +
> +	ftype__for_each_parameter(ftype, param) {
> +		if (param_idx++ < cu->nr_register_params)
> +			continue;
> +
> +		struct tag *type = cu__type(cu, param->tag.type);
> +
> +		if (type == NULL || !tag__is_struct(type))
> +			continue;
> +
> +		struct type *ctype = tag__type(type);
> +		if (ctype->namespace.name == 0)
> +			continue;
> +
> +		struct class *class = tag__class(type);
> +
> +		class__find_holes(class);
> +		class__infer_packed_attributes(class, cu);
> +
> +		if (class->is_packed)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>   int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct conf_load *conf_load)
>   {
>   	struct llvm_annotation *annot;
> @@ -2647,6 +2688,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>   		 * Skip functions that:
>   		 *   - are marked as declarations
>   		 *   - do not have full argument names
> +		 *   - have arguments with uncertain locations, e.g packed
> +		 *   structs passed by value on stack
>   		 *   - are not in ftrace list (if it's available)
>   		 *   - are not external (in case ftrace filter is not available)
>   		 */
> @@ -2693,6 +2736,9 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>   		if (!func)
>   			continue;
>   
> +		if (ftype__has_uncertain_arg_loc(cu, &fn->proto))
> +			fn->proto.uncertain_parm_loc = 1;
> +
>   		err = btf_encoder__save_func(encoder, fn, func);

I think checking and setting uncertain_parm_loc flag should be done
inside btf_encoder__save_func(), because that's where we inspect DWARF
function prototype and add a new btf_encoder_func_state.

>   		if (err)
>   			goto out;
> diff --git a/dwarves.h b/dwarves.h
> index 36c689847ebf29a1ab9936f9d0f928dd46514547..d689aee5910f4b40dc13b3e9dc596dfbe6a2c3d0 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -1021,6 +1021,7 @@ struct ftype {
>   	uint8_t		 unexpected_reg:1;
>   	uint8_t		 processed:1;
>   	uint8_t		 inconsistent_proto:1;
> +	uint8_t		 uncertain_parm_loc:1;
>   	struct list_head template_type_params;
>   	struct list_head template_value_params;
>   	struct template_parameter_pack *template_parameter_pack;
> 

Re: [PATCH v2 2/3] tests: add some tests validating skipped functions due to uncertain arg location

[Ihor Solodrai]

On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:
> Add a small kernel module representing specific cases likely absent from
> standard vmlinux files. As a starter, the introduced module exposes a
> few functions consuming structs passed by value, some passed by
> register, some passed on the stack:
> 
>    int kmod_test_init(void);
>    int test_kmod_func_ok(int, void *, char, short int);
>    int test_kmod_func_struct_ok(int, void *, char, struct kmod_struct);
>    int test_kmod_func_struct_on_stack_ok(int, void *, char, short int, int, \
>      void *, char, short int, struct kmod_struct);
>    int test_kmod_func_struct_on_stack_ko(int, void *, char, short int, int, \
>      void *, char, short int, struct kmod_struct_packed);
> 
> Then enrich btf_functions.sh to make it perform the following steps:
> - build the module
> - generate BTF info and pfunct listing, both with dwarf and the
>    generated BTF
> - check that any function encoded in BTF is found in DWARF
> - check that any function announced as skipped is indeed absent from BTF
> - check that any skipped function has been skipped due to uncertain
>    parameter location
> 
> Those new tests are executed only if a kernel directory is provided as
> script's second argument, they are otherwise skipped.

While this shouldn't be a problem for CI, since it checks out a kernel
tree to test vmlinux as input, I wonder if there is a way to do the
same test without this dependency.

We need to generate a binary with DWARF, containing function
prototypes with packed/aligned attributes. Give it to pahole and see
that those functions were skipped.

Any reason it must be a kernel module? Am I missing something?


> 
> Example of the new test execution:
>    Encoding...Matched 4 functions exactly.
>    Ok
>    Validation of skipped function logic...
>    Skipped encoding 1 functions in BTF.
>    Ok
>    Validating skipped functions have uncertain parameter location...
>    Found 1 legitimately skipped function due to uncertain loc
>    Ok
> 
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
> Changes in v2:
> - new patch
> ---
>   tests/btf_functions.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   tests/kmod/Makefile    |  1 +
>   tests/kmod/kmod.c      | 69 +++++++++++++++++++++++++++++++++++
>   3 files changed, 169 insertions(+)
> 
> diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
> index c92e5ae906f90badfede86eb530108894fbc8c93..64810b7eb51e7f2693929fbf66e0641a9d4e0277 100755
> --- a/tests/btf_functions.sh
> +++ b/tests/btf_functions.sh
> @@ -193,4 +193,103 @@ if [[ -n "$VERBOSE" ]]; then
>   fi
>   echo "Ok"
>   
> +# Some specific cases can not  be tested directly with a standard kernel.
> +# We can use the kernel module in kmod/ to test those cases, like packed
> +# structs passed on the stack. Run this test only if we have the needed
> +# dependencies (eg: some kernel sources directory passed as argument; those
> +# must match the used vmlinux file)
> +
> +KDIR=${KDIR:-$2}
> +if [ -z "$KDIR" ] ; then
> +	echo "Skipping kmod tests"
> +	exit 0
> +fi
> +
> +echo -n "Validation of BTF encoding corner cases with kmod functions; this may take some time: "
> +
> +test -n "$VERBOSE" && printf "\nBuilding kmod..."
> +tests_dir=$(dirname $0)
> +make -C ${KDIR} M=${tests_dir}/kmod

This part fails for me:

isolodrai@isolodrai-fedora-PC2K40WQ:~/pahole/tests$ 
KDIR=/home/isolodrai/kernels/bpf-next 
vmlinux=/home/isolodrai/kernels/bpf-next/vmlinux ./btf_functions.sh
Validation of BTF encoding of functions; this may take some time: Ok
Validation of BTF encoding corner cases with kmod functions; this may 
take some time: make: Entering directory '/home/isolodrai/kernels/bpf-next'
Makefile:199: *** specified external module directory "./kmod" does not 
exist.  Stop.
make: Leaving directory '/home/isolodrai/kernels/bpf-next'
No skipped functions.  Done.

Maybe:

diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
index 64810b7..fcb1591 100755
--- a/tests/btf_functions.sh
+++ b/tests/btf_functions.sh
@@ -208,7 +208,7 @@ fi
  echo -n "Validation of BTF encoding corner cases with kmod functions; 
this may take some time: "

  test -n "$VERBOSE" && printf "\nBuilding kmod..."
-tests_dir=$(dirname $0)
+tests_dir=$(realpath $(dirname $0))
  make -C ${KDIR} M=${tests_dir}/kmod

  test -n "$VERBOSE" && printf "\nEncoding..."


Also, in case kernel is built with LLVM, one must set LLVM=1.
Not sure if this is detectable by the test.

> +
> +test -n "$VERBOSE" && printf "\nEncoding..."
> +pahole --btf_features=default --lang_exclude=rust --btf_encode_detached=$outdir/kmod.btf \
> +	--verbose ${tests_dir}/kmod/kmod.ko | grep "skipping BTF encoding of function" \
> +	> ${outdir}/kmod_skipped_fns
> +
> +funcs=$(pfunct --format_path=btf $outdir/kmod.btf 2>/dev/null|sort)
> +pfunct --all --no_parm_names --format_path=dwarf kmod/kmod.ko | \
> +	sort|uniq > $outdir/kmod_dwarf.funcs
> +pfunct --all --no_parm_names --format_path=btf $outdir/kmod.btf 2>/dev/null|\
> +	awk '{ gsub("^(bpf_kfunc |bpf_fastcall )+",""); print $0}'|sort|uniq > $outdir/kmod_btf.funcs
> +
> +exact=0
> +while IFS= read -r btf ; do
> +	# Matching process can be kept simpler as the tested binary is
> +	# specifically tailored for tests
> +	dwarf=$(grep -F "$btf" $outdir/kmod_dwarf.funcs)
> +	if [[ "$btf" != "$dwarf" ]]; then
> +		echo "ERROR: mismatch : BTF '$btf' not found; DWARF '$dwarf'"
> +		fail
> +	else
> +		exact=$((exact+1))
> +	fi
> +done < $outdir/kmod_btf.funcs
> +
> +if [[ -n "$VERBOSE" ]]; then
> +	echo "Matched $exact functions exactly."
> +	echo "Ok"
> +	echo "Validation of skipped function logic..."
> +fi
> +
> +skipped_cnt=$(wc -l ${outdir}/kmod_skipped_fns | awk '{ print $1}')
> +if [[ "$skipped_cnt" == "0" ]]; then
> +	echo "No skipped functions.  Done."
> +	exit 0
> +fi
> +
> +skipped_fns=$(awk '{print $1}' $outdir/kmod_skipped_fns)
> +for s in $skipped_fns ; do
> +	# Ensure the skipped function are not in BTF
> +	inbtf=$(grep " $s(" $outdir/kmod_btf.funcs)
> +	if [[ -n "$inbtf" ]]; then
> +		echo "ERROR: '${s}()' was added incorrectly to BTF: '$inbtf'"
> +		fail
> +	fi
> +done
> +
> +if [[ -n "$VERBOSE" ]]; then
> +	echo "Skipped encoding $skipped_cnt functions in BTF."
> +	echo "Ok"
> +	echo "Validating skipped functions have uncertain parameter location..."
> +fi
> +
> +uncertain_loc=$(awk '/due to uncertain parameter location/ { print $1 }' $outdir/kmod_skipped_fns)
> +legitimate_skip=0
> +
> +for f in $uncertain_loc ; do
> +	# Extract parameters types
> +	raw_params=$(grep ${f} $outdir/kmod_dwarf.funcs|sed -n 's/^[^(]*(\([^)]*\)).*/\1/p')
> +	IFS=',' read -ra params <<< "${raw_params}"
> +	for param in "${params[@]}"
> +	do
> +		# Search any param that could be a struct
> +		struct_type=$(echo ${param}|grep -E '^struct [^*]' | sed -E 's/^struct //')
> +		if [ -n "${struct_type}" ]; then
> +			# Check with pahole if the struct is detected as
> +			# packed
> +			if pahole -C "${struct_type}" kmod/kmod.ko|grep -q __packed__
> +			then
> +				legitimate_skip=$((legitimate_skip+1))
> +				continue 2
> +			fi
> +		fi
> +	done
> +	echo "ERROR: '${f}()' should not have been skipped; it has no parameter with uncertain location"
> +	fail
> +done
> +
> +if [[ -n "$VERBOSE" ]]; then
> +	echo "Found ${legitimate_skip} legitimately skipped function due to uncertain loc"
> +fi
> +echo "Ok"
>   exit 0
> diff --git a/tests/kmod/Makefile b/tests/kmod/Makefile
> new file mode 100644
> index 0000000000000000000000000000000000000000..e7c2ed929eaf81e91429f744c3778156ed2be2d2
> --- /dev/null
> +++ b/tests/kmod/Makefile
> @@ -0,0 +1 @@
> +obj-m += kmod.o
> diff --git a/tests/kmod/kmod.c b/tests/kmod/kmod.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5b93614b6156b05925a6cb48809ad63533ccba3e
> --- /dev/null
> +++ b/tests/kmod/kmod.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/module.h>
> +#include <linux/printk.h>
> +
> +struct kmod_struct {
> +	char a;
> +	short b;
> +	int c;
> +	unsigned long long d;
> +};
> +
> +struct kmod_struct_packed {
> +	char a;
> +	short b;
> +	int c;
> +	unsigned long long d;
> +}__packed;
> +
> +int test_kmod_func_ok(int a, void *b, char c, short d);
> +int test_kmod_func_struct_ok(int a, void *b, char c, struct kmod_struct d);
> +int test_kmod_func_struct_on_stack_ok(int a, void *b, char c, short d, int e,
> +                                      void *f, char g, short h,
> +                                      struct kmod_struct i);
> +int test_kmod_func_struct_on_stack_ko(int a, void *b, char c, short d, int e,
> +                                      void *f, char g, short h,
> +                                      struct kmod_struct_packed i);
> +
> +noinline int test_kmod_func_ok(int a, void *b, char c, short d)
> +{
> +	return a + (long)b + c + d;
> +}
> +
> +noinline int test_kmod_func_struct_ok(int a, void *b, char c,
> +                                      struct kmod_struct d)
> +{
> +	return a + (long)b + c + d.a + d.b + d.c + d.d;
> +}
> +
> +noinline int test_kmod_func_struct_on_stack_ok(int a, void *b, char c, short d,
> +                                               int e, void *f, char g, short h,
> +                                               struct kmod_struct i)
> +{
> +	return a + (long)b + c + d + e + (long)f + g + h + i.a + i.b + i.c + i.d;
> +}
> +
> +noinline int test_kmod_func_struct_on_stack_ko(int a, void *b, char c, short d,
> +                                               int e, void *f, char g, short h,
> +                                               struct kmod_struct_packed i)
> +{
> +	return a + (long)b + c + d + e + (long)f + g + h + i.a + i.b + i.c + i.d;
> +}
> +
> +static int kmod_test_init(void)
> +{
> +	struct kmod_struct test;
> +	struct kmod_struct_packed test_bis;
> +
> +	test_kmod_func_ok(0, NULL, 0, 0);
> +	test_kmod_func_struct_ok(0, NULL, 0, test);
> +	test_kmod_func_struct_on_stack_ok(0, NULL, 0, 0, 0, NULL, 0, 0, test);
> +	test_kmod_func_struct_on_stack_ko(0, NULL, 0, 0, 0, NULL, 0, 0, test_bis);
> +	return 0;
> +}
> +
> +module_init(kmod_test_init);
> +
> +MODULE_AUTHOR("Alexis Lothoré");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Pahole testing module");
> 

Re: [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack

[Alexis Lothoré]

Hello Ihor,

thanks for the prompt feedback and testing !

On Thu Jul 3, 2025 at 8:17 PM CEST, Ihor Solodrai wrote:
> On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:

[...]

>>   		/* do not exclude functions with optimized-out parameters; they
>>   		 * may still be _called_ with the right parameter values, they
>>   		 * just do not _use_ them.  Only exclude functions with
>> -		 * unexpected register use or multiple inconsistent prototypes.
>> +		 * unexpected register use, multiple inconsistent prototypes or
>> +		 * uncertain parameters location
>>   		 */
>> -		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto;
>> +		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc;
>
>
> Is it possible for a function to have uncertain_parm_loc in one CU,
> but not in another?
>
> If yes, we still don't want the function in BTF, right?

TBH, my understanding about those discrepancies between CUs about the same
functions and how pahole handle them is still a bit fragile. Have you got
any example about how it could be the case ?

If it _can_ happen, I guess you are suggesting to make sure that copies are
compared in saved_functions_combine and their uncertain_loc_parm flag are
aligned. Something like this:

uncertain_parm_loc = a->uncertain_parm_loc | b->uncertain_parm_loc;
[...]
a->uncertain_parm_loc = b->uncertain_parm_loc = uncertain_parm_loc;

>> @@ -2693,6 +2736,9 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>>   		if (!func)
>>   			continue;
>>   
>> +		if (ftype__has_uncertain_arg_loc(cu, &fn->proto))
>> +			fn->proto.uncertain_parm_loc = 1;
>> +
>>   		err = btf_encoder__save_func(encoder, fn, func);
>
> I think checking and setting uncertain_parm_loc flag should be done
> inside btf_encoder__save_func(), because that's where we inspect DWARF
> function prototype and add a new btf_encoder_func_state.

ACK, it can be moved there

Thanks,

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 2/3] tests: add some tests validating skipped functions due to uncertain arg location

[Alexis Lothoré]

On Thu Jul 3, 2025 at 8:31 PM CEST, Ihor Solodrai wrote:
> On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:
>> Add a small kernel module representing specific cases likely absent from
>> standard vmlinux files. As a starter, the introduced module exposes a
>> few functions consuming structs passed by value, some passed by
>> register, some passed on the stack:
>> 
>>    int kmod_test_init(void);
>>    int test_kmod_func_ok(int, void *, char, short int);
>>    int test_kmod_func_struct_ok(int, void *, char, struct kmod_struct);
>>    int test_kmod_func_struct_on_stack_ok(int, void *, char, short int, int, \
>>      void *, char, short int, struct kmod_struct);
>>    int test_kmod_func_struct_on_stack_ko(int, void *, char, short int, int, \
>>      void *, char, short int, struct kmod_struct_packed);
>> 
>> Then enrich btf_functions.sh to make it perform the following steps:
>> - build the module
>> - generate BTF info and pfunct listing, both with dwarf and the
>>    generated BTF
>> - check that any function encoded in BTF is found in DWARF
>> - check that any function announced as skipped is indeed absent from BTF
>> - check that any skipped function has been skipped due to uncertain
>>    parameter location
>> 
>> Those new tests are executed only if a kernel directory is provided as
>> script's second argument, they are otherwise skipped.
>
> While this shouldn't be a problem for CI, since it checks out a kernel
> tree to test vmlinux as input, I wonder if there is a way to do the
> same test without this dependency.
>
> We need to generate a binary with DWARF, containing function
> prototypes with packed/aligned attributes. Give it to pahole and see
> that those functions were skipped.
>
> Any reason it must be a kernel module? Am I missing something?

I guess I have no valid reason, I just focused too much on a specific use
case :) It would indeed be simpler with a bare userspace binary, I'll check
further and change it.

>> Example of the new test execution:
>>    Encoding...Matched 4 functions exactly.
>>    Ok
>>    Validation of skipped function logic...
>>    Skipped encoding 1 functions in BTF.
>>    Ok
>>    Validating skipped functions have uncertain parameter location...
>>    Found 1 legitimately skipped function due to uncertain loc
>>    Ok

> This part fails for me:
>
> isolodrai@isolodrai-fedora-PC2K40WQ:~/pahole/tests$ 
> KDIR=/home/isolodrai/kernels/bpf-next 
> vmlinux=/home/isolodrai/kernels/bpf-next/vmlinux ./btf_functions.sh
> Validation of BTF encoding of functions; this may take some time: Ok
> Validation of BTF encoding corner cases with kmod functions; this may 
> take some time: make: Entering directory '/home/isolodrai/kernels/bpf-next'
> Makefile:199: *** specified external module directory "./kmod" does not 
> exist.  Stop.
> make: Leaving directory '/home/isolodrai/kernels/bpf-next'
> No skipped functions.  Done.
>
> Maybe:
>
> diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
> index 64810b7..fcb1591 100755
> --- a/tests/btf_functions.sh
> +++ b/tests/btf_functions.sh
> @@ -208,7 +208,7 @@ fi
>   echo -n "Validation of BTF encoding corner cases with kmod functions; 
> this may take some time: "
>
>   test -n "$VERBOSE" && printf "\nBuilding kmod..."
> -tests_dir=$(dirname $0)
> +tests_dir=$(realpath $(dirname $0))
>   make -C ${KDIR} M=${tests_dir}/kmod
>
>   test -n "$VERBOSE" && printf "\nEncoding..."
>
>
> Also, in case kernel is built with LLVM, one must set LLVM=1.
> Not sure if this is detectable by the test.

Yeah, the tests_dir computation is a bit fragile. I saw it in tests.sh, and
so I assumed use cases were simple enough to keep this simple logic. I'll
update it to make it more robust.

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack

[Ihor Solodrai]

On 7/4/25 2:01 AM, Alexis Lothoré wrote:
> Hello Ihor,
> 
> thanks for the prompt feedback and testing !
> 
> On Thu Jul 3, 2025 at 8:17 PM CEST, Ihor Solodrai wrote:
>> On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:
> 
> [...]
> 
>>>    		/* do not exclude functions with optimized-out parameters; they
>>>    		 * may still be _called_ with the right parameter values, they
>>>    		 * just do not _use_ them.  Only exclude functions with
>>> -		 * unexpected register use or multiple inconsistent prototypes.
>>> +		 * unexpected register use, multiple inconsistent prototypes or
>>> +		 * uncertain parameters location
>>>    		 */
>>> -		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto;
>>> +		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc;
>>
>>
>> Is it possible for a function to have uncertain_parm_loc in one CU,
>> but not in another?
>>
>> If yes, we still don't want the function in BTF, right?
> 
> TBH, my understanding about those discrepancies between CUs about the same
> functions and how pahole handle them is still a bit fragile. Have you got
> any example about how it could be the case ?

I would hope stuff like this doesn't happen often in the real
world, but in principle you could have the following situation:

#ifdef ENABLE_PACKING
struct some_data {
     char header;
     int payload;
     short footer;
} __attribute__((packed));
#else
struct some_data {
     char header;
     int payload;
     short footer;
};
#endif

void read_data(/* lots of args */, struct some_data data) { ... }

And then one user has #define ENABLE_PACKING and the other doesn't,
for example:

#define ENABLE_PACKING // or not
#include "some_data.h"

void user() {
      struct some_data = get_some_data();
      ...
      read_data(/* args */, some_data);
}

And then you compile a binary with both users:

$ gcc -g -O0 user1.c user2.c

DWARF will contain both packed and not packed instances of struct
some_data and two corresponding read_data() funcs.

> 
> If it _can_ happen, I guess you are suggesting to make sure that copies are
> compared in saved_functions_combine and their uncertain_loc_parm flag are
> aligned. Something like this:
> 
> uncertain_parm_loc = a->uncertain_parm_loc | b->uncertain_parm_loc;
> [...]
> a->uncertain_parm_loc = b->uncertain_parm_loc = uncertain_parm_loc;

I asked out of curiosity, but you're right that this piece is needed.

> 
>>> @@ -2693,6 +2736,9 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>>>    		if (!func)
>>>    			continue;
>>>    
>>> +		if (ftype__has_uncertain_arg_loc(cu, &fn->proto))
>>> +			fn->proto.uncertain_parm_loc = 1;
>>> +
>>>    		err = btf_encoder__save_func(encoder, fn, func);
>>
>> I think checking and setting uncertain_parm_loc flag should be done
>> inside btf_encoder__save_func(), because that's where we inspect DWARF
>> function prototype and add a new btf_encoder_func_state.
> 
> ACK, it can be moved there
> 
> Thanks,
> 
> Alexis
> 

Re: [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack

[Ihor Solodrai]

On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:
> Most ABIs allow functions to receive structs passed by value, if they
> fit in a register or a pair of registers, depending on the exact ABI.
> However, when there is a struct passed by value but all registers are
> already used for parameters passing, the struct is still passed by value
> but on the stack. This becomes an issue if the passed struct is defined
> with some attributes like __attribute__((packed)) or
> __attribute__((aligned(X)), as its location on the stack is altered, but
> this change is not reflected in dwarf information. The corresponding BTF
> data generated from this can lead to incorrect BPF trampolines
> generation (eg to attach bpf tracing programs to kernel functions) in
> the Linux kernel.
> 
> Prevent those wrong cases by not encoding functions consuming structs
> passed by value on stack, when those structs do not have the expected
> alignment due to some attribute usage.
> 
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
> Changes in v2:
> - do not deny any struct passed by value, only those passed on stack AND
>    with some attribute alteration
> - use the existing class__infer_packed_attributes to deduce is a struct
>    is "altered". As a consequence, move the function filtering from
>    parameter__new to btf_encoder__encode_cu, to make sure that all the
>    needed data has been parsed from debug info
> ---
>   btf_encoder.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
>   dwarves.h     |  1 +
>   2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 0bc23349b5d740c3ddab8208b2e15cdbdd139b9d..16739066caae808aea77175e6c221afbe37b7c70 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -87,6 +87,7 @@ struct btf_encoder_func_state {
>   	uint8_t optimized_parms:1;
>   	uint8_t unexpected_reg:1;
>   	uint8_t inconsistent_proto:1;
> +	uint8_t uncertain_parm_loc:1;
>   	int ret_type_id;
>   	struct btf_encoder_func_parm *parms;
>   	struct btf_encoder_func_annot *annots;
> @@ -1203,6 +1204,7 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
>   	state->inconsistent_proto = ftype->inconsistent_proto;
>   	state->unexpected_reg = ftype->unexpected_reg;
>   	state->optimized_parms = ftype->optimized_parms;
> +	state->uncertain_parm_loc = ftype->uncertain_parm_loc;
>   	ftype__for_each_parameter(ftype, param) {
>   		const char *name = parameter__name(param) ?: "";
>   
> @@ -1430,9 +1432,15 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
>   		/* do not exclude functions with optimized-out parameters; they
>   		 * may still be _called_ with the right parameter values, they
>   		 * just do not _use_ them.  Only exclude functions with
> -		 * unexpected register use or multiple inconsistent prototypes.
> +		 * unexpected register use, multiple inconsistent prototypes or
> +		 * uncertain parameters location
>   		 */
> -		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto;
> +		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc;
> +
> +		if (state->uncertain_parm_loc)
> +			btf_encoder__log_func_skip(encoder, saved_fns[i].elf,
> +					"uncertain parameter location\n",
> +					0, 0);
>   
>   		if (add_to_btf) {
>   			err = btf_encoder__add_func(state->encoder, state);
> @@ -2553,6 +2561,39 @@ void btf_encoder__delete(struct btf_encoder *encoder)
>   	free(encoder);
>   }
>   
> +static bool ftype__has_uncertain_arg_loc(struct cu *cu, struct ftype *ftype)
> +{
> +	struct parameter *param;
> +	int param_idx = 0;
> +
> +	if (ftype->nr_parms < cu->nr_register_params)
> +		return false;
> +
> +	ftype__for_each_parameter(ftype, param) {
> +		if (param_idx++ < cu->nr_register_params)
> +			continue;
> +
> +		struct tag *type = cu__type(cu, param->tag.type);
> +
> +		if (type == NULL || !tag__is_struct(type))
> +			continue;
> +
> +		struct type *ctype = tag__type(type);
> +		if (ctype->namespace.name == 0)
> +			continue;
> +
> +		struct class *class = tag__class(type);
> +
> +		class__find_holes(class);
> +		class__infer_packed_attributes(class, cu);

I just noticed that class__infer_packed_attributes() already does call
class__find_holes() [1], so calling it here is unnecessary. Although
there is already a flag to detect repeated calls [2].

[1] https://github.com/acmel/dwarves/blob/master/dwarves.c#L1859
[2] https://github.com/acmel/dwarves/blob/master/dwarves.c#L1604-L1605

> +
> +		if (class->is_packed)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>   int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct conf_load *conf_load)
>   {
>   	struct llvm_annotation *annot;
> @@ -2647,6 +2688,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>   		 * Skip functions that:
>   		 *   - are marked as declarations
>   		 *   - do not have full argument names
> +		 *   - have arguments with uncertain locations, e.g packed
> +		 *   structs passed by value on stack
>   		 *   - are not in ftrace list (if it's available)
>   		 *   - are not external (in case ftrace filter is not available)
>   		 */
> @@ -2693,6 +2736,9 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>   		if (!func)
>   			continue;
>   
> +		if (ftype__has_uncertain_arg_loc(cu, &fn->proto))
> +			fn->proto.uncertain_parm_loc = 1;
> +
>   		err = btf_encoder__save_func(encoder, fn, func);
>   		if (err)
>   			goto out;
> diff --git a/dwarves.h b/dwarves.h
> index 36c689847ebf29a1ab9936f9d0f928dd46514547..d689aee5910f4b40dc13b3e9dc596dfbe6a2c3d0 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -1021,6 +1021,7 @@ struct ftype {
>   	uint8_t		 unexpected_reg:1;
>   	uint8_t		 processed:1;
>   	uint8_t		 inconsistent_proto:1;
> +	uint8_t		 uncertain_parm_loc:1;
>   	struct list_head template_type_params;
>   	struct list_head template_value_params;
>   	struct template_parameter_pack *template_parameter_pack;
> 

Re: [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack

[Alexis Lothoré]

On Fri Jul 4, 2025 at 9:59 PM CEST, Ihor Solodrai wrote:
> On 7/4/25 2:01 AM, Alexis Lothoré wrote:
>> Hello Ihor,
>> 
>> thanks for the prompt feedback and testing !
>> 
>> On Thu Jul 3, 2025 at 8:17 PM CEST, Ihor Solodrai wrote:
>>> On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:
>> 
>> [...]
>> 
>>>>    		/* do not exclude functions with optimized-out parameters; they
>>>>    		 * may still be _called_ with the right parameter values, they
>>>>    		 * just do not _use_ them.  Only exclude functions with
>>>> -		 * unexpected register use or multiple inconsistent prototypes.
>>>> +		 * unexpected register use, multiple inconsistent prototypes or
>>>> +		 * uncertain parameters location
>>>>    		 */
>>>> -		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto;
>>>> +		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc;
>>>
>>>
>>> Is it possible for a function to have uncertain_parm_loc in one CU,
>>> but not in another?
>>>
>>> If yes, we still don't want the function in BTF, right?
>> 
>> TBH, my understanding about those discrepancies between CUs about the same
>> functions and how pahole handle them is still a bit fragile. Have you got
>> any example about how it could be the case ?
>
> I would hope stuff like this doesn't happen often in the real
> world, but in principle you could have the following situation:
>
> #ifdef ENABLE_PACKING
> struct some_data {
>      char header;
>      int payload;
>      short footer;
> } __attribute__((packed));
> #else
> struct some_data {
>      char header;
>      int payload;
>      short footer;
> };
> #endif
>
> void read_data(/* lots of args */, struct some_data data) { ... }
>
> And then one user has #define ENABLE_PACKING and the other doesn't,
> for example:
>
> #define ENABLE_PACKING // or not
> #include "some_data.h"
>
> void user() {
>       struct some_data = get_some_data();
>       ...
>       read_data(/* args */, some_data);
> }
>
> And then you compile a binary with both users:
>
> $ gcc -g -O0 user1.c user2.c
>
> DWARF will contain both packed and not packed instances of struct
> some_data and two corresponding read_data() funcs.

Got it, thanks for the clarification !

Re: [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack

[Alexis Lothoré]

On Fri Jul 4, 2025 at 10:05 PM CEST, Ihor Solodrai wrote:
> On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:

[...]

>> +static bool ftype__has_uncertain_arg_loc(struct cu *cu, struct ftype *ftype)
>> +{
>> +	struct parameter *param;
>> +	int param_idx = 0;
>> +
>> +	if (ftype->nr_parms < cu->nr_register_params)
>> +		return false;
>> +
>> +	ftype__for_each_parameter(ftype, param) {
>> +		if (param_idx++ < cu->nr_register_params)
>> +			continue;
>> +
>> +		struct tag *type = cu__type(cu, param->tag.type);
>> +
>> +		if (type == NULL || !tag__is_struct(type))
>> +			continue;
>> +
>> +		struct type *ctype = tag__type(type);
>> +		if (ctype->namespace.name == 0)
>> +			continue;
>> +
>> +		struct class *class = tag__class(type);
>> +
>> +		class__find_holes(class);
>> +		class__infer_packed_attributes(class, cu);
>
> I just noticed that class__infer_packed_attributes() already does call
> class__find_holes() [1], so calling it here is unnecessary. Although
> there is already a flag to detect repeated calls [2].
>
> [1] https://github.com/acmel/dwarves/blob/master/dwarves.c#L1859
> [2] https://github.com/acmel/dwarves/blob/master/dwarves.c#L1604-L1605

ACK, I'll remove the duplicate class__find_holes then.