Re: [PATCH dwarves 2/9] dwarf_loader: Handle signatures with dead arguments

On 05/03/2026 22:55, Yonghong Song wrote:
> For llvm dwarf, the dead argument may be in the middle of
> DW_TAG_subprogram. So we introduce skip_idx in order to
> match expected registers properly.
> 
> For example:
>   0x00042897:   DW_TAG_subprogram
>                   DW_AT_name      ("create_dev")
>                   DW_AT_calling_convention        (DW_CC_nocall)
>                   DW_AT_type      (0x0002429a "int")
>                   ...
> 
>   0x000428ab:     DW_TAG_formal_parameter
>                     DW_AT_name    ("name")
>                     DW_AT_type    (0x000242ed "char *")
>                     ...
> 
>   0x000428b5:     DW_TAG_formal_parameter
>                     DW_AT_location        (indexed (0x3f) loclist = 0x000027f8:
>                        [0xffffffff87681370, 0xffffffff8768137a): DW_OP_reg5 RDI
>                        [0xffffffff8768137a, 0xffffffff87681392): DW_OP_reg3 RBX
>                        [0xffffffff87681392, 0xffffffff876813ae): DW_OP_entry_value(DW_OP_reg5 RDI), DW_OP_stack_value)
>                     DW_AT_name    ("dev")
>                     DW_AT_type    (0x00026859 "dev_t")
>                     ...
> 
> With skip_idx, we can identify that the second original argument
> 'dev' becomes the first one after optimization.
> 
> Signed-off-by: Yonghong Song <[email protected]>
> ---
>  dwarf_loader.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 610b69e..1ced5e2 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -1192,6 +1192,7 @@ static ptrdiff_t __dwarf_getlocations(Dwarf_Attribute *attr,
>  
>  struct func_info {
>  	bool signature_changed;
> +	int skip_idx;
>  };
>  
>  /* For DW_AT_location 'attr':
> @@ -1264,13 +1265,28 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>  	if (parm != NULL) {
>  		bool has_const_value;
>  		Dwarf_Attribute attr;
> +		int reg_idx;
>  
>  		tag__init(&parm->tag, cu, die);
>  		parm->name = attr_string(die, DW_AT_name, conf);
>  		parm->idx = param_idx;
> -		if (param_idx >= cu->nr_register_params || param_idx < 0)
> +		if (param_idx < 0)
>  			return parm;
> -		if (cu->producer_clang && !info->signature_changed)
> +		if (!cu->producer_clang && param_idx >= cu->nr_register_params)
> +			return parm;
> +		if (cu->producer_clang) {
> +			if (!info->signature_changed)
> +				return parm;
> +			/* if true_signature is not enabled, mark parameter as
> +			 * unexpected_reg since there is a skipped parameter before.
> +			 */
> +			if (!conf->true_signature && info->skip_idx) {
> +				parm->unexpected_reg = 1;
> +				return parm;
> +			}
> +		}
> +		reg_idx = param_idx - info->skip_idx;
> +		if (reg_idx >= cu->nr_register_params)
>  			return parm;
>  		/* Parameters which use DW_AT_abstract_origin to point at
>  		 * the original parameter definition (with no name in the DIE)
> @@ -1309,7 +1325,7 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>  		parm->has_loc = dwarf_attr(die, DW_AT_location, &attr) != NULL;
>  
>  		if (parm->has_loc) {
> -			int expected_reg = cu->register_params[param_idx];
> +			int expected_reg = cu->register_params[reg_idx];
>  			int actual_reg = parameter__reg(&attr, expected_reg);
>  
>  			if (actual_reg < 0)
> @@ -1322,8 +1338,11 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>  				 * contents.
>  				 */
>  				parm->unexpected_reg = 1;
> -		} else if (has_const_value) {
> +		} else if (!cu->producer_clang && has_const_value) {
> +			parm->optimized = 1;
> +		} else if (cu->producer_clang) {
>  			parm->optimized = 1;
> +			info->skip_idx++;
>  		}
>  	}
>  

In [1]  (dwarf_loader/btf_encoder: Detect reordered parameters)

we detect reordered/missing parameters for gcc by comparing abstract origin to concrete representation
and mark any such functions by setting the reordered_parm bitfield in the encoder func state. In this
approach reordered encompasses both missing and actual reordering; in practice it's always the former, but
the DWARF representation was confusing because it had the actually-used parameters (with location info)
followed by the unused ones (without location info). Before the above commit we were treating these
function signatures incorrectly as valid leading to wrong functions signatures.

All of this is to say: should we mark with reordered_parm instead? The reason I suggest this is that
btf_encoder__save_func() has handling for skipping functions without locations with reordered_parm set;
maybe we could find a way to unify that across clang/gcc cases?

Now in the clang case, I guess reordered is a poor description; for gcc it really means "reordered as
compared to abstract origin". If we could find a better way to describe/encompass both cases that would
be ideal I think. The terminology used in the gcc true signature code isn't great today.

[1] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/commit/?id=109e5e4554f663e5f12fb634bc54cceb710aec61