[PATCH dwarves v1] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[PATCH dwarves v1] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Tony Ambardar]

While doing JIT development on armhf BTF kernels, I hit a strange issue
where some functions were missing in BTF data. This required considerable
debugging but can be reproduced simply:

$ bpftool --version
bpftool v7.6.0
using libbpf v1.6
features: llvm, skeletons

$ pahole --version
v1.29

$ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'

$ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
<nothing>

$ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);

$ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf

$ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);

The key things to note are the pahole 'consistent_func' feature and the u64
'wake_flags' parameter vs. arm 32-bit registers. These point to existing
code handling arguments larger than register-size, but only structs.

Generalize the code for any type of argument exceeding register size (i.e.
cu->addr_size). This should work for integral or aggregate types, and also
avoids a bug in the current code where a register-sized struct could be
mistaken for larger.

Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
---
 dwarf_loader.c | 37 ++++++++++++-------------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index e1ba7bc..22abfdb 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2914,23 +2914,9 @@ out:
 	return 0;
 }
 
-static bool param__is_struct(struct cu *cu, struct tag *tag)
+static bool param__is_wide(struct cu *cu, struct tag *tag)
 {
-	struct tag *type = cu__type(cu, tag->type);
-
-	if (!type)
-		return false;
-
-	switch (type->tag) {
-	case DW_TAG_structure_type:
-		return true;
-	case DW_TAG_const_type:
-	case DW_TAG_typedef:
-		/* handle "typedef struct", const parameter */
-		return param__is_struct(cu, type);
-	default:
-		return false;
-	}
+	return tag__size(tag, cu) > cu->addr_size;
 }
 
 static int cu__resolve_func_ret_types_optimized(struct cu *cu)
@@ -2942,9 +2928,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		struct tag *tag = pt->entries[i];
 		struct parameter *pos;
 		struct function *fn = tag__function(tag);
-		bool has_unexpected_reg = false, has_struct_param = false;
+		bool has_unexpected_reg = false, has_wide_param = false;
 
-		/* mark function as optimized if parameter is, or
+		/* Mark function as optimized if parameter is, or
 		 * if parameter does not have a location; at this
 		 * point location presence has been marked in
 		 * abstract origins for cases where a parameter
@@ -2953,10 +2939,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		 *
 		 * Also mark functions which, due to optimization,
 		 * use an unexpected register for a parameter.
-		 * Exception is functions which have a struct
-		 * as a parameter, as multiple registers may
-		 * be used to represent it, throwing off register
-		 * to parameter mapping.
+		 * Exception is functions which have a wide
+		 * parameter, as multiple registers may be used
+		 * to represent it, throwing off register to
+		 * parameter mapping. Examples could include
+		 * structs or 64-bit types on a 32-bit arch.
 		 */
 		ftype__for_each_parameter(&fn->proto, pos) {
 			if (pos->optimized || !pos->has_loc)
@@ -2967,11 +2954,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		}
 		if (has_unexpected_reg) {
 			ftype__for_each_parameter(&fn->proto, pos) {
-				has_struct_param = param__is_struct(cu, &pos->tag);
-				if (has_struct_param)
+				has_wide_param = param__is_wide(cu, &pos->tag);
+				if (has_wide_param)
 					break;
 			}
-			if (!has_struct_param)
+			if (!has_wide_param)
 				fn->proto.unexpected_reg = 1;
 		}
 
-- 
2.34.1

Re: [PATCH dwarves v1] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Alan Maguire]

On 10/04/2025 09:33, Tony Ambardar wrote:
> While doing JIT development on armhf BTF kernels, I hit a strange issue
> where some functions were missing in BTF data. This required considerable
> debugging but can be reproduced simply:
> 
> $ bpftool --version
> bpftool v7.6.0
> using libbpf v1.6
> features: llvm, skeletons
> 
> $ pahole --version
> v1.29
> 
> $ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
> btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
> btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'
> 
> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> <nothing>
> 
> $ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> 
> $ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf
> 
> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> 
> The key things to note are the pahole 'consistent_func' feature and the u64
> 'wake_flags' parameter vs. arm 32-bit registers. These point to existing
> code handling arguments larger than register-size, but only structs.
> 
> Generalize the code for any type of argument exceeding register size (i.e.
> cu->addr_size). This should work for integral or aggregate types, and also
> avoids a bug in the current code where a register-sized struct could be
> mistaken for larger.
> 
> Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>

Thanks for investigating this! I've tested this versus baseline on
x86_64 and aarch64. I'm seeing some small divergence in functions
encoded; for example on aarch64 we don't get a representation for

static int __io_run_local_work(struct io_ring_ctx *ctx, io_tw_token_t
tw, int min_events, int max_events);

The reason for that is the second argument is a typedef io_tw_token_t,
which is in turn a typedef for:

struct io_tw_state {
};

i.e. an empty struct.

The reason is with your patch we've moved from type-centric to
size-centric criteria used to allow functions into BTF that have
unexpected register usage; because the above function uses unexpected
registers _and_ does not exceed the address size, the function is marked
as having an inconsistent reg mapping. In this case, that seems
reasonable since it is true; there is no register needed to represent
the second argument.

The deeper rationale here in allowing functions that have structs that
may be represented by multiple registers is that we can handle this
outcome; the BPF_PROG2() macro was added to handle such cases and seems
to handle multi-register representation but _not_ representations where
a register is not needed at all. I'm basing that on the
___bpf_union_arg() macro in bpf_tracing.h so please correct me if I'm
wrong (we could potentially add a sizeof(t) == 0 clause here perhaps).

So in other words, though we see small divergences in representation I
_think_ they are consistent with our expectations.

I'd really like to see wider testing of this patch before it lands
however so we can shake out other problematic cases if any. If folks
could try this and compare BTF representations to baseline that would be
great! In particular comparing raw BTF is necessary since vmlinux.h
representations don't include functions (aside from kfuncs). Now that we
have always-reproducible BTF a simple diff of "bpftool btf dump file
vmlinux" can be used to make such comparisons.

However perhaps we could also think about enhancing the bpf_tracing.h
macro to handle zero-sized parameters like empty structs such that later
parameters are mapped to registers correctly (presuming that's
possible)? Yonghong, what do you think?

Thanks!

Alan

> ---
>  dwarf_loader.c | 37 ++++++++++++-------------------------
>  1 file changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index e1ba7bc..22abfdb 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -2914,23 +2914,9 @@ out:
>  	return 0;
>  }
>  
> -static bool param__is_struct(struct cu *cu, struct tag *tag)
> +static bool param__is_wide(struct cu *cu, struct tag *tag)
>  {
> -	struct tag *type = cu__type(cu, tag->type);
> -
> -	if (!type)
> -		return false;
> -
> -	switch (type->tag) {
> -	case DW_TAG_structure_type:
> -		return true;
> -	case DW_TAG_const_type:
> -	case DW_TAG_typedef:
> -		/* handle "typedef struct", const parameter */
> -		return param__is_struct(cu, type);
> -	default:
> -		return false;
> -	}
> +	return tag__size(tag, cu) > cu->addr_size;
>  }
>  
>  static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> @@ -2942,9 +2928,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		struct tag *tag = pt->entries[i];
>  		struct parameter *pos;
>  		struct function *fn = tag__function(tag);
> -		bool has_unexpected_reg = false, has_struct_param = false;
> +		bool has_unexpected_reg = false, has_wide_param = false;
>  
> -		/* mark function as optimized if parameter is, or
> +		/* Mark function as optimized if parameter is, or
>  		 * if parameter does not have a location; at this
>  		 * point location presence has been marked in
>  		 * abstract origins for cases where a parameter
> @@ -2953,10 +2939,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		 *
>  		 * Also mark functions which, due to optimization,
>  		 * use an unexpected register for a parameter.
> -		 * Exception is functions which have a struct
> -		 * as a parameter, as multiple registers may
> -		 * be used to represent it, throwing off register
> -		 * to parameter mapping.
> +		 * Exception is functions which have a wide
> +		 * parameter, as multiple registers may be used
> +		 * to represent it, throwing off register to
> +		 * parameter mapping. Examples could include
> +		 * structs or 64-bit types on a 32-bit arch.
>  		 */
>  		ftype__for_each_parameter(&fn->proto, pos) {
>  			if (pos->optimized || !pos->has_loc)
> @@ -2967,11 +2954,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		}
>  		if (has_unexpected_reg) {
>  			ftype__for_each_parameter(&fn->proto, pos) {
> -				has_struct_param = param__is_struct(cu, &pos->tag);
> -				if (has_struct_param)
> +				has_wide_param = param__is_wide(cu, &pos->tag);
> +				if (has_wide_param)
>  					break;
>  			}
> -			if (!has_struct_param)
> +			if (!has_wide_param)
>  				fn->proto.unexpected_reg = 1;
>  		}
>  

Re: [PATCH dwarves v1] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Tony Ambardar]

On Thu, Apr 10, 2025 at 01:20:45PM +0100, Alan Maguire wrote:
> On 10/04/2025 09:33, Tony Ambardar wrote:
> > While doing JIT development on armhf BTF kernels, I hit a strange issue
> > where some functions were missing in BTF data. This required considerable
> > debugging but can be reproduced simply:
> > 
> > $ bpftool --version
> > bpftool v7.6.0
> > using libbpf v1.6
> > features: llvm, skeletons
> > 
> > $ pahole --version
> > v1.29
> > 
> > $ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
> > btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
> > btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'
> > 
> > $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> > <nothing>
> > 
> > $ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> > s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> > 
> > $ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf
> > 
> > $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> > bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> > 
> > The key things to note are the pahole 'consistent_func' feature and the u64
> > 'wake_flags' parameter vs. arm 32-bit registers. These point to existing
> > code handling arguments larger than register-size, but only structs.
> > 
> > Generalize the code for any type of argument exceeding register size (i.e.
> > cu->addr_size). This should work for integral or aggregate types, and also
> > avoids a bug in the current code where a register-sized struct could be
> > mistaken for larger.
> > 
> > Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> 
> Thanks for investigating this! I've tested this versus baseline on
> x86_64 and aarch64. I'm seeing some small divergence in functions
> encoded; for example on aarch64 we don't get a representation for
> 
> static int __io_run_local_work(struct io_ring_ctx *ctx, io_tw_token_t
> tw, int min_events, int max_events);
> 
> The reason for that is the second argument is a typedef io_tw_token_t,
> which is in turn a typedef for:
> 
> struct io_tw_state {
> };
> 
> i.e. an empty struct.
> 
> The reason is with your patch we've moved from type-centric to
> size-centric criteria used to allow functions into BTF that have
> unexpected register usage; because the above function uses unexpected
> registers _and_ does not exceed the address size, the function is marked
> as having an inconsistent reg mapping. In this case, that seems
> reasonable since it is true; there is no register needed to represent
> the second argument.
> 
> The deeper rationale here in allowing functions that have structs that
> may be represented by multiple registers is that we can handle this
> outcome; the BPF_PROG2() macro was added to handle such cases and seems
> to handle multi-register representation but _not_ representations where
> a register is not needed at all. I'm basing that on the
> ___bpf_union_arg() macro in bpf_tracing.h so please correct me if I'm
> wrong (we could potentially add a sizeof(t) == 0 clause here perhaps).
> 
> So in other words, though we see small divergences in representation I
> _think_ they are consistent with our expectations.
> 
> I'd really like to see wider testing of this patch before it lands
> however so we can shake out other problematic cases if any. If folks
> could try this and compare BTF representations to baseline that would be
> great! In particular comparing raw BTF is necessary since vmlinux.h
> representations don't include functions (aside from kfuncs). Now that we
> have always-reproducible BTF a simple diff of "bpftool btf dump file
> vmlinux" can be used to make such comparisons.
> 
> However perhaps we could also think about enhancing the bpf_tracing.h
> macro to handle zero-sized parameters like empty structs such that later
> parameters are mapped to registers correctly (presuming that's
> possible)? Yonghong, what do you think?

Hi Alan,

Thanks so much for the additional context. I pressed pause to consider
this while waiting for further testing news or feedback, but haven't seen
anything since. Have you heard anything OOB?

I also understood dwarves could have CI working now, so wondering how
those tests with the patch might have gone. In fact, it would be great to
have a regular arm32 CI running if that's possible. Could you share how
the CI changes are being managed? I've recently been trying to update
the arm32 JIT and test_progs in tandem, with the goal of having a working
32-bit target for kernel-patches/bpf CI, but some baby-steps with dwarves
or libbpf could be very helpful.

As far as type-based vs size-based criteria, I'm not wedded to either, and
did look at the type-based route as currently exists. I needed to add
cases for DW_TAG_base_type (for ints), DW_TAG_volatile_type (recursive),
DW_TAG_union_type (same issues as structs), and then we still need size
tests anyway. Sticking with size-based (and a zero-test as you suggested)
seemed the simplest and preserved the functions you noticed missing.

Cheers,
Tony

> 
> Thanks!
> 
> Alan
> 
> > ---
> >  dwarf_loader.c | 37 ++++++++++++-------------------------
> >  1 file changed, 12 insertions(+), 25 deletions(-)
> > 
> > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > index e1ba7bc..22abfdb 100644
> > --- a/dwarf_loader.c
> > +++ b/dwarf_loader.c
> > @@ -2914,23 +2914,9 @@ out:
> >  	return 0;
> >  }
> >  
> > -static bool param__is_struct(struct cu *cu, struct tag *tag)
> > +static bool param__is_wide(struct cu *cu, struct tag *tag)
> >  {
> > -	struct tag *type = cu__type(cu, tag->type);
> > -
> > -	if (!type)
> > -		return false;
> > -
> > -	switch (type->tag) {
> > -	case DW_TAG_structure_type:
> > -		return true;
> > -	case DW_TAG_const_type:
> > -	case DW_TAG_typedef:
> > -		/* handle "typedef struct", const parameter */
> > -		return param__is_struct(cu, type);
> > -	default:
> > -		return false;
> > -	}
> > +	return tag__size(tag, cu) > cu->addr_size;
> >  }
> >  
> >  static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> > @@ -2942,9 +2928,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> >  		struct tag *tag = pt->entries[i];
> >  		struct parameter *pos;
> >  		struct function *fn = tag__function(tag);
> > -		bool has_unexpected_reg = false, has_struct_param = false;
> > +		bool has_unexpected_reg = false, has_wide_param = false;
> >  
> > -		/* mark function as optimized if parameter is, or
> > +		/* Mark function as optimized if parameter is, or
> >  		 * if parameter does not have a location; at this
> >  		 * point location presence has been marked in
> >  		 * abstract origins for cases where a parameter
> > @@ -2953,10 +2939,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> >  		 *
> >  		 * Also mark functions which, due to optimization,
> >  		 * use an unexpected register for a parameter.
> > -		 * Exception is functions which have a struct
> > -		 * as a parameter, as multiple registers may
> > -		 * be used to represent it, throwing off register
> > -		 * to parameter mapping.
> > +		 * Exception is functions which have a wide
> > +		 * parameter, as multiple registers may be used
> > +		 * to represent it, throwing off register to
> > +		 * parameter mapping. Examples could include
> > +		 * structs or 64-bit types on a 32-bit arch.
> >  		 */
> >  		ftype__for_each_parameter(&fn->proto, pos) {
> >  			if (pos->optimized || !pos->has_loc)
> > @@ -2967,11 +2954,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> >  		}
> >  		if (has_unexpected_reg) {
> >  			ftype__for_each_parameter(&fn->proto, pos) {
> > -				has_struct_param = param__is_struct(cu, &pos->tag);
> > -				if (has_struct_param)
> > +				has_wide_param = param__is_wide(cu, &pos->tag);
> > +				if (has_wide_param)
> >  					break;
> >  			}
> > -			if (!has_struct_param)
> > +			if (!has_wide_param)
> >  				fn->proto.unexpected_reg = 1;
> >  		}
> >  
> 

[PATCH dwarves v2] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Tony Ambardar]

I encountered an issue building BTF kernels for 32-bit armhf, where many
functions are missing in BTF data:

  LD      vmlinux
  BTFIDS  vmlinux
WARN: resolve_btfids: unresolved symbol vfs_truncate
WARN: resolve_btfids: unresolved symbol vfs_fallocate
WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl
WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node
WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu
WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node
WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu
WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu
WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch
WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq
WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq
WARN: resolve_btfids: unresolved symbol scx_bpf_consume
WARN: resolve_btfids: unresolved symbol bpf_throw
WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp
WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl
WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl
WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key
WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key
WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new
WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new
WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache
WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp
WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb
WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id
  NM      System.map

After further debugging this can be reproduced more simply:

$ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'

$ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
<nothing>

$ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);

$ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf

$ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);

The key things to note are the pahole 'consistent_func' feature and the u64
'wake_flags' parameter vs. arm 32-bit registers. These point to existing
code handling arguments larger than register-size, but only structs.

Generalize the code for any type of argument misfit to register size (i.e.
zero or > cu->addr_size). This should work for integral or aggregate types,
and also avoids a bug in the current code where a register-sized struct
could be mistaken for larger.

Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
---
v1 -> v2:
 - Update to preserve existing behaviour where zero-sized struct params
   still permit the function to be encoded, as noted by Alan.

---
 dwarf_loader.c | 37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index e1ba7bc..abf1717 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2914,23 +2914,11 @@ out:
 	return 0;
 }
 
-static bool param__is_struct(struct cu *cu, struct tag *tag)
+static bool param__misfits_reg(struct cu *cu, struct tag *tag)
 {
-	struct tag *type = cu__type(cu, tag->type);
+	size_t sz = tag__size(tag, cu);
 
-	if (!type)
-		return false;
-
-	switch (type->tag) {
-	case DW_TAG_structure_type:
-		return true;
-	case DW_TAG_const_type:
-	case DW_TAG_typedef:
-		/* handle "typedef struct", const parameter */
-		return param__is_struct(cu, type);
-	default:
-		return false;
-	}
+	return sz == 0 || sz > cu->addr_size;
 }
 
 static int cu__resolve_func_ret_types_optimized(struct cu *cu)
@@ -2942,9 +2930,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		struct tag *tag = pt->entries[i];
 		struct parameter *pos;
 		struct function *fn = tag__function(tag);
-		bool has_unexpected_reg = false, has_struct_param = false;
+		bool has_unexpected_reg = false, has_misfit_param = false;
 
-		/* mark function as optimized if parameter is, or
+		/* Mark function as optimized if parameter is, or
 		 * if parameter does not have a location; at this
 		 * point location presence has been marked in
 		 * abstract origins for cases where a parameter
@@ -2953,10 +2941,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		 *
 		 * Also mark functions which, due to optimization,
 		 * use an unexpected register for a parameter.
-		 * Exception is functions which have a struct
-		 * as a parameter, as multiple registers may
-		 * be used to represent it, throwing off register
-		 * to parameter mapping.
+		 * Exception is functions with a wide/zero-sized
+		 * parameter, as single register won't be used
+		 * to represent it, throwing off register to
+		 * parameter mapping. Examples include large
+		 * structs or 64-bit types on a 32-bit arch.
 		 */
 		ftype__for_each_parameter(&fn->proto, pos) {
 			if (pos->optimized || !pos->has_loc)
@@ -2967,11 +2956,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		}
 		if (has_unexpected_reg) {
 			ftype__for_each_parameter(&fn->proto, pos) {
-				has_struct_param = param__is_struct(cu, &pos->tag);
-				if (has_struct_param)
+				has_misfit_param = param__misfits_reg(cu, &pos->tag);
+				if (has_misfit_param)
 					break;
 			}
-			if (!has_struct_param)
+			if (!has_misfit_param)
 				fn->proto.unexpected_reg = 1;
 		}
 
-- 
2.34.1

Re: [PATCH dwarves v2] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Alexis Lothoré]

Hello,

On Fri May 2, 2025 at 9:03 AM CEST, Tony Ambardar wrote:
> I encountered an issue building BTF kernels for 32-bit armhf, where many
> functions are missing in BTF data:

[...]

> Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>

I encountered some issues with pahole 1.30 when trying to generate BTF data
for functions having some __int128 values ([1]), and have been redirected
here by Tony. I gave a try to the patch below and confirm that it fixes my
issue: BTF data is now properly generated for my target function, so:

Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>

While at it, to follow-up on Alan's request for more testing, I did the
following:
- build kernel and bpf selftests with pahole 1.30, extract BTF raw data
  with bpftool
- repeat with pahole 1.30 + Tony's patch
- I build my kernel for arm64, it is based on bpf-next_base and I use a
  defconfig very close to the one used in BPF CI (so based on
  tools/testing/selftests/bpf/config*)

I observe the following when comparing the resulting BTF data with/without
Tony's patch:
- There is no difference on vmlinux BTF data
- For bpf_testmod.ko, there is a slight shift in the first BTF ID (first ID
  is 46 with pristine pahole, 47 with patched pahole), which in turns makes
  a lot of noise in the diff, but the actual diff seems to be about two new
  BTF entries related to my custom function now being properly detected
  (BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO)

Alexis

[1] https://lore.kernel.org/bpf/D9Q73OTLEOU4.LNAO9K4POETM@bootlin.com/

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

Re: [PATCH dwarves v2] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Tony Ambardar]

Hi Alexis,

On Thu, May 08, 2025 at 11:38:06AM +0200, Alexis Lothoré wrote:
> Hello,
> 
> On Fri May 2, 2025 at 9:03 AM CEST, Tony Ambardar wrote:
> > I encountered an issue building BTF kernels for 32-bit armhf, where many
> > functions are missing in BTF data:
> 
> [...]
> 
> > Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> 
> I encountered some issues with pahole 1.30 when trying to generate BTF data
> for functions having some __int128 values ([1]), and have been redirected
> here by Tony. I gave a try to the patch below and confirm that it fixes my
> issue: BTF data is now properly generated for my target function, so:
> 
> Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> 

Glad that resolved your issue, and thank you for the extra testing data
point.

> While at it, to follow-up on Alan's request for more testing, I did the
> following:
> - build kernel and bpf selftests with pahole 1.30, extract BTF raw data
>   with bpftool
> - repeat with pahole 1.30 + Tony's patch
> - I build my kernel for arm64, it is based on bpf-next_base and I use a
>   defconfig very close to the one used in BPF CI (so based on
>   tools/testing/selftests/bpf/config*)
> 

Nice! I notice bootlin has worked on several BPF testing contributions,
and was wondering if your build is some new standard buildroot/yocto
config tailored for BPF testing, and what archs it might support? Reason
for asking is I have a large stack of WIP patches for enabling use of
test_progs across 64/32-bit archs and cross-compilation, and I'm keen to
see other examples of configs, root images, etc. (especially for 32-bit)
At the moment I'm targeting 32-bit armhf support to make progress..

> I observe the following when comparing the resulting BTF data with/without
> Tony's patch:
> - There is no difference on vmlinux BTF data
> - For bpf_testmod.ko, there is a slight shift in the first BTF ID (first ID
>   is 46 with pristine pahole, 47 with patched pahole), which in turns makes
>   a lot of noise in the diff, but the actual diff seems to be about two new
>   BTF entries related to my custom function now being properly detected
>   (BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO)
> 

Right, I noticed the same skew and it does make checking equivalence more
complicated.

> Alexis
> 
> [1] https://lore.kernel.org/bpf/D9Q73OTLEOU4.LNAO9K4POETM@bootlin.com/
> 
> -- 
> Alexis Lothoré, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 
Thanks again,
Tony

Re: [PATCH dwarves v2] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Alexis Lothoré]

On Fri May 9, 2025 at 7:21 AM CEST, Tony Ambardar wrote:
> Hi Alexis,
>
> On Thu, May 08, 2025 at 11:38:06AM +0200, Alexis Lothoré wrote:
>> Hello,

[...]

> Nice! I notice bootlin has worked on several BPF testing contributions,
> and was wondering if your build is some new standard buildroot/yocto
> config tailored for BPF testing, and what archs it might support? Reason
> for asking is I have a large stack of WIP patches for enabling use of
> test_progs across 64/32-bit archs and cross-compilation, and I'm keen to
> see other examples of configs, root images, etc. (especially for 32-bit)
> At the moment I'm targeting 32-bit armhf support to make progress..

No,  that's really a custom, minimal setup that I am using, based on
buildroot. My workflow is roughly the following:
- use buildroot to download an arm64 toolchain and build a minimal rootfs.
  No specific defconfig used, it is a configuration from scratch, with
  additional tools for development and debugging
- configure a kernel for arm64 testing:
$ cat tools/testing/selftest/bpf/{config,config.vm,config.aarch64} >
.config
- use the toolchain downloaded by buildroot to build the kernel
- build the selftests with the same toolchain (so I am cross-building those
  directly from my host, I am not really using vmtest)
- run all of those in qemu, and run the selftests directly with test_progs
  in there

Alexis

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

Re: [PATCH dwarves v2] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Tony Ambardar]

On Fri, May 09, 2025 at 10:33:50AM +0200, Alexis Lothoré wrote:
> On Fri May 9, 2025 at 7:21 AM CEST, Tony Ambardar wrote:
> > Hi Alexis,
> >
> > On Thu, May 08, 2025 at 11:38:06AM +0200, Alexis Lothoré wrote:
> >> Hello,
> 
> [...]
> 
> > Nice! I notice bootlin has worked on several BPF testing contributions,
> > and was wondering if your build is some new standard buildroot/yocto
> > config tailored for BPF testing, and what archs it might support? Reason
> > for asking is I have a large stack of WIP patches for enabling use of
> > test_progs across 64/32-bit archs and cross-compilation, and I'm keen to
> > see other examples of configs, root images, etc. (especially for 32-bit)
> > At the moment I'm targeting 32-bit armhf support to make progress..
> 
> No,  that's really a custom, minimal setup that I am using, based on
> buildroot. My workflow is roughly the following:
> - use buildroot to download an arm64 toolchain and build a minimal rootfs.
>   No specific defconfig used, it is a configuration from scratch, with
>   additional tools for development and debugging
> - configure a kernel for arm64 testing:
> $ cat tools/testing/selftest/bpf/{config,config.vm,config.aarch64} >
> .config
> - use the toolchain downloaded by buildroot to build the kernel
> - build the selftests with the same toolchain (so I am cross-building those
>   directly from my host, I am not really using vmtest)
> - run all of those in qemu, and run the selftests directly with test_progs
>   in there

Understood, and thanks for the details. I basically do the same, with only a
couple of differences intended to ease adding armhf as a bpf-ci target
eventually:

 - use the Ubuntu arm cross-toolchain to build on x86_64
 - use mkrootfs tools from bpf-ci to make a Debian Bookworm rootfs

Take care,
Tony

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

Re: [PATCH dwarves v2] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Alan Maguire]

On 02/05/2025 08:03, Tony Ambardar wrote:
> I encountered an issue building BTF kernels for 32-bit armhf, where many
> functions are missing in BTF data:
> 
>   LD      vmlinux
>   BTFIDS  vmlinux
> WARN: resolve_btfids: unresolved symbol vfs_truncate
> WARN: resolve_btfids: unresolved symbol vfs_fallocate
> WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl
> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node
> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu
> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node
> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu
> WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu
> WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch
> WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq
> WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq
> WARN: resolve_btfids: unresolved symbol scx_bpf_consume
> WARN: resolve_btfids: unresolved symbol bpf_throw
> WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp
> WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl
> WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl
> WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key
> WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key
> WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new
> WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new
> WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache
> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp
> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb
> WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id
>   NM      System.map
> 
> After further debugging this can be reproduced more simply:
> 
> $ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
> btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
> btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'
> 
> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> <nothing>
> 
> $ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> 
> $ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf
> 
> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> 
> The key things to note are the pahole 'consistent_func' feature and the u64
> 'wake_flags' parameter vs. arm 32-bit registers. These point to existing
> code handling arguments larger than register-size, but only structs.
> 
> Generalize the code for any type of argument misfit to register size (i.e.
> zero or > cu->addr_size). This should work for integral or aggregate types,
> and also avoids a bug in the current code where a register-sized struct
> could be mistaken for larger.
> 
> Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>

I've tried this on x86_64 and the issue of missing functions has
disappeared; I get the exact same number of functions encoded. From a
pahole perspective

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

However as mentioned previously I think we need to think a bit about how
libbpf for example might accommodate such representations, as the
implict assumption for fentry in BPF_PROG() is that the function call
register conventions apply. BPF_PROG2() handles the struct case, but not
the 0-sized struct case, and in fact there are checks in btf.c that also
need to be fixed to enable verification once we have 0-sized struct
argument support.

So in investigating this I've put together a short RFC series [1] that
seems to do the job in

1. fixing up the BPF_PROG2() handling of 0-sized structs.
2. fixing the verification failures with 0-sized parameters, carving out
an exception for 0-sized structs.
3. testing the 0-sized struct case to ensure we get the correct data by
adding a function with a 0-sized struct parameter to bpf_testmod and
adding a tracing_struct test to verify the subsequent arguments are correct.

In terms of cadence, I would propose that if the BPF folks are happy
with the approach, we land this patch in pahole, then after that try to
land the BPF changes. Does that work from your side? Thanks!

[1]
https://lore.kernel.org/bpf/20250508132237.1817317-1-alan.maguire@oracle.com/

Alan

> ---
> v1 -> v2:
>  - Update to preserve existing behaviour where zero-sized struct params
>    still permit the function to be encoded, as noted by Alan.
> 
> ---
>  dwarf_loader.c | 37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index e1ba7bc..abf1717 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -2914,23 +2914,11 @@ out:
>  	return 0;
>  }
>  
> -static bool param__is_struct(struct cu *cu, struct tag *tag)
> +static bool param__misfits_reg(struct cu *cu, struct tag *tag)
>  {
> -	struct tag *type = cu__type(cu, tag->type);
> +	size_t sz = tag__size(tag, cu);
>  
> -	if (!type)
> -		return false;
> -
> -	switch (type->tag) {
> -	case DW_TAG_structure_type:
> -		return true;
> -	case DW_TAG_const_type:
> -	case DW_TAG_typedef:
> -		/* handle "typedef struct", const parameter */
> -		return param__is_struct(cu, type);
> -	default:
> -		return false;
> -	}
> +	return sz == 0 || sz > cu->addr_size;
>  }
>  
>  static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> @@ -2942,9 +2930,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		struct tag *tag = pt->entries[i];
>  		struct parameter *pos;
>  		struct function *fn = tag__function(tag);
> -		bool has_unexpected_reg = false, has_struct_param = false;
> +		bool has_unexpected_reg = false, has_misfit_param = false;
>  
> -		/* mark function as optimized if parameter is, or
> +		/* Mark function as optimized if parameter is, or
>  		 * if parameter does not have a location; at this
>  		 * point location presence has been marked in
>  		 * abstract origins for cases where a parameter
> @@ -2953,10 +2941,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		 *
>  		 * Also mark functions which, due to optimization,
>  		 * use an unexpected register for a parameter.
> -		 * Exception is functions which have a struct
> -		 * as a parameter, as multiple registers may
> -		 * be used to represent it, throwing off register
> -		 * to parameter mapping.
> +		 * Exception is functions with a wide/zero-sized
> +		 * parameter, as single register won't be used
> +		 * to represent it, throwing off register to
> +		 * parameter mapping. Examples include large
> +		 * structs or 64-bit types on a 32-bit arch.
>  		 */
>  		ftype__for_each_parameter(&fn->proto, pos) {
>  			if (pos->optimized || !pos->has_loc)
> @@ -2967,11 +2956,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		}
>  		if (has_unexpected_reg) {
>  			ftype__for_each_parameter(&fn->proto, pos) {
> -				has_struct_param = param__is_struct(cu, &pos->tag);
> -				if (has_struct_param)
> +				has_misfit_param = param__misfits_reg(cu, &pos->tag);
> +				if (has_misfit_param)
>  					break;
>  			}
> -			if (!has_struct_param)
> +			if (!has_misfit_param)
>  				fn->proto.unexpected_reg = 1;
>  		}
>  

Re: [PATCH dwarves v2] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Tony Ambardar]

On Thu, May 08, 2025 at 02:24:57PM +0100, Alan Maguire wrote:
> On 02/05/2025 08:03, Tony Ambardar wrote:
> > I encountered an issue building BTF kernels for 32-bit armhf, where many
> > functions are missing in BTF data:
> > 
> >   LD      vmlinux
> >   BTFIDS  vmlinux
> > WARN: resolve_btfids: unresolved symbol vfs_truncate

[...]

> > 
> > Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> 

Hello Alan,

> I've tried this on x86_64 and the issue of missing functions has
> disappeared; I get the exact same number of functions encoded. From a
> pahole perspective
> 
> Tested-by: Alan Maguire <alan.maguire@oracle.com>
> 

Thanks for reviewing the latest update and putting it through its paces.
I was noticing many small changes in my own testing so this is welcome
confirmation.

> However as mentioned previously I think we need to think a bit about how
> libbpf for example might accommodate such representations, as the
> implict assumption for fentry in BPF_PROG() is that the function call
> register conventions apply. BPF_PROG2() handles the struct case, but not
> the 0-sized struct case, and in fact there are checks in btf.c that also
> need to be fixed to enable verification once we have 0-sized struct
> argument support.
> 
> So in investigating this I've put together a short RFC series [1] that
> seems to do the job in
> 
> 1. fixing up the BPF_PROG2() handling of 0-sized structs.
> 2. fixing the verification failures with 0-sized parameters, carving out
> an exception for 0-sized structs.
> 3. testing the 0-sized struct case to ensure we get the correct data by
> adding a function with a 0-sized struct parameter to bpf_testmod and
> adding a tracing_struct test to verify the subsequent arguments are correct.
>

OK, I'll try looking at this for 32-bit armhf since I'm in the middle of
that, and comment in the RFC thread.

> In terms of cadence, I would propose that if the BPF folks are happy
> with the approach, we land this patch in pahole, then after that try to
> land the BPF changes. Does that work from your side? Thanks!
> 
> [1]
> https://lore.kernel.org/bpf/20250508132237.1817317-1-alan.maguire@oracle.com/
> 
> Alan
> 

Thanks,
Tony

[...]

[PATCH dwarves v3] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Tony Ambardar]

I encountered an issue building BTF kernels for 32-bit armhf, where many
functions are missing in BTF data:

  LD      vmlinux
  BTFIDS  vmlinux
WARN: resolve_btfids: unresolved symbol vfs_truncate
WARN: resolve_btfids: unresolved symbol vfs_fallocate
WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl
WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node
WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu
WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node
WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu
WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu
WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime
WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq
WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch
WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq
WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq
WARN: resolve_btfids: unresolved symbol scx_bpf_consume
WARN: resolve_btfids: unresolved symbol bpf_throw
WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp
WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl
WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl
WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key
WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key
WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new
WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new
WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache
WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp
WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb
WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id
  NM      System.map

After further debugging this can be reproduced more simply:

$ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'

$ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
<nothing>

$ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);

$ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf

$ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);

The key things to note are the pahole 'consistent_func' feature and the u64
'wake_flags' parameter vs. arm 32-bit registers. These point to existing
code handling arguments larger than register-size, allowing them to be
BTF encoded but only if structs.

Generalize the code for any argument type larger than register size (i.e.
size > cu->addr_size). This should work for integral or aggregate types,
and also avoids a bug in the current code where a register-sized struct
could be mistaken for larger. Note that zero-sized arguments will still
be marked as inconsistent and not encoded.

Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>
Tested-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
---
v2 -> v3:
 - Added Tested-by: from Alexis and Alan.
 - Revert support for encoding 0-sized structs (as v1) after discussion:
   https://lore.kernel.org/dwarves/9a41b21f-c0ae-4298-bf95-09d0cdc3f3ab@oracle.com/
 - Inline param__is_wide() and clarify some naming/wording.

v1 -> v2:
 - Update to preserve existing behaviour where zero-sized struct params
   still permit the function to be encoded, as noted by Alan.

---
 dwarf_loader.c | 37 ++++++++++++-------------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index e1ba7bc..134a76b 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2914,23 +2914,9 @@ out:
 	return 0;
 }
 
-static bool param__is_struct(struct cu *cu, struct tag *tag)
+static inline bool param__is_wide(struct cu *cu, struct tag *tag)
 {
-	struct tag *type = cu__type(cu, tag->type);
-
-	if (!type)
-		return false;
-
-	switch (type->tag) {
-	case DW_TAG_structure_type:
-		return true;
-	case DW_TAG_const_type:
-	case DW_TAG_typedef:
-		/* handle "typedef struct", const parameter */
-		return param__is_struct(cu, type);
-	default:
-		return false;
-	}
+	return tag__size(tag, cu) > cu->addr_size;
 }
 
 static int cu__resolve_func_ret_types_optimized(struct cu *cu)
@@ -2942,9 +2928,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		struct tag *tag = pt->entries[i];
 		struct parameter *pos;
 		struct function *fn = tag__function(tag);
-		bool has_unexpected_reg = false, has_struct_param = false;
+		bool has_unexpected_reg = false, has_wide_param = false;
 
-		/* mark function as optimized if parameter is, or
+		/* Mark function as optimized if parameter is, or
 		 * if parameter does not have a location; at this
 		 * point location presence has been marked in
 		 * abstract origins for cases where a parameter
@@ -2953,10 +2939,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		 *
 		 * Also mark functions which, due to optimization,
 		 * use an unexpected register for a parameter.
-		 * Exception is functions which have a struct
-		 * as a parameter, as multiple registers may
-		 * be used to represent it, throwing off register
-		 * to parameter mapping.
+		 * Exception is functions with a wide parameter,
+		 * as single register won't be used to represent
+		 * it, throwing off register to parameter mapping.
+		 * Examples include large structs or 64-bit types
+		 * on a 32-bit arch.
 		 */
 		ftype__for_each_parameter(&fn->proto, pos) {
 			if (pos->optimized || !pos->has_loc)
@@ -2967,11 +2954,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		}
 		if (has_unexpected_reg) {
 			ftype__for_each_parameter(&fn->proto, pos) {
-				has_struct_param = param__is_struct(cu, &pos->tag);
-				if (has_struct_param)
+				has_wide_param = param__is_wide(cu, &pos->tag);
+				if (has_wide_param)
 					break;
 			}
-			if (!has_struct_param)
+			if (!has_wide_param)
 				fn->proto.unexpected_reg = 1;
 		}
 
-- 
2.34.1

Re: [PATCH dwarves v3] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Alan Maguire]

On 22/05/2025 07:37, Tony Ambardar wrote:
> I encountered an issue building BTF kernels for 32-bit armhf, where many
> functions are missing in BTF data:
> 
>   LD      vmlinux
>   BTFIDS  vmlinux
> WARN: resolve_btfids: unresolved symbol vfs_truncate
> WARN: resolve_btfids: unresolved symbol vfs_fallocate
> WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl
> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node
> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu
> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node
> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu
> WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu
> WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime
> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq
> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch
> WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq
> WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq
> WARN: resolve_btfids: unresolved symbol scx_bpf_consume
> WARN: resolve_btfids: unresolved symbol bpf_throw
> WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp
> WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl
> WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl
> WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key
> WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key
> WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new
> WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new
> WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache
> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp
> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb
> WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id
>   NM      System.map
> 
> After further debugging this can be reproduced more simply:
> 
> $ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
> btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
> btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'
> 
> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> <nothing>
> 
> $ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> 
> $ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf
> 
> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> 
> The key things to note are the pahole 'consistent_func' feature and the u64
> 'wake_flags' parameter vs. arm 32-bit registers. These point to existing
> code handling arguments larger than register-size, allowing them to be
> BTF encoded but only if structs.
> 
> Generalize the code for any argument type larger than register size (i.e.
> size > cu->addr_size). This should work for integral or aggregate types,
> and also avoids a bug in the current code where a register-sized struct
> could be mistaken for larger. Note that zero-sized arguments will still
> be marked as inconsistent and not encoded.
> 
> Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
> Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> Tested-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>

hi Tony,

I'm planning on landing this shortly unless anyone objects; and on that
topic if anyone has the cycles to test with this patch that would be
great! I ran it through the work-in-progress BTF comparison in github CI
and all looks good; see the "Compare functions generated" step in [1].

Thanks!

Alan

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

> ---
> v2 -> v3:
>  - Added Tested-by: from Alexis and Alan.
>  - Revert support for encoding 0-sized structs (as v1) after discussion:
>    https://lore.kernel.org/dwarves/9a41b21f-c0ae-4298-bf95-09d0cdc3f3ab@oracle.com/
>  - Inline param__is_wide() and clarify some naming/wording.
> 
> v1 -> v2:
>  - Update to preserve existing behaviour where zero-sized struct params
>    still permit the function to be encoded, as noted by Alan.
> 
> ---
>  dwarf_loader.c | 37 ++++++++++++-------------------------
>  1 file changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index e1ba7bc..134a76b 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -2914,23 +2914,9 @@ out:
>  	return 0;
>  }
>  
> -static bool param__is_struct(struct cu *cu, struct tag *tag)
> +static inline bool param__is_wide(struct cu *cu, struct tag *tag)
>  {
> -	struct tag *type = cu__type(cu, tag->type);
> -
> -	if (!type)
> -		return false;
> -
> -	switch (type->tag) {
> -	case DW_TAG_structure_type:
> -		return true;
> -	case DW_TAG_const_type:
> -	case DW_TAG_typedef:
> -		/* handle "typedef struct", const parameter */
> -		return param__is_struct(cu, type);
> -	default:
> -		return false;
> -	}
> +	return tag__size(tag, cu) > cu->addr_size;
>  }
>  
>  static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> @@ -2942,9 +2928,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		struct tag *tag = pt->entries[i];
>  		struct parameter *pos;
>  		struct function *fn = tag__function(tag);
> -		bool has_unexpected_reg = false, has_struct_param = false;
> +		bool has_unexpected_reg = false, has_wide_param = false;
>  
> -		/* mark function as optimized if parameter is, or
> +		/* Mark function as optimized if parameter is, or
>  		 * if parameter does not have a location; at this
>  		 * point location presence has been marked in
>  		 * abstract origins for cases where a parameter
> @@ -2953,10 +2939,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		 *
>  		 * Also mark functions which, due to optimization,
>  		 * use an unexpected register for a parameter.
> -		 * Exception is functions which have a struct
> -		 * as a parameter, as multiple registers may
> -		 * be used to represent it, throwing off register
> -		 * to parameter mapping.
> +		 * Exception is functions with a wide parameter,
> +		 * as single register won't be used to represent
> +		 * it, throwing off register to parameter mapping.
> +		 * Examples include large structs or 64-bit types
> +		 * on a 32-bit arch.
>  		 */
>  		ftype__for_each_parameter(&fn->proto, pos) {
>  			if (pos->optimized || !pos->has_loc)
> @@ -2967,11 +2954,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>  		}
>  		if (has_unexpected_reg) {
>  			ftype__for_each_parameter(&fn->proto, pos) {
> -				has_struct_param = param__is_struct(cu, &pos->tag);
> -				if (has_struct_param)
> +				has_wide_param = param__is_wide(cu, &pos->tag);
> +				if (has_wide_param)
>  					break;
>  			}
> -			if (!has_struct_param)
> +			if (!has_wide_param)
>  				fn->proto.unexpected_reg = 1;
>  		}
>  

Re: [PATCH dwarves v3] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Alan Maguire]

On 24/06/2025 17:14, Alan Maguire wrote:
> On 22/05/2025 07:37, Tony Ambardar wrote:
>> I encountered an issue building BTF kernels for 32-bit armhf, where many
>> functions are missing in BTF data:
>>
>>   LD      vmlinux
>>   BTFIDS  vmlinux
>> WARN: resolve_btfids: unresolved symbol vfs_truncate
>> WARN: resolve_btfids: unresolved symbol vfs_fallocate
>> WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl
>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node
>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu
>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node
>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu
>> WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu
>> WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq
>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch
>> WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq
>> WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq
>> WARN: resolve_btfids: unresolved symbol scx_bpf_consume
>> WARN: resolve_btfids: unresolved symbol bpf_throw
>> WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp
>> WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl
>> WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl
>> WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key
>> WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key
>> WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new
>> WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new
>> WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache
>> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp
>> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb
>> WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id
>>   NM      System.map
>>
>> After further debugging this can be reproduced more simply:
>>
>> $ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
>> btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
>> btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'
>>
>> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>> <nothing>
>>
>> $ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>> s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
>>
>> $ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf
>>
>> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>> bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
>>
>> The key things to note are the pahole 'consistent_func' feature and the u64
>> 'wake_flags' parameter vs. arm 32-bit registers. These point to existing
>> code handling arguments larger than register-size, allowing them to be
>> BTF encoded but only if structs.
>>
>> Generalize the code for any argument type larger than register size (i.e.
>> size > cu->addr_size). This should work for integral or aggregate types,
>> and also avoids a bug in the current code where a register-sized struct
>> could be mistaken for larger. Note that zero-sized arguments will still
>> be marked as inconsistent and not encoded.
>>
>> Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
>> Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>
>> Tested-by: Alan Maguire <alan.maguire@oracle.com>
>> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> 
> hi Tony,
> 
> I'm planning on landing this shortly unless anyone objects; and on that
> topic if anyone has the cycles to test with this patch that would be
> great! I ran it through the work-in-progress BTF comparison in github CI
> and all looks good; see the "Compare functions generated" step in [1].
> 
> Thanks!
>

In fact I spoke too soon; there was a bug in the function comparison.
After that was fixed, I reran with this patch; see [1].

It shows that - as expected - functions with 0-sized params are left
out, specifically

< int __io_run_local_work(struct io_ring_ctx * ctx, io_tw_token_t tw,
int min_events, int max_events);
< int __io_run_local_work_loop(struct llist_node * * node, io_tw_token_t
tw, int events);

We expect this since io_tw_token_t is 0-sized. However on x86_64 it did
show one _extra_ function that I didn't expect:

> int __vxlan_fdb_delete(struct vxlan_dev * vxlan, const unsigned char
* addr, union vxlan_addr ip, __be16 port, __be32 src_vni, __be32 vni,
u32 ifindex, bool swdev_notify);

It's not clear to me why that function was added with this change - I
would have expected it either with or without the change. Any idea why
that might be?

[1]
https://github.com/alan-maguire/dwarves/actions/runs/15872520906/job/44752273776

> Alan
> 
> [1] https://github.com/alan-maguire/dwarves/actions/runs/15854137212
> 
>> ---
>> v2 -> v3:
>>  - Added Tested-by: from Alexis and Alan.
>>  - Revert support for encoding 0-sized structs (as v1) after discussion:
>>    https://lore.kernel.org/dwarves/9a41b21f-c0ae-4298-bf95-09d0cdc3f3ab@oracle.com/
>>  - Inline param__is_wide() and clarify some naming/wording.
>>
>> v1 -> v2:
>>  - Update to preserve existing behaviour where zero-sized struct params
>>    still permit the function to be encoded, as noted by Alan.
>>
>> ---
>>  dwarf_loader.c | 37 ++++++++++++-------------------------
>>  1 file changed, 12 insertions(+), 25 deletions(-)
>>
>> diff --git a/dwarf_loader.c b/dwarf_loader.c
>> index e1ba7bc..134a76b 100644
>> --- a/dwarf_loader.c
>> +++ b/dwarf_loader.c
>> @@ -2914,23 +2914,9 @@ out:
>>  	return 0;
>>  }
>>  
>> -static bool param__is_struct(struct cu *cu, struct tag *tag)
>> +static inline bool param__is_wide(struct cu *cu, struct tag *tag)
>>  {
>> -	struct tag *type = cu__type(cu, tag->type);
>> -
>> -	if (!type)
>> -		return false;
>> -
>> -	switch (type->tag) {
>> -	case DW_TAG_structure_type:
>> -		return true;
>> -	case DW_TAG_const_type:
>> -	case DW_TAG_typedef:
>> -		/* handle "typedef struct", const parameter */
>> -		return param__is_struct(cu, type);
>> -	default:
>> -		return false;
>> -	}
>> +	return tag__size(tag, cu) > cu->addr_size;
>>  }
>>  
>>  static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>> @@ -2942,9 +2928,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>>  		struct tag *tag = pt->entries[i];
>>  		struct parameter *pos;
>>  		struct function *fn = tag__function(tag);
>> -		bool has_unexpected_reg = false, has_struct_param = false;
>> +		bool has_unexpected_reg = false, has_wide_param = false;
>>  
>> -		/* mark function as optimized if parameter is, or
>> +		/* Mark function as optimized if parameter is, or
>>  		 * if parameter does not have a location; at this
>>  		 * point location presence has been marked in
>>  		 * abstract origins for cases where a parameter
>> @@ -2953,10 +2939,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>>  		 *
>>  		 * Also mark functions which, due to optimization,
>>  		 * use an unexpected register for a parameter.
>> -		 * Exception is functions which have a struct
>> -		 * as a parameter, as multiple registers may
>> -		 * be used to represent it, throwing off register
>> -		 * to parameter mapping.
>> +		 * Exception is functions with a wide parameter,
>> +		 * as single register won't be used to represent
>> +		 * it, throwing off register to parameter mapping.
>> +		 * Examples include large structs or 64-bit types
>> +		 * on a 32-bit arch.
>>  		 */
>>  		ftype__for_each_parameter(&fn->proto, pos) {
>>  			if (pos->optimized || !pos->has_loc)
>> @@ -2967,11 +2954,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>>  		}
>>  		if (has_unexpected_reg) {
>>  			ftype__for_each_parameter(&fn->proto, pos) {
>> -				has_struct_param = param__is_struct(cu, &pos->tag);
>> -				if (has_struct_param)
>> +				has_wide_param = param__is_wide(cu, &pos->tag);
>> +				if (has_wide_param)
>>  					break;
>>  			}
>> -			if (!has_struct_param)
>> +			if (!has_wide_param)
>>  				fn->proto.unexpected_reg = 1;
>>  		}
>>  
> 
> 

Re: [PATCH dwarves v3] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Jiri Olsa]

On Mon, Jun 30, 2025 at 11:01:19AM +0100, Alan Maguire wrote:
> On 24/06/2025 17:14, Alan Maguire wrote:
> > On 22/05/2025 07:37, Tony Ambardar wrote:
> >> I encountered an issue building BTF kernels for 32-bit armhf, where many
> >> functions are missing in BTF data:
> >>
> >>   LD      vmlinux
> >>   BTFIDS  vmlinux
> >> WARN: resolve_btfids: unresolved symbol vfs_truncate
> >> WARN: resolve_btfids: unresolved symbol vfs_fallocate
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq
> >> WARN: resolve_btfids: unresolved symbol scx_bpf_consume
> >> WARN: resolve_btfids: unresolved symbol bpf_throw
> >> WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp
> >> WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl
> >> WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl
> >> WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key
> >> WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key
> >> WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new
> >> WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new
> >> WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache
> >> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp
> >> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb
> >> WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id
> >>   NM      System.map
> >>
> >> After further debugging this can be reproduced more simply:
> >>
> >> $ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
> >> btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
> >> btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'
> >>
> >> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> >> <nothing>
> >>
> >> $ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> >> s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> >>
> >> $ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf
> >>
> >> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
> >> bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
> >>
> >> The key things to note are the pahole 'consistent_func' feature and the u64
> >> 'wake_flags' parameter vs. arm 32-bit registers. These point to existing
> >> code handling arguments larger than register-size, allowing them to be
> >> BTF encoded but only if structs.
> >>
> >> Generalize the code for any argument type larger than register size (i.e.
> >> size > cu->addr_size). This should work for integral or aggregate types,
> >> and also avoids a bug in the current code where a register-sized struct
> >> could be mistaken for larger. Note that zero-sized arguments will still
> >> be marked as inconsistent and not encoded.
> >>
> >> Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
> >> Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> >> Tested-by: Alan Maguire <alan.maguire@oracle.com>
> >> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> > 
> > hi Tony,
> > 
> > I'm planning on landing this shortly unless anyone objects; and on that
> > topic if anyone has the cycles to test with this patch that would be
> > great! I ran it through the work-in-progress BTF comparison in github CI
> > and all looks good; see the "Compare functions generated" step in [1].
> > 
> > Thanks!
> >
> 
> In fact I spoke too soon; there was a bug in the function comparison.
> After that was fixed, I reran with this patch; see [1].
> 
> It shows that - as expected - functions with 0-sized params are left
> out, specifically
> 
> < int __io_run_local_work(struct io_ring_ctx * ctx, io_tw_token_t tw,
> int min_events, int max_events);
> < int __io_run_local_work_loop(struct llist_node * * node, io_tw_token_t
> tw, int events);
> 
> We expect this since io_tw_token_t is 0-sized. However on x86_64 it did
> show one _extra_ function that I didn't expect:
> 
> > int __vxlan_fdb_delete(struct vxlan_dev * vxlan, const unsigned char
> * addr, union vxlan_addr ip, __be16 port, __be32 src_vni, __be32 vni,
> u32 ifindex, bool swdev_notify);
> 
> It's not clear to me why that function was added with this change - I
> would have expected it either with or without the change. Any idea why
> that might be?

hi,
I can see that as well, IIUC the 'ip' argument is:

union vxlan_addr {
        struct sockaddr_in sin;
        struct sockaddr_in6 sin6;
        struct sockaddr sa;
};

so we have struct as 4th argument, which sets the has_wide_param condition
and won't set the fn->proto.unexpected_reg for the function, because of:

   if (!has_wide_param)
      fn->proto.unexpected_reg = 1;

I'm not sure it's correct.. if the ip struct is big enough that it's passed
on stack, why are the rest of the arguments marked with unexpected_reg
(in parameter__new) I think I'm missing something

jirka


> 
> [1]
> https://github.com/alan-maguire/dwarves/actions/runs/15872520906/job/44752273776
> 
> > Alan
> > 
> > [1] https://github.com/alan-maguire/dwarves/actions/runs/15854137212
> > 
> >> ---
> >> v2 -> v3:
> >>  - Added Tested-by: from Alexis and Alan.
> >>  - Revert support for encoding 0-sized structs (as v1) after discussion:
> >>    https://lore.kernel.org/dwarves/9a41b21f-c0ae-4298-bf95-09d0cdc3f3ab@oracle.com/
> >>  - Inline param__is_wide() and clarify some naming/wording.
> >>
> >> v1 -> v2:
> >>  - Update to preserve existing behaviour where zero-sized struct params
> >>    still permit the function to be encoded, as noted by Alan.
> >>
> >> ---
> >>  dwarf_loader.c | 37 ++++++++++++-------------------------
> >>  1 file changed, 12 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/dwarf_loader.c b/dwarf_loader.c
> >> index e1ba7bc..134a76b 100644
> >> --- a/dwarf_loader.c
> >> +++ b/dwarf_loader.c
> >> @@ -2914,23 +2914,9 @@ out:
> >>  	return 0;
> >>  }
> >>  
> >> -static bool param__is_struct(struct cu *cu, struct tag *tag)
> >> +static inline bool param__is_wide(struct cu *cu, struct tag *tag)
> >>  {
> >> -	struct tag *type = cu__type(cu, tag->type);
> >> -
> >> -	if (!type)
> >> -		return false;
> >> -
> >> -	switch (type->tag) {
> >> -	case DW_TAG_structure_type:
> >> -		return true;
> >> -	case DW_TAG_const_type:
> >> -	case DW_TAG_typedef:
> >> -		/* handle "typedef struct", const parameter */
> >> -		return param__is_struct(cu, type);
> >> -	default:
> >> -		return false;
> >> -	}
> >> +	return tag__size(tag, cu) > cu->addr_size;
> >>  }
> >>  
> >>  static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> >> @@ -2942,9 +2928,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> >>  		struct tag *tag = pt->entries[i];
> >>  		struct parameter *pos;
> >>  		struct function *fn = tag__function(tag);
> >> -		bool has_unexpected_reg = false, has_struct_param = false;
> >> +		bool has_unexpected_reg = false, has_wide_param = false;
> >>  
> >> -		/* mark function as optimized if parameter is, or
> >> +		/* Mark function as optimized if parameter is, or
> >>  		 * if parameter does not have a location; at this
> >>  		 * point location presence has been marked in
> >>  		 * abstract origins for cases where a parameter
> >> @@ -2953,10 +2939,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> >>  		 *
> >>  		 * Also mark functions which, due to optimization,
> >>  		 * use an unexpected register for a parameter.
> >> -		 * Exception is functions which have a struct
> >> -		 * as a parameter, as multiple registers may
> >> -		 * be used to represent it, throwing off register
> >> -		 * to parameter mapping.
> >> +		 * Exception is functions with a wide parameter,
> >> +		 * as single register won't be used to represent
> >> +		 * it, throwing off register to parameter mapping.
> >> +		 * Examples include large structs or 64-bit types
> >> +		 * on a 32-bit arch.
> >>  		 */
> >>  		ftype__for_each_parameter(&fn->proto, pos) {
> >>  			if (pos->optimized || !pos->has_loc)
> >> @@ -2967,11 +2954,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> >>  		}
> >>  		if (has_unexpected_reg) {
> >>  			ftype__for_each_parameter(&fn->proto, pos) {
> >> -				has_struct_param = param__is_struct(cu, &pos->tag);
> >> -				if (has_struct_param)
> >> +				has_wide_param = param__is_wide(cu, &pos->tag);
> >> +				if (has_wide_param)
> >>  					break;
> >>  			}
> >> -			if (!has_struct_param)
> >> +			if (!has_wide_param)
> >>  				fn->proto.unexpected_reg = 1;
> >>  		}
> >>  
> > 
> > 
> 
> 

Re: [PATCH dwarves v3] dwarf_loader: Fix skipped encoding of function BTF on 32-bit systems

[Alan Maguire]

On 30/06/2025 14:51, Jiri Olsa wrote:
> On Mon, Jun 30, 2025 at 11:01:19AM +0100, Alan Maguire wrote:
>> On 24/06/2025 17:14, Alan Maguire wrote:
>>> On 22/05/2025 07:37, Tony Ambardar wrote:
>>>> I encountered an issue building BTF kernels for 32-bit armhf, where many
>>>> functions are missing in BTF data:
>>>>
>>>>   LD      vmlinux
>>>>   BTFIDS  vmlinux
>>>> WARN: resolve_btfids: unresolved symbol vfs_truncate
>>>> WARN: resolve_btfids: unresolved symbol vfs_fallocate
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq
>>>> WARN: resolve_btfids: unresolved symbol scx_bpf_consume
>>>> WARN: resolve_btfids: unresolved symbol bpf_throw
>>>> WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp
>>>> WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl
>>>> WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl
>>>> WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key
>>>> WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key
>>>> WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new
>>>> WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new
>>>> WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache
>>>> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp
>>>> WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb
>>>> WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id
>>>>   NM      System.map
>>>>
>>>> After further debugging this can be reproduced more simply:
>>>>
>>>> $ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf
>>>> btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF
>>>> btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl'
>>>>
>>>> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>>>> <nothing>
>>>>
>>>> $ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>>>> s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
>>>>
>>>> $ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf
>>>>
>>>> $ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf
>>>> bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle);
>>>>
>>>> The key things to note are the pahole 'consistent_func' feature and the u64
>>>> 'wake_flags' parameter vs. arm 32-bit registers. These point to existing
>>>> code handling arguments larger than register-size, allowing them to be
>>>> BTF encoded but only if structs.
>>>>
>>>> Generalize the code for any argument type larger than register size (i.e.
>>>> size > cu->addr_size). This should work for integral or aggregate types,
>>>> and also avoids a bug in the current code where a register-sized struct
>>>> could be mistaken for larger. Note that zero-sized arguments will still
>>>> be marked as inconsistent and not encoded.
>>>>
>>>> Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params")
>>>> Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>
>>>> Tested-by: Alan Maguire <alan.maguire@oracle.com>
>>>> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
>>>
>>> hi Tony,
>>>
>>> I'm planning on landing this shortly unless anyone objects; and on that
>>> topic if anyone has the cycles to test with this patch that would be
>>> great! I ran it through the work-in-progress BTF comparison in github CI
>>> and all looks good; see the "Compare functions generated" step in [1].
>>>
>>> Thanks!
>>>
>>
>> In fact I spoke too soon; there was a bug in the function comparison.
>> After that was fixed, I reran with this patch; see [1].
>>
>> It shows that - as expected - functions with 0-sized params are left
>> out, specifically
>>
>> < int __io_run_local_work(struct io_ring_ctx * ctx, io_tw_token_t tw,
>> int min_events, int max_events);
>> < int __io_run_local_work_loop(struct llist_node * * node, io_tw_token_t
>> tw, int events);
>>
>> We expect this since io_tw_token_t is 0-sized. However on x86_64 it did
>> show one _extra_ function that I didn't expect:
>>
>>> int __vxlan_fdb_delete(struct vxlan_dev * vxlan, const unsigned char
>> * addr, union vxlan_addr ip, __be16 port, __be32 src_vni, __be32 vni,
>> u32 ifindex, bool swdev_notify);
>>
>> It's not clear to me why that function was added with this change - I
>> would have expected it either with or without the change. Any idea why
>> that might be?
> 
> hi,
> I can see that as well, IIUC the 'ip' argument is:
> 
> union vxlan_addr {
>         struct sockaddr_in sin;
>         struct sockaddr_in6 sin6;
>         struct sockaddr sa;
> };
> 
> so we have struct as 4th argument, which sets the has_wide_param condition
> and won't set the fn->proto.unexpected_reg for the function, because of:
> 
>    if (!has_wide_param)
>       fn->proto.unexpected_reg = 1;
> 
> I'm not sure it's correct.. if the ip struct is big enough that it's passed
> on stack, why are the rest of the arguments marked with unexpected_reg
> (in parameter__new) I think I'm missing something
>

Ah you've nailed it, that's the reason! Before we only checked for
struct/typedef or const struct/typedef. However in this case it is a
union, so moving to a size-based rather than an (incomplete) type-based
tests means we previously excluded this function but don't now.

Alan