From: Jiri Olsa <olsajiri@gmail.com>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: acme@kernel.org, ast@kernel.org, andrii@kernel.org,
eddyz87@gmail.com, qmo@kernel.org, dwarves@vger.kernel.org,
daniel@iogearbox.net, ihor.solodrai@linux.dev,
yonghong.song@linux.dev, song@kernel.org, martin.lau@linux.dev,
memxor@gmail.com, emil@etsalapatis.com, bpf@vger.kernel.org,
nsc@kernel.org, puranjay@kernel.org, yatsenko@meta.com
Subject: Re: [PATCH v3 dwarves 1/9] dwarf_loader: Add parameter list to inlined expansions
Date: Wed, 30 Sep 2026 17:45:28 +0200 [thread overview]
Message-ID: <ar0umERDiFj2sN3W@krava> (raw)
In-Reply-To: <20260925185200.1318067-2-alan.maguire@oracle.com>
On Fri, Sep 25, 2026 at 07:51:50PM +0100, Alan Maguire wrote:
SNIP
> -static int die__process_inline_expansion(Dwarf_Die *die, struct lexblock *lexblock, struct cu *cu, struct conf_load *conf)
> +static int die__process_inline_expansion(Dwarf_Die *die,
> + struct inline_expansion *exp,
> + struct lexblock *lexblock,
> + struct cu *cu, struct conf_load *conf)
> {
> Dwarf_Die child;
> struct tag *tag;
> + int parm_idx = 0;
>
> if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0)
> return 0;
> @@ -2957,6 +2965,7 @@ static int die__process_inline_expansion(Dwarf_Die *die, struct lexblock *lexblo
> die = &child;
> do {
> uint32_t id;
> + bool add_to_inline_expansion = false;
>
> switch (dwarf_tag(die)) {
> case DW_TAG_call_site:
> @@ -2975,13 +2984,10 @@ static int die__process_inline_expansion(Dwarf_Die *die, struct lexblock *lexblo
> goto out_enomem;
> continue;
> case DW_TAG_formal_parameter:
> - /*
> - * Inline expansions can have their own formal
> - * parameter children duplicating the abstract
> - * origin's parameters. These are not needed
> - * for type reconstruction — skip them.
> - */
> - continue;
> + tag = die__create_new_parameter(die, NULL, lexblock, exp,
> + cu, conf, parm_idx++);
> + add_to_inline_expansion = true;
> + break;
> case DW_TAG_inlined_subroutine:
> tag = die__create_new_inline_expansion(die, lexblock, cu, conf);
> break;
> @@ -3010,6 +3016,8 @@ static int die__process_inline_expansion(Dwarf_Die *die, struct lexblock *lexblo
>
> if (cu__table_add_tag(cu, tag, &id) < 0)
> goto out_delete_tag;
> + if (add_to_inline_expansion)
could we just check (tag->tag == DW_TAG_formal_parameter) instead the
new bool?
> + inline_expansion__add_parameter(exp, tag__parameter(tag));
> hash:
> cu__hash(cu, tag);
> struct dwarf_tag *dtag = tag__dwarf(tag);
> @@ -3032,8 +3040,8 @@ static struct tag *die__create_new_inline_expansion(Dwarf_Die *die,
> if (exp == NULL)
> return NULL;
>
> - if (die__process_inline_expansion(die, lexblock, cu, conf) != 0) {
> - tag__free(&exp->ip.tag, cu);
> + if (die__process_inline_expansion(die, exp, lexblock, cu, conf) != 0) {
> + tag__delete(&exp->ip.tag, cu);
> return NULL;
> }
>
> @@ -3115,7 +3123,8 @@ 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, NULL,
> + cu, conf, param_idx++);
> break;
> case DW_TAG_variable:
> tag = die__create_new_variable(die, cu, conf, 0);
> @@ -3562,75 +3571,103 @@ static void __tag__print_abstract_origin_not_found(struct tag *tag,
> #define tag__print_abstract_origin_not_found(tag) \
> __tag__print_abstract_origin_not_found(tag, __func__, __LINE__)
>
> -static void ftype__recode_dwarf_types(struct tag *tag, struct cu *cu)
> +static void parameter__share_state_with_abstract_origin(struct parameter *parm,
> + struct parameter *oparm)
> +{
> + if (parm->has_loc)
> + oparm->has_loc = parm->has_loc;
> + if (parm->has_const_value)
> + oparm->has_const_value = parm->has_const_value;
> + if (parm->loc_const_value)
> + oparm->loc_const_value = parm->loc_const_value;
> + if (parm->loc_stack)
> + oparm->loc_stack = parm->loc_stack;
> + if (parm->loc_reg != PARAMETER_UNKNOWN_REG)
> + oparm->loc_reg = parm->loc_reg;
> + if (parm->type_byte_size != 0)
> + oparm->type_byte_size = parm->type_byte_size;
> + if (parm->passed_in_memory)
> + oparm->passed_in_memory = parm->passed_in_memory;
> + oparm->first_reg_fields |= parm->first_reg_fields;
> + oparm->second_reg_fields |= parm->second_reg_fields;
> + if (parm->true_sig_member_name && !oparm->true_sig_member_name) {
> + oparm->true_sig_member_name = parm->true_sig_member_name;
> + oparm->true_sig_type = parm->true_sig_type;
> + oparm->true_sig_type_from_types = parm->true_sig_type_from_types;
> + }
> + if (parm->optimized)
> + oparm->optimized = parm->optimized;
> + if (parm->unexpected_reg)
> + oparm->unexpected_reg = parm->unexpected_reg;
> +}
> +
> +static void parameter__recode_dwarf_type(struct parameter *parm, struct cu *cu,
> + struct ftype *ftype)
could this be added in separate change? seems like it could
ease up the readability a bit
> {
> - struct parameter *pos;
> struct dwarf_cu *dcu = cu->priv;
> - struct ftype *type = tag__ftype(tag);
> -
> - ftype__for_each_parameter(type, pos) {
> - struct dwarf_tag *dpos = tag__dwarf(&pos->tag);
> - struct parameter *opos;
> - struct dwarf_tag *dtype;
> + struct dwarf_tag *dparm = tag__dwarf(&parm->tag);
> + struct dwarf_tag *dtype;
>
> - if (dpos->type == 0) {
> - if (dpos->abstract_origin == 0) {
> - /* Function without parameters */
> - pos->tag.type = 0;
> - continue;
> - }
> - dtype = dwarf_cu__find_tag_by_ref(dcu, dpos, abstract_origin);
> - if (dtype == NULL) {
> - tag__print_abstract_origin_not_found(&pos->tag);
> - continue;
> - }
> - opos = tag__parameter(dtag__tag(dtype));
> - pos->name = opos->name;
> - if (pos->idx != opos->idx)
> - type->reordered_parm = 1;
> - pos->tag.type = dtag__tag(dtype)->type;
> - /* share location information between parameter and
> - * abstract origin; if neither have location, we will
> - * mark the parameter as optimized out. Also share
> - * info regarding unexpected register use for
> - * parameters.
> - */
> - if (pos->has_loc)
> - opos->has_loc = pos->has_loc;
> - if (pos->has_const_value)
> - opos->has_const_value = pos->has_const_value;
> - if (pos->loc_const_value)
> - opos->loc_const_value = pos->loc_const_value;
> - if (pos->loc_stack)
> - opos->loc_stack = pos->loc_stack;
> - if (pos->loc_reg != PARAMETER_UNKNOWN_REG)
> - opos->loc_reg = pos->loc_reg;
> - if (pos->type_byte_size != 0)
> - opos->type_byte_size = pos->type_byte_size;
> - if (pos->passed_in_memory)
> - opos->passed_in_memory = pos->passed_in_memory;
> - opos->first_reg_fields |= pos->first_reg_fields;
> - opos->second_reg_fields |= pos->second_reg_fields;
> - if (pos->true_sig_member_name && !opos->true_sig_member_name) {
> - opos->true_sig_member_name = pos->true_sig_member_name;
> - opos->true_sig_type = pos->true_sig_type;
> - opos->true_sig_type_from_types = pos->true_sig_type_from_types;
> - }
> + if (dparm->type == 0) {
> + struct parameter *oparm;
>
> - if (pos->optimized)
> - opos->optimized = pos->optimized;
> - if (pos->unexpected_reg)
> - opos->unexpected_reg = pos->unexpected_reg;
> - continue;
> + if (dparm->abstract_origin == 0) {
> + parm->tag.type = 0;
> + return;
> }
>
> - dtype = dwarf_cu__find_type_by_ref(dcu, dpos, type);
> + dtype = dwarf_cu__find_tag_by_ref(dcu, dparm, abstract_origin);
> if (dtype == NULL) {
> - tag__print_type_not_found(&pos->tag);
> - continue;
> + tag__print_abstract_origin_not_found(&parm->tag);
> + return;
> }
> - pos->tag.type = dtype->small_id;
> +
> + oparm = tag__parameter(dtag__tag(dtype));
> + parm->name = oparm->name;
> + if (ftype != NULL && parm->idx != oparm->idx)
> + ftype->reordered_parm = 1;
looks like we could end up with wrong argument index if the dwarf
info is missing for the argument, my agent is suggesting this fix
it gets me over 16k changes like this:
-0xffffffff8100e322 [__show_trace_log_lvl+0xb2, .text +0xe322] get_stack_pointer(struct task_struct * task [const 0], struct pt_regs * regs [%r14])
+0xffffffff8100e322 [__show_trace_log_lvl+0xb2, .text +0xe322] get_stack_pointer(struct task_struct * task [%r14], struct pt_regs * regs [const 0])
I tried to validate one against dwarf info and it seems
to be correct with the fix ;-)
thanks,
jirak
---
diff --git a/dwarf_loader.c b/dwarf_loader.c
index 49a88d526d9e..d0fb8fda08cd 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -3867,8 +3867,15 @@ static void parameter__recode_dwarf_type(struct parameter *parm, struct cu *cu,
oparm = tag__parameter(dtag__tag(dtype));
parm->name = oparm->name;
- if (ftype != NULL && parm->idx != oparm->idx)
- ftype->reordered_parm = 1;
+ if (ftype != NULL) {
+ if (parm->idx != oparm->idx)
+ ftype->reordered_parm = 1;
+ } else {
+ /* Inline-site parameters may omit or reorder children;
+ * use the abstract origin's position as the slot.
+ */
+ parm->idx = oparm->idx;
+ }
parm->tag.type = dtag__tag(dtype)->type;
if (ftype != NULL)
parameter__share_state_with_abstract_origin(parm, oparm);
next prev parent reply other threads:[~2026-09-30 15:45 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-25 18:51 [PATCH v3 dwarves 0/9] Encoding of inline functions using BTF Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 1/9] dwarf_loader: Add parameter list to inlined expansions Alan Maguire
2026-09-30 15:45 ` Jiri Olsa [this message]
2026-10-02 11:02 ` Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 2/9] dwarf_loader: Resolve inline expansion names during recoding Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 3/9] dwarf_loader: Collect inline expansion location data Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 4/9] pahole: Support inline BTF location encoding Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 5/9] btf_loader: Add "inline" annotation for inlined functions Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 6/9] btf_loader: Record inline sites in CU for BTF Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 7/9] pfunct: Print inline site information Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 8/9] tests: Validate inline BTF encoding Alan Maguire
2026-09-25 18:51 ` [PATCH v3 dwarves 9/9] man-pages: Document --inline_sites option Alan Maguire
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=ar0umERDiFj2sN3W@krava \
--to=olsajiri@gmail.com \
--cc=acme@kernel.org \
--cc=alan.maguire@oracle.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=dwarves@vger.kernel.org \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=ihor.solodrai@linux.dev \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=nsc@kernel.org \
--cc=puranjay@kernel.org \
--cc=qmo@kernel.org \
--cc=song@kernel.org \
--cc=yatsenko@meta.com \
--cc=yonghong.song@linux.dev \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.