From: Matt Bobrowski <mattbobrowski@google.com>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: yonghong.song@linux.dev, ihor.solodrai@linux.dev,
eddyz87@gmail.com, jolsa@kernel.org, andrii@kernel.org,
ast@kernel.org, david.faust@oracle.com, dwarves@vger.kernel.org,
bpf@vger.kernel.org
Subject: Re: [PATCH v2 dwarves 2/5] btf_encoder: Add true_signature feature support for "."-suffixed functions
Date: Mon, 26 Jan 2026 09:52:29 +0000 [thread overview]
Message-ID: <aXc5XXZvgJAHMf6v@google.com> (raw)
In-Reply-To: <20260123172650.4062362-3-alan.maguire@oracle.com>
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 <alan.maguire@oracle.com>
Some minor nits, but apart from that it looks OK to me.
Acked-by: Matt Bobrowski <mattbobrowski@google.com>
> ---
> 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
>
next prev parent reply other threads:[~2026-01-26 9:52 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-23 17:26 [PATCH v2 dwarves 0/5] Improve BTF concrete function accuracy Alan Maguire
2026-01-23 17:26 ` [PATCH v2 dwarves 1/5] dwarf_loader/btf_encoder: Detect reordered parameters Alan Maguire
2026-01-26 9:30 ` Matt Bobrowski
2026-01-23 17:26 ` [PATCH v2 dwarves 2/5] btf_encoder: Add true_signature feature support for "."-suffixed functions Alan Maguire
2026-01-23 19:05 ` Yonghong Song
2026-01-26 9:52 ` Matt Bobrowski [this message]
2026-01-26 10:49 ` Alan Maguire
2026-01-26 11:18 ` Matt Bobrowski
2026-01-27 12:18 ` Alan Maguire
2026-01-23 17:26 ` [PATCH v2 dwarves 3/5] test: add gcc true signature test Alan Maguire
2026-01-23 19:06 ` Yonghong Song
2026-01-23 17:26 ` [PATCH v2 dwarves 4/5] man-pages: document true_signature btf_feature Alan Maguire
2026-01-23 19:07 ` Yonghong Song
2026-01-26 10:02 ` Matt Bobrowski
2026-01-26 10:51 ` Alan Maguire
2026-01-26 11:21 ` Matt Bobrowski
2026-01-26 17:10 ` Alan Maguire
2026-01-26 18:13 ` Yonghong Song
2026-01-23 17:26 ` [PATCH v2 dwarves 5/5] btf_encoder: Prefer strong function definitions for BTF generation Alan Maguire
2026-01-23 19:09 ` Yonghong Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aXc5XXZvgJAHMf6v@google.com \
--to=mattbobrowski@google.com \
--cc=alan.maguire@oracle.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=david.faust@oracle.com \
--cc=dwarves@vger.kernel.org \
--cc=eddyz87@gmail.com \
--cc=ihor.solodrai@linux.dev \
--cc=jolsa@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox