BPF List
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Alan Maguire <alan.maguire@oracle.com>,
	Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
	dwarves@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH dwarves v8 3/5] dwarf_loader: Analyze per-parameter information for true signatures
Date: Tue, 23 Jun 2026 18:21:16 -0700	[thread overview]
Message-ID: <7e2960f3-cad7-46a5-a691-29228fb696c6@linux.dev> (raw)
In-Reply-To: <20260623222906.3293063-1-yonghong.song@linux.dev>



On 6/23/26 3:29 PM, Yonghong Song wrote:
> Add a function-level pass, function__analyze_parameter_locations(), run
> from cu__resolve_func_ret_types_optimized() which walks a function's
> parameters in ABI argument-register order and consumes the location state
> decoded by parameter__decode_location() in the previous commit. Each
> parameter advances the expected-register index by the number of argument
> registers it occupies (parameter__abi_slots(), e.g. a two-eightbyte
> aggregate consumes two registers).
>
> For every producer it keeps the existing bookkeeping, now driven by the
> decoded fields:
>   - a parameter with no location, a constant value, or (for non-clang) no
>     register found is marked optimized out
>   - a parameter found in a register other than the expected one is marked
>     unexpected_reg
>
> When true_signature is enabled it reconstructs the real register-level
> signature:
>   - parameters that were optimized out are dropped from the signature
>   - a parameter whose location cannot be tied to its expected register,
>     wrong register, no register found, or a non-aggregate sitting on the
>     stack - marks the function unexpected_reg so no untrustworthy signature
>     is emitted;
>   - an aggregate genuinely passed on the stack (passed_in_memory) is kept;
>   - an aggregate split across registers via DW_OP_piece is kept whole when
>     it is fully used or the next parameter still lands on its expected
>     register, otherwise it is rewritten to the single member actually passed
>     in a register (true_sig_*).
>
> Together with the decoding commit this replaces the previous inline,
> per-parameter register check in parameter__new().
>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>   dwarf_loader.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 149 insertions(+), 3 deletions(-)
>
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 01954c6..7ccad93 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -3013,6 +3013,150 @@ static void ftype__recode_dwarf_types(struct tag *tag, struct cu *cu)
>   	}
>   }
>   
> +static struct parameter *ftype__next_parameter(struct ftype *ftype, struct parameter *parm)
> +{
> +	if (parm->tag.node.next == &ftype->parms)
> +		return NULL;
> +	return list_entry(parm->tag.node.next, struct parameter, tag.node);
> +}
> +
> +static int parameter__abi_slots(const struct parameter *parm, const struct cu *cu)
> +{
> +	int slots;
> +
> +	if (!cu->agg_use_two_regs || parm->type_byte_size <= cu->addr_size)
> +		return 1;
> +
> +	slots = (parm->type_byte_size + cu->addr_size - 1) / cu->addr_size;
> +	return slots > 0 ? slots : 1;
> +}
> +
> +static bool parameter__has_piece_info(const struct parameter *parm)
> +{
> +	return parm->first_reg_fields || parm->second_reg_fields;
> +}
> +
> +static bool parameter__uses_full_aggregate(const struct parameter *parm)
> +{
> +	return parm->first_reg_fields && parm->second_reg_fields;
> +}
> +
> +static bool ftype__next_parameter_preserves_slots(struct ftype *ftype, struct parameter *parm,
> +						  int reg_idx, int slots, struct cu *cu)
> +{
> +	struct parameter *next = ftype__next_parameter(ftype, parm);
> +	int next_reg_idx;
> +
> +	if (!next || next->loc_reg == PARAMETER_UNKNOWN_REG)
> +		return false;
> +
> +	next_reg_idx = reg_idx + slots;
> +	return next_reg_idx < cu->nr_register_params &&
> +	       next->loc_reg == cu->register_params[next_reg_idx];
> +}
> +
> +static bool parameter__apply_true_sig_member(struct parameter *parm, struct cu *cu)
> +{
> +	struct dwarf_tag tmp = {};
> +	struct dwarf_tag *dtype;
> +
> +	if (!parm->true_sig_member_name || parm->true_sig_type == 0)
> +		return false;
> +
> +	tmp.type = parm->true_sig_type;
> +	tmp.from_types_section.type = parm->true_sig_type_from_types;
> +	dtype = __dwarf_cu__find_type_by_ref(cu->priv, tmp.type, tmp.from_types_section.type);
> +	if (!dtype)
> +		return false;
> +
> +	parm->tag.type = dtype->small_id;
> +	return true;
> +}
> +
> +static void function__analyze_parameter_locations(struct function *fn, struct cu *cu,
> +						  struct conf_load *conf)
> +{
> +	struct ftype *ftype = &fn->proto;
> +	struct parameter *pos;
> +	bool true_sig_enabled = conf->true_signature;
> +	int reg_idx = 0;
> +
> +	ftype__for_each_parameter(ftype, pos) {
> +		bool consumes_register = true;
> +		bool regs_available = reg_idx < cu->nr_register_params;
> +		int slots = parameter__abi_slots(pos, cu);
> +		int expected_reg = regs_available ? cu->register_params[reg_idx] : -1;
> +		int reg_slots = pos->passed_in_memory ? 1 : slots;
> +
> +		if (pos->has_loc) {
> +			if (true_sig_enabled && pos->loc_const_value) {
> +				pos->optimized = 1;
> +				consumes_register = false;
> +				goto next;
> +			}
> +
> +			if (!regs_available) {
> +				consumes_register = false;
> +				goto next;
> +			}
> +
> +			if (true_sig_enabled && pos->loc_stack) {
> +				if (pos->passed_in_memory)
> +					consumes_register = false;
> +				else
> +					pos->unexpected_reg = 1;
> +				goto next;
> +			}
> +
> +			if (pos->loc_reg == PARAMETER_UNKNOWN_REG) {
> +				if (true_sig_enabled)
> +					pos->unexpected_reg = 1;
> +				else
> +					pos->optimized = 1;
> +				goto next;
> +			}
> +
> +			if (expected_reg >= 0 && expected_reg != pos->loc_reg) {
> +				pos->unexpected_reg = 1;
> +				goto next;
> +			}
> +
> +			if (true_sig_enabled && parameter__has_piece_info(pos)) {
> +				if (parameter__uses_full_aggregate(pos)) {
> +					reg_idx += slots;
> +					continue;
> +				}
> +
> +				if (ftype__next_parameter_preserves_slots(ftype, pos, reg_idx, slots, cu)) {
> +					pos->true_sig_member_name = 0;
> +					reg_idx += slots;
> +					continue;
> +				}
> +
> +				if (parameter__apply_true_sig_member(pos, cu)) {
> +					reg_idx++;
> +					continue;
> +				}
> +			}
> +		} else if (pos->has_const_value && !cu->producer_clang) {
> +			pos->optimized = 1;
> +		} else if (true_sig_enabled) {
> +			if (regs_available &&
> +			    ftype__next_parameter_preserves_slots(ftype, pos, reg_idx, slots, cu)) {
> +				reg_idx += slots;
> +				continue;
> +			}
> +
> +			pos->optimized = 1;
> +			consumes_register = false;
> +		}
> +
> +next:
> +		if (consumes_register)
> +			reg_idx += reg_slots;
> +	}
> +}
> +
>   static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
>   {
>   	struct tag *pos;
> @@ -3288,7 +3432,7 @@ static bool param__is_struct(struct cu *cu, struct tag *tag)
>   	}
>   }
>   
> -static int cu__resolve_func_ret_types_optimized(struct cu *cu)
> +static int cu__resolve_func_ret_types_optimized(struct cu *cu, struct conf_load *conf)
>   {
>   	struct ptr_table *pt = &cu->functions_table;
>   	uint32_t i;
> @@ -3299,6 +3443,8 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
>   		struct function *fn = tag__function(tag);
>   		bool has_unexpected_reg = false, has_struct_param = false;
>   
> +		function__analyze_parameter_locations(fn, cu, conf);
> +
>   		/* mark function as optimized if parameter is, or
>   		 * if parameter does not have a location; at this
>   		 * point location presence has been marked in
> @@ -3477,7 +3623,7 @@ static int die__process_and_recode(Dwarf_Die *die, struct cu *cu, struct conf_lo
>   	if (ret != 0)
>   		return ret;
>   
> -	return cu__resolve_func_ret_types_optimized(cu);
> +	return cu__resolve_func_ret_types_optimized(cu, conf);
>   }
>   
>   static int class_member__cache_byte_size(struct tag *tag, struct cu *cu,
> @@ -4240,7 +4386,7 @@ static int cus__merge_and_process_cu(struct cus *cus, struct conf_load *conf,
>   	 * encoded in another subprogram through abstract_origin
>   	 * tag. Let us visit all subprograms again to resolve this.
>   	 */
> -	if (cu__resolve_func_ret_types_optimized(cu) != LSK__KEEPIT)
> +	if (cu__resolve_func_ret_types_optimized(cu, conf) != LSK__KEEPIT)
>   		goto out_abort;
>   
>   	cu__finalize(cu, cus, conf);

Okay, I think there exists a problem here.
For example, if there is function without nocall:
      foo(a, b, c, d)
where a: RDI, b/c: no locations, d: RCX

of another function with nocall:
      bar(a, b, c, d)
where a: RDI, b/c: no locations, d: RSI

Without nocall, we expect the next argument e.g. 'b' will stay.
Here, no-nocall means source signatures are not changed (although locations may differ from ABI).

With nocall, we expect the next argument e.g., 'b' will be optimized.

I will fix it in the next revision.


  reply	other threads:[~2026-06-24  1:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-23 22:28 [PATCH dwarves v8 0/5] pahole: Encode true signatures in kernel BTF Yonghong Song
2026-06-23 22:28 ` [PATCH dwarves v8 1/5] dwarf_loader: Detect aggregate ABI register usage and clang compiler Yonghong Song
2026-06-23 22:29 ` [PATCH dwarves v8 2/5] dwarf_loader: Collect per-parameter information Yonghong Song
2026-06-23 22:29 ` [PATCH dwarves v8 3/5] dwarf_loader: Analyze per-parameter information for true signatures Yonghong Song
2026-06-24  1:21   ` Yonghong Song [this message]
2026-06-23 22:29 ` [PATCH dwarves v8 4/5] btf_encoder: Emit true function signatures Yonghong Song
2026-06-24  1:45   ` Yonghong Song
2026-06-23 22:29 ` [PATCH dwarves v8 5/5] tests: Add BTF true_signature encoding tests 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=7e2960f3-cad7-46a5-a691-29228fb696c6@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=alan.maguire@oracle.com \
    --cc=andrii@kernel.org \
    --cc=arnaldo.melo@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dwarves@vger.kernel.org \
    --cc=kernel-team@fb.com \
    /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