Re: [PATCH dwarves 1/4] dwarf_loader/btf_encoder: Detect reordered parameters

On 1/13/26 5:13 AM, Alan Maguire wrote:
> When encoding concrete instances of optimized functions it is possible
> parameters get reordered, often due to a parameter being optimized out;
> in such cases the order of abstract origin references to the abstract
> function is different, and the parameters that are optimized out
> usually appear after all the non-optimized parameters with no
> DW_AT_location information [1].
>
> As an example consider
>
> static void __blkcg_rstat_flush(struct blkcg *blkcg, int cpu);
>
> It has - as expected - an abstract representation as follows:
>
>   <1><6392a2d>: Abbrev Number: 47 (DW_TAG_subprogram)
>      <6392a2e>   DW_AT_name        : (indirect string, offset: 0x261e25): __blkcg_rstat_flush
>      <6392a32>   DW_AT_decl_file   : 1
>      <6392a33>   DW_AT_decl_line   : 1043
>      <6392a35>   DW_AT_decl_column : 13
>      <6392a36>   DW_AT_prototyped  : 1
>      <6392a36>   DW_AT_inline      : 1   (inlined)
>      <6392a37>   DW_AT_sibling     : <0x6392bac>
>   <2><6392a3b>: Abbrev Number: 38 (DW_TAG_formal_parameter)
>      <6392a3c>   DW_AT_name        : (indirect string, offset: 0xa7a9f): blkcg
>      <6392a40>   DW_AT_decl_file   : 1
>      <6392a41>   DW_AT_decl_line   : 1043
>      <6392a43>   DW_AT_decl_column : 47
>      <6392a44>   DW_AT_type        : <0x638b611>
>   <2><6392a48>: Abbrev Number: 20 (DW_TAG_formal_parameter)
>      <6392a49>   DW_AT_name        : cpu
>      <6392a4d>   DW_AT_decl_file   : 1
>      <6392a4e>   DW_AT_decl_line   : 1043
>      <6392a50>   DW_AT_decl_column : 58
>      <6392a51>   DW_AT_type        : <0x6377f8f>
>
> However the concrete representation after optimization becomes:
>
> ffffffff8186d180 t __blkcg_rstat_flush.isra.0
>
> and has a concrete representation with parameter order switched:
>
> <1><6399661>: Abbrev Number: 110 (DW_TAG_subprogram)
>      <6399662>   DW_AT_abstract_origin: <0x6392a2d>
>      <6399666>   DW_AT_low_pc      : 0xffffffff8186d180
>      <639966e>   DW_AT_high_pc     : 0x169
>      <6399676>   DW_AT_frame_base  : 1 byte block: 9c    (DW_OP_call_frame_cfa)
>      <6399678>   DW_AT_GNU_all_call_sites: 1
>      <6399678>   DW_AT_sibling     : <0x6399a8a>
>   <2><639967c>: Abbrev Number: 4 (DW_TAG_formal_parameter)
>      <639967d>   DW_AT_abstract_origin: <0x6392a48>
>      <6399681>   DW_AT_location    : 0x1fe21fb (location list)
>      <6399685>   DW_AT_GNU_locviews: 0x1fe21f5
>   <2><63996e4>: Abbrev Number: 4 (DW_TAG_formal_parameter)
>      <63996e5>   DW_AT_abstract_origin: <0x6392a3b>
>      <63996e9>   DW_AT_location    : 0x1fe2387 (location list)
>      <63996ed>   DW_AT_GNU_locviews: 0x1fe2385
>
> In other words we end up with
>
> static void __blkcg_rstat_flush.isra(int cpu, struct blkcg *blkcg);
>
> We are not detecting cases like this in pahole, so we need to
> catch it to exclude such cases since they could lead to incorrect
> fentry attachment.
>
> Future work around true function signatures will allow such functions
> with their "." suffixes, but even for such cases it is good to
> detect the reordering.
>
> In practice we just end up excluding a few more .isra/.constprop
> functions which we cannot fentry-attach by name anyway; see [2] for an
> example list from CI.
>
> [1] https://lore.kernel.org/bpf/[email protected]/
> [2] https://github.com/alan-maguire/dwarves/actions/runs/20031993822
>
> Signed-off-by: Alan Maguire <[email protected]>

Acked-by: Yonghong Song <[email protected]>