Re: [PATCH dwarves v5 01/11] dwarf_loader: Reduce parameter checking with clang DW_AT_calling_convention attr
Alan Maguire <[email protected]> Thu, 11 Jun 2026 10:15:26 +0100
| Newsgroups | org.kernel.vger.dwarves,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 23/05/2026 17:57, Yonghong Song wrote:
> Currently every function is checked for its parameters to identify whether
> the signature changed or not. If signature indeed changed, pahole may do
> some adjustment for parameters for true signatures.
>
> In clang, any function with the following attribute
> DW_AT_calling_convention (DW_CC_nocall)
> indicates this function having signature changed.
> pahole can take advantage of this to avoid parameter checking if
> DW_AT_calling_convention is not DW_CC_nocall.
>
> But more importantly, DW_CC_nocall can identify signature-changed functions
> and parameters can be checked one-after-another to create the true
> signatures. Otherwise, it takes more effort to identify whether a
> function has signature changed or not. For example, for funciton
> __bpf_kfunc static void bbr_main(struct sock *sk, u32 ack, int flag,
> const struct rate_sample *rs) { ... }
> and bbr_main() is a callback function in
> .cong_control = bbr_main
> in 'struct tcp_congestion_ops tcp_bbr_cong_ops'.
> In the above bbr_main(...), parameter 'ack' and 'flag' are not used.
> The following are some details:
>
> 0x0a713b8d: DW_TAG_formal_parameter
> DW_AT_location (indexed (0x28) loclist = 0x0166d452:
> [0xffffffff83e77fd9, 0xffffffff83e78016): DW_OP_reg5 RDI
> ...
> DW_AT_name ("sk")
> DW_AT_type (0x0a6f5b2b "sock *")
> ...
>
> 0x0a713b98: DW_TAG_formal_parameter
> DW_AT_name ("ack")
> DW_AT_type (0x0a6f58fd "u32")
> ...
>
> 0x0a713ba2: DW_TAG_formal_parameter
> DW_AT_name ("flag")
> DW_AT_type (0x0a6f57d1 "int")
> ...
>
> 0x0a713bac: DW_TAG_formal_parameter
> DW_AT_location (indexed (0x29) loclist = 0x0166d4a8:
> [0xffffffff83e77fd9, 0xffffffff83e78016): DW_OP_reg2 RCX
> ...
> DW_AT_name ("rs")
> DW_AT_type (0x0a710da5 "const rate_sample *")
>
> Some analysis for the above dwarf can conclude that the 'ark' and 'flag'
> may be related to RSI and RDX, considering the last one is RCX. Basically this
> requires all parameters are available to collectively decide whether the
> true signature can be found or not. In such case, DW_CC_nocall can make things
> easier as parameter can be checked one after another.
>
> For a clang built bpf-next kernel with x86_64, in non-LTO setup,
> the number of kernel functions is 69103 and the number of signature changed
> functions is 875, based on
> DW_AT_calling_convention (DW_CC_nocall)
> indication.
>
> Among 875 signature changed functions, after this patch, 343 functions
> can have proper true signatures, mostly due to simple dead argument
> elimination. The number of remaining functions, which cannot get the
> true signature, is 532 due to dead or additional-checked parameters.
>
> They will be addressed in the subsequent commits.
>
> In llvm23, I implemented [1] which added DW_CC_nocall for ArgumentPromotion pass.
> This compiler pass can add additional DW_CC_nocall cases for the following
> compilation:
> - Flag -O3 or FullLTO
> So once llvm23 available, we may have more DW_CC_nocall cases, hence more
> potential true signatures if the kernel is built with -O3 or
> with FullLTO (CONFIG_LTO_CLANG_FULL).
>
> [1] https://github.com/llvm/llvm-project/pull/178973
>
> Signed-off-by: Yonghong Song <[email protected]>
> ---
> dwarf_loader.c | 86 ++++++++++++++++++++++++++++++++++++++++++--------
> dwarves.h | 1 +
> 2 files changed, 73 insertions(+), 14 deletions(-)
>
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 16fb7be..0bc4fc4 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -1190,6 +1190,10 @@ static ptrdiff_t __dwarf_getlocations(Dwarf_Attribute *attr,
> return ret;
> }
>
> +struct func_info {
> + bool signature_changed;
> +};
Looking at the code, I wonder if we could simplify by passing in the ftype instead.
parameter__new() is called via
formal_paramater_pack__new
formal_parameter_pack__load_params
and
die__create_new_parameter
In both cases we can access the ftype (though in the case of the formal parameter
codepath we'd need to pass it through formal_parameter__pack_new()). If feels like
it might be a cleaner design to do that and set a signature_changed bitfield for
the ftype, since that's where we usually set attributes that apply the to signature,
what do you think?
The other aspects added in later patches I think would work there too. For example
the accumulation of parameters would likely be a bit easier as we could better
handle the skip_idx logic which is tricky. On that topic could we just add a
next_reg_idx directly to the ftype (or the func_info if we keep it) as the goal
of the skip_idx seems to be to figure out what the next expected reg idx from
the calling conventions should be.
> +
> /* For DW_AT_location 'attr':
> * - if first location is DW_OP_regXX with expected number, return the register;
> * otherwise save the register for later return
> @@ -1252,7 +1256,8 @@ out:
> }
>
> static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
> - struct conf_load *conf, int param_idx)
> + struct conf_load *conf, int param_idx,
> + struct func_info *info)
> {
> struct parameter *parm = tag__alloc(cu, sizeof(*parm));
>
> @@ -1263,8 +1268,15 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
> 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 (!info->signature_changed) {
> + if (cu->producer_clang || param_idx >= cu->nr_register_params)
> + return parm;
> + } else if (param_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)
> * are the result of later DWARF generation during compilation
> @@ -1337,7 +1349,7 @@ static int formal_parameter_pack__load_params(struct formal_parameter_pack *pack
> continue;
> }
>
> - struct parameter *param = parameter__new(die, cu, conf, -1);
> + struct parameter *param = parameter__new(die, cu, conf, -1, NULL);
>
> if (param == NULL)
> return -1;
> @@ -1502,6 +1514,29 @@ static struct ftype *ftype__new(Dwarf_Die *die, struct cu *cu)
> return ftype;
> }
>
> +static bool function__signature_changed(struct function *func, Dwarf_Die *die)
> +{
> + /* The inlined DW_TAG_subprogram typically has the original source type for
> + * abstract origin of a concrete function with address range, inlined subroutine,
> + * or call site.
> + */
> + if (func->inlined)
> + return false;
> +
> + if (!func->abstract_origin)
> + return attr_numeric(die, DW_AT_calling_convention) == DW_CC_nocall;
> +
> + Dwarf_Attribute attr;
> + if (dwarf_attr(die, DW_AT_abstract_origin, &attr)) {
> + Dwarf_Die origin;
> + if (dwarf_formref_die(&attr, &origin))
> + return attr_numeric(&origin, DW_AT_calling_convention) == DW_CC_nocall;
> + }
> +
> + /* This should not happen */
> + return false;
> +}
> +
> static struct function *function__new(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
> {
> struct function *func = tag__alloc(cu, sizeof(*func));
> @@ -1800,9 +1835,9 @@ static struct tag *die__create_new_parameter(Dwarf_Die *die,
> struct ftype *ftype,
> struct lexblock *lexblock,
> struct cu *cu, struct conf_load *conf,
> - int param_idx)
> + int param_idx, struct func_info *info)
> {
> - struct parameter *parm = parameter__new(die, cu, conf, param_idx);
> + struct parameter *parm = parameter__new(die, cu, conf, param_idx, info);
>
> if (parm == NULL)
> return NULL;
> @@ -1889,7 +1924,7 @@ static struct tag *die__create_new_subroutine_type(Dwarf_Die *die,
> tag__print_not_supported(die);
> continue;
> case DW_TAG_formal_parameter:
> - tag = die__create_new_parameter(die, ftype, NULL, cu, conf, -1);
> + tag = die__create_new_parameter(die, ftype, NULL, cu, conf, -1, NULL);
> break;
> case DW_TAG_unspecified_parameters:
> ftype->unspec_parms = 1;
> @@ -2118,7 +2153,8 @@ out_enomem:
> }
>
> static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
> - struct lexblock *lexblock, struct cu *cu, struct conf_load *conf);
> + struct lexblock *lexblock, struct cu *cu, struct conf_load *conf,
> + struct func_info *info);
>
> static int die__create_new_lexblock(Dwarf_Die *die,
> struct cu *cu, struct lexblock *father, struct conf_load *conf)
> @@ -2126,7 +2162,7 @@ static int die__create_new_lexblock(Dwarf_Die *die,
> struct lexblock *lexblock = lexblock__new(die, cu);
>
> if (lexblock != NULL) {
> - if (die__process_function(die, NULL, lexblock, cu, conf) != 0)
> + if (die__process_function(die, NULL, lexblock, cu, conf, NULL) != 0)
> goto out_delete;
> }
> if (father != NULL)
> @@ -2246,7 +2282,8 @@ static struct tag *die__create_new_inline_expansion(Dwarf_Die *die,
> }
>
> static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
> - struct lexblock *lexblock, struct cu *cu, struct conf_load *conf)
> + struct lexblock *lexblock, struct cu *cu, struct conf_load *conf,
> + struct func_info *info)
> {
> int param_idx = 0;
> Dwarf_Die child;
> @@ -2320,7 +2357,7 @@ static int die__process_function(Dwarf_Die *die, struct ftype *ftype,
> continue;
> }
> case DW_TAG_formal_parameter:
> - tag = die__create_new_parameter(die, ftype, lexblock, cu, conf, param_idx++);
> + tag = die__create_new_parameter(die, ftype, lexblock, cu, conf, param_idx++, info);
> break;
> case DW_TAG_variable:
> tag = die__create_new_variable(die, cu, conf, 0);
> @@ -2391,11 +2428,19 @@ out_enomem:
> static struct tag *die__create_new_function(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
> {
> struct function *function = function__new(die, cu, conf);
> + struct func_info info = {};
>
> - if (function != NULL &&
> - die__process_function(die, &function->proto, &function->lexblock, cu, conf) != 0) {
> - function__delete(function, cu);
> - function = NULL;
> + if (function != NULL) {
> + /* For clang, we determine if function signature changes via DW_AT_calling_convention
> + * set to DW_CC_nocall.
> + */
> + if (cu->producer_clang)
> + info.signature_changed = function__signature_changed(function, die);
> +
> + if (die__process_function(die, &function->proto, &function->lexblock, cu, conf, &info) != 0) {
> + function__delete(function, cu);
> + function = NULL;
> + }
> }
>
> return function ? &function->proto.tag : NULL;
> @@ -3045,6 +3090,17 @@ static unsigned long long dwarf_tag__orig_id(const struct tag *tag,
> return cu->extra_dbg_info ? dtag->id : 0;
> }
>
> +static bool attr_producer_clang(Dwarf_Die *die)
> +{
> + const char *producer;
> +
> + producer = attr_string(die, DW_AT_producer, NULL);
> + if (!producer)
> + return false;
> +
> + return !!strstr(producer, "clang");
> +}
> +
> struct debug_fmt_ops dwarf__ops;
>
> static int die__process(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
> @@ -3082,6 +3138,7 @@ static int die__process(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
> }
>
> cu->language = attr_numeric(die, DW_AT_language);
> + cu->producer_clang = attr_producer_clang(die);
>
> if (conf->early_cu_filter)
> cu = conf->early_cu_filter(cu);
> @@ -3841,6 +3898,7 @@ static int cus__merge_and_process_cu(struct cus *cus, struct conf_load *conf,
> cu->priv = dcu;
> cu->dfops = &dwarf__ops;
> cu->language = attr_numeric(cu_die, DW_AT_language);
> + cu->producer_clang = attr_producer_clang(cu_die);
> cus__add(cus, cu);
> }
>
> diff --git a/dwarves.h b/dwarves.h
> index 5ec16e7..b49e651 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -306,6 +306,7 @@ struct cu {
> uint8_t has_addr_info:1;
> uint8_t uses_global_strings:1;
> uint8_t little_endian:1;
> + uint8_t producer_clang:1;
> uint8_t nr_register_params;
> int register_params[ARCH_MAX_REGISTER_PARAMS];
> int functions_saved;