Re: [PATCH dwarves] btf_encoder: prefer strong function definitions for BTF generation

[Matt Bobrowski <mattbobrowski@google.com>]

On Fri, Jan 02, 2026 at 10:46:00AM -0800, Yonghong Song wrote:
> 
> 
> On 12/31/25 12:53 AM, Matt Bobrowski wrote:
> > Currently, when a function has both a weak and a strong definition
> > across different compilation units (CUs), the BTF encoder arbitrarily
> > selects one to generate the BTF entry. This selection fundamentally is
> > dependent on the order in which pahole processes the CUs.
> > 
> > This indifference often leads to a mismatch where the generated BTF
> > reflects the weak definition's prototype, even though the linker
> > selected the strong definition for the final vmlinux binary.
> > 
> > A notable example described in [0] involving function
> > bpf_lsm_mmap_file(). Both weak and strong definitions exist,
> > distinguished only by parameter names (e.g., file vs
> > file__nullable). While the strong definition is linked into the
> > vmlinux object, the generated BTF contained the prototype for the weak
> > definition. This causes issues for BPF verifier (e.g., __nullable
> > annotation semantics), or tools relying on accurate type information.
> > 
> > To fix this, ensure the BTF encoder selects the function definition
> > corresponding to the actual code linked into the binary. This is
> > achieved by comparing the DWARF function address (DW_AT_low_pc) with
> > the ELF symbol address (st_value). Only the DWARF entry for the strong
> > definition will match the final resolved ELF symbol address.
> > 
> > [0] https://lore.kernel.org/all/aVJY9H-e83T7ivT4@google.com/
> > 
> > Link: https://lore.kernel.org/all/aVJY9H-e83T7ivT4@google.com/
> > Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> 
> LGTM with some nits below.

Thanks for the review.

> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> 
> > ---
> >   btf_encoder.c | 36 ++++++++++++++++++++++++++++++++++++
> >   1 file changed, 36 insertions(+)
> > 
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index b37ee7f..0462094 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -79,6 +79,7 @@ struct btf_encoder_func_annot {
> >   /* state used to do later encoding of saved functions */
> >   struct btf_encoder_func_state {
> > +	uint64_t addr;
> >   	struct elf_function *elf;
> >   	uint32_t type_id_off;
> >   	uint16_t nr_parms;
> > @@ -1258,6 +1259,7 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
> >   	if (!state)
> >   		return -ENOMEM;
> > +	state->addr = function__addr(fn);
> >   	state->elf = func;
> >   	state->nr_parms = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
> >   	state->ret_type_id = ftype->tag.type == 0 ? 0 : encoder->type_id_off + ftype->tag.type;
> > @@ -1477,6 +1479,29 @@ static void btf_encoder__delete_saved_funcs(struct btf_encoder *encoder)
> >   	encoder->func_states.cap = 0;
> >   }
> > +static struct btf_encoder_func_state *btf_encoder__select_canonical_state(struct btf_encoder_func_state *combined_states,
> > +									  int combined_cnt)
> > +{
> > +	int i, j;
> > +
> > +	/*
> > +	 * The same elf_function is shared amongst combined functions,
> > +	 * as per saved_functions_combine().
> > +	 */
> > +	struct elf_function *elf = combined_states[0].elf;
> 
> The logic is okay. But can we limit elf->sym_cnt to be 1 here?
> This will match the case where two functions (weak and strong)
> co-exist in compiler and eventually only strong/global function
> will survive.

In fact, checking again I believe that the loop is redundant because
elf_function__has_ambiguous_address() ensures that if we reach this
point, all symbols for the function share the same address. Therefore,
checking the first symbol (elf->syms[0]) should be sufficient and
equivalent to checking all of them.

Will send through a v2 with this amendment.

> > +
> > +	for (i = 0; i < combined_cnt; i++) {
> > +		struct btf_encoder_func_state *state = &combined_states[i];
> > +
> > +		for (j = 0; j < elf->sym_cnt; j++) {
> > +			if (state->addr == elf->syms[j].addr)
> > +				return state;
> > +		}
> > +	}
> > +
> > +	return &combined_states[0];
> > +}
> > +
> >   static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_encoding_inconsistent_proto)
> >   {
> >   	struct btf_encoder_func_state *saved_fns = encoder->func_states.array;
> > @@ -1517,6 +1542,17 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
> >   					0, 0);
> >   		if (add_to_btf) {
> > +			/*
> > +			 * We're to add the current function within
> > +			 * BTF. Although, from all functions that have
> > +			 * possibly been combined via
> > +			 * saved_functions_combine(), ensure to only
> > +			 * select and emit BTF for the most canonical
> > +			 * function definition.
> > +			 */
> > +			if (j - i > 1)
> > +				state = btf_encoder__select_canonical_state(state, j - i);
> > +
> >   			if (is_kfunc_state(state))
> >   				err = btf_encoder__add_bpf_kfunc(encoder, state);
> >   			else
>