Re: [PATCH dwarves v5 10/11] btf_encoder: Handle optimized parameter properly
Alan Maguire <[email protected]> Thu, 11 Jun 2026 10:08:01 +0100
| Newsgroups | org.kernel.vger.dwarves,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 23/05/2026 17:58, Yonghong Song wrote:
> Ensure to skip optimized parameter so btf can generate
> proper true signatures.
>
> In the first patch of the patch set, with DW_CC_nocall filtering, 875 functions
> have signature changed. With a series of improvement, eventually only 18 functions
> remain and unfortunately these functions cannot be converted to true signatures
> due to locations. For example,
>
> 0x0242f1f7: DW_TAG_subprogram
> DW_AT_name ("memblock_find_in_range")
> DW_AT_calling_convention (DW_CC_nocall)
> DW_AT_type (0x0242decc "phys_addr_t")
> ...
>
> 0x0242f22e: DW_TAG_formal_parameter
> DW_AT_location (indexed (0x14a) loclist = 0x005595bc:
> [0xffffffff87a000f9, 0xffffffff87a00178): DW_OP_reg5 RDI
> [0xffffffff87a00178, 0xffffffff87a001be): DW_OP_reg14 R14
> [0xffffffff87a001be, 0xffffffff87a001c7): DW_OP_entry_value(DW_OP_reg5 RDI), DW_OP_stack_value
> [0xffffffff87a001c7, 0xffffffff87a00214): DW_OP_reg14 R14)
> DW_AT_name ("start")
> DW_AT_type (0x0242decc "phys_addr_t")
> ...
>
> 0x0242f239: DW_TAG_formal_parameter
> DW_AT_location (indexed (0x14b) loclist = 0x005595e6:
> [0xffffffff87a000f9, 0xffffffff87a00175): DW_OP_reg4 RSI
> [0xffffffff87a00175, 0xffffffff87a001b8): DW_OP_reg3 RBX
> [0xffffffff87a001b8, 0xffffffff87a001c7): DW_OP_entry_value(DW_OP_reg4 RSI), DW_OP_stack_value
> [0xffffffff87a001c7, 0xffffffff87a00214): DW_OP_reg3 RBX)
> DW_AT_name ("end")
> DW_AT_type (0x0242decc "phys_addr_t")
> ...
>
> 0x0242f245: DW_TAG_formal_parameter
> DW_AT_location (indexed (0x14c) loclist = 0x00559610:
> [0xffffffff87a001e3, 0xffffffff87a001ef): DW_OP_breg4 RSI+0)
> DW_AT_name ("size")
> DW_AT_type (0x0242decc "phys_addr_t")
> ...
>
> 0x0242f250: DW_TAG_formal_parameter
> DW_AT_const_value (4096)
> DW_AT_name ("align")
> DW_AT_type (0x0242decc "phys_addr_t")
> ...
>
> The third parameter 'size' is not from RDX. Hence, true signature is not possible for this function.
>
> I also did some experiments on arm64. The number of signature-changed funcitons
> is 863 and finally there are 70 functions cannot be converted to true signatures.
> Through dwarf comparison of x86_64 vs. arm64, llvm arm64 backend looks like having
> more relaxation to compute parameter values for those signature-changed functions.
>
> Signed-off-by: Yonghong Song <[email protected]>
> ---
> btf_encoder.c | 32 +++++++++++++++++++++++++++++---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 633bc61..26be31d 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -1257,15 +1257,21 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
> struct btf *btf = encoder->btf;
> struct llvm_annotation *annot;
> struct parameter *param;
> - uint8_t param_idx = 0;
> + uint8_t param_idx = 0, skip_idx = 0;
> int str_off, err = 0;
>
> if (!state)
> return -ENOMEM;
>
> + if (encoder->true_signature && encoder->cu->producer_clang) {
> + ftype__for_each_parameter(ftype, param) {
> + if (param->optimized) skip_idx++;
> + }
> + }
> +
the logic here is a bit confusing (to me at least). Later on in the loop below
we subtract out param->optimized parameters from the state->nr_parms count for
the ftype->reordered_parm case. Why not just do the same for the clang optimized
out case below, i.e.
> state->addr = function__addr(fn);
> state->elf = func;
> - state->nr_parms = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
> + state->nr_parms = ftype->nr_parms - skip_idx + (ftype->unspec_parms ? 1 : 0);
> state->ret_type_id = ftype->tag.type == 0 ? 0 : encoder->type_id_off + ftype->tag.type;
> if (state->nr_parms > 0) {
> state->parms = zalloc(state->nr_parms * sizeof(*state->parms));
> @@ -1297,14 +1303,34 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
> state->reordered_parm = ftype->reordered_parm;
> ftype__for_each_parameter(ftype, param) {
> const char *name;
> + char *final_name = NULL;
>
> /* 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) ?: "";
> + if (encoder->true_signature && encoder->cu->producer_clang && param->optimized)
just add a "state->nr_parms--;" here and get rid of skip_idx.
> + continue;
> +
> + name = parameter__name(param);
> + if (!name) {
> + name = "";
do we see more parameter DIEs without parameter names for the nocall cases?
> + } else if (param->true_sig_member_name) {
> + /* Non-null param->true_sig_member_name indicates that the parameter
> + * name is <parameter_name>__<field_name>.
> + */
> + if (asprintf(&final_name, "%s__%s", name, param->true_sig_member_name) == -1) {
> + err = -ENOMEM;
> + goto out;
> + }
> + name = final_name;
> + }
> +
> str_off = btf__add_str(btf, name);
> + if (final_name)
> + free(final_name);
> +
> if (str_off < 0) {
> err = str_off;
> goto out;