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;