Re: [PATCH v2 dwarves 2/5] btf_encoder: Add true_signature feature support for "."-suffixed functions
Matt Bobrowski <[email protected]>
| Newsgroups | org.kernel.vger.dwarves,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On Fri, Jan 23, 2026 at 05:26:47PM +0000, Alan Maguire wrote: > Currently we collate function information by name and add functions > provided there are no inconsistencies across various representations. > > For true_signature support - where we wish to add the real signature > of a function even if it differs from source level - we need to do > a few things: > > 1. For "."-suffixed functions, we need to match from DWARF->ELF; > we can do this via the address associated with the function. > In doing this, we can then be confident that the debug info > for foo.isra.0 is the right info for the function at that > address. > > 2. When adding saved functions we need to look for such cases > and provided they do not violate other constraints around BTF > representation - unexpected reg usage for function, uncertain > parameter location or ambiguous address - we add them with > their "."-suffixed name. The latter can be used as a signal > that the function is transformed from the original. > > Doing this adds 500 functions to BTF. These are traceable with > their "."-suffix names and because we have excluded ambiguous > address cases we know exactly which function address they refer > to. > > Signed-off-by: Alan Maguire <[email protected]> Some minor nits, but apart from that it looks OK to me. Acked-by: Matt Bobrowski <[email protected]> > --- > btf_encoder.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++----- > dwarves.h | 1 + > pahole.c | 1 + > 3 files changed, 71 insertions(+), 7 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index 9a567e4..c1002c3 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -77,9 +77,16 @@ struct btf_encoder_func_annot { > int16_t component_idx; > }; > > +struct elf_function_sym { > + const char *name; > + uint64_t addr; > +}; > + > /* state used to do later encoding of saved functions */ > struct btf_encoder_func_state { > struct elf_function *elf; > + struct elf_function_sym *sym; > + uint64_t addr; ^ This appears to have leaked into wrong commit? This member should've been introduced within patch 5/5. > uint32_t type_id_off; > uint16_t nr_parms; > uint16_t nr_annots; > @@ -94,11 +101,6 @@ struct btf_encoder_func_state { > struct btf_encoder_func_annot *annots; > }; > > -struct elf_function_sym { > - const char *name; > - uint64_t addr; > -}; > - > struct elf_function { > char *name; > struct elf_function_sym *syms; > @@ -145,7 +147,8 @@ struct btf_encoder { > skip_encoding_decl_tag, > tag_kfuncs, > gen_distilled_base, > - encode_attributes; > + encode_attributes, > + true_signature; > uint32_t array_index_id; > struct elf_secinfo *secinfo; > size_t seccnt; > @@ -1270,14 +1273,36 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi > goto out; > } > } > + if (encoder->true_signature && fn->lexblock.ip.addr) { > + int i; > + > + for (i = 0; i < func->sym_cnt; i++) { > + if (fn->lexblock.ip.addr != func->syms[i].addr) > + continue; > + /* Only need to record address for '.'-suffixed > + * functions, since we only currently need true > + * signatures for them. > + */ > + if (!strchr(func->syms[i].name, '.')) > + continue; > + state->sym = &func->syms[i]; > + break; > + } > + } > 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; > state->reordered_parm = ftype->reordered_parm; > ftype__for_each_parameter(ftype, param) { > - const char *name = parameter__name(param) ?: ""; > + const char *name; > > + /* No location info/optimized + reordered means optimized out. */ > + if (ftype->reordered_parm && (!param->has_loc || param->optimized)) { > + state->nr_parms--; > + continue; > + } > + name = parameter__name(param) ?: ""; > str_off = btf__add_str(btf, name); > if (str_off < 0) { > err = str_off; > @@ -1366,6 +1391,9 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder, > > btf_fnproto_id = btf_encoder__add_func_proto_for_state(encoder, state); > name = func->name; > + if (encoder->true_signature && state->sym) > + name = state->sym->name; > + > if (btf_fnproto_id >= 0) > btf_fn_id = btf_encoder__add_ref_type(encoder, BTF_KIND_FUNC, btf_fnproto_id, > name, false); > @@ -1508,6 +1536,39 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e > while (j < nr_saved_fns && saved_functions_combine(encoder, &saved_fns[i], &saved_fns[j]) == 0) > j++; > > + /* Add true signatures for case where we have an exact > + * symbol match by address from DWARF->ELF and have a > + * "." suffixed name. > + */ > + if (encoder->true_signature) { > + bool true_added = false; > + int k; > + > + for (k = i; k < nr_saved_fns; k++) { > + struct btf_encoder_func_state *true_state = &saved_fns[k]; > + > + if (state->elf != true_state->elf) > + break; > + if (!true_state->sym) > + continue; > + /* Unexpected reg, uncertain parm loc and > + * ambiguous address mean we cannot trust fentry. > + */ > + if (true_state->unexpected_reg || > + true_state->uncertain_parm_loc || > + true_state->ambiguous_addr) > + continue; > + err = btf_encoder__add_func(encoder, true_state); > + if (err < 0) > + goto out; > + true_added = true; > + break; > + } > + /* If true symbol was added, skip the below. */ > + if (true_added) > + continue; > + } > + I think this hunk should be factored out into a helper. btf_encoder__add_saved_funcs() is starting to get hairy IMO. > /* 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 > @@ -2584,6 +2645,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam > encoder->tag_kfuncs = conf_load->btf_decl_tag_kfuncs; > encoder->gen_distilled_base = conf_load->btf_gen_distilled_base; > encoder->encode_attributes = conf_load->btf_attributes; > + encoder->true_signature = conf_load->true_signature; > encoder->verbose = verbose; > encoder->has_index_type = false; > encoder->need_index_type = false; > diff --git a/dwarves.h b/dwarves.h > index 78bedf5..d7c6474 100644 > --- a/dwarves.h > +++ b/dwarves.h > @@ -101,6 +101,7 @@ struct conf_load { > bool btf_decl_tag_kfuncs; > bool btf_gen_distilled_base; > bool btf_attributes; > + bool true_signature; > uint8_t hashtable_bits; > uint8_t max_hashtable_bits; > uint16_t kabi_prefix_len; > diff --git a/pahole.c b/pahole.c > index ef01e58..02a0d19 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -1234,6 +1234,7 @@ struct btf_feature { > BTF_NON_DEFAULT_FEATURE(global_var, encode_btf_global_vars, false), > BTF_NON_DEFAULT_FEATURE_CHECK(attributes, btf_attributes, false, > attributes_check), > + BTF_NON_DEFAULT_FEATURE(true_signature, true_signature, false), > }; > > #define BTF_MAX_FEATURE_STR 1024 > -- > 2.43.5 >