Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF

[Yonghong Song <yonghong.song@linux.dev>]

On 10/20/25 3:53 AM, Alan Maguire wrote:
> On 03/10/2025 18:36, Yonghong Song wrote:
>> In llvm pull request [1], the dwarf is changed to accommodate functions
>> whose signatures are different from source level although they have
>> the same name. Other non-source functions are also included in dwarf.
>>
>> The following is an example:
>>
>> The source:
>> ====
>>    $ cat test.c
>>    struct t { int a; };
>>    char *tar(struct t *a, struct t *d);
>>    __attribute__((noinline)) static char * foo(struct t *a, struct t *d, int b)
>>    {
>>      return tar(a, d);
>>    }
>>    char *bar(struct t *a, struct t *d)
>>    {
>>      return foo(a, d, 1);
>>    }
>> ====
>>
>> Part of generated dwarf:
>> ====
>> 0x0000005c:   DW_TAG_subprogram
>>                  DW_AT_low_pc    (0x0000000000000010)
>>                  DW_AT_high_pc   (0x0000000000000015)
>>                  DW_AT_frame_base        (DW_OP_reg7 RSP)
>>                  DW_AT_linkage_name      ("foo")
>>                  DW_AT_name      ("foo")
>>                  DW_AT_decl_file ("/home/yhs/tests/sig-change/deadarg/test.c")
>>                  DW_AT_decl_line (3)
>>                  DW_AT_type      (0x000000bb "char *")
>>                  DW_AT_artificial        (true)
>>                  DW_AT_external  (true)
>>
>> 0x0000006c:     DW_TAG_formal_parameter
>>                    DW_AT_location        (DW_OP_reg5 RDI)
>>                    DW_AT_decl_file       ("/home/yhs/tests/sig-change/deadarg/test.c")
>>                    DW_AT_decl_line       (3)
>>                    DW_AT_type    (0x000000c4 "t *")
>>
>> 0x00000075:     DW_TAG_formal_parameter
>>                    DW_AT_location        (DW_OP_reg4 RSI)
>>                    DW_AT_decl_file       ("/home/yhs/tests/sig-change/deadarg/test.c")
>>                    DW_AT_decl_line       (3)
>>                    DW_AT_type    (0x000000c4 "t *")
>>
>> 0x0000007e:     DW_TAG_inlined_subroutine
>>                    DW_AT_abstract_origin (0x0000009a "foo")
>>                    DW_AT_low_pc  (0x0000000000000010)
>>                    DW_AT_high_pc (0x0000000000000015)
>>                    DW_AT_call_file       ("/home/yhs/tests/sig-change/deadarg/test.c")
>>                    DW_AT_call_line       (0)
>>
>> 0x0000008a:       DW_TAG_formal_parameter
>>                      DW_AT_location      (DW_OP_reg5 RDI)
>>                      DW_AT_abstract_origin       (0x000000a2 "a")
>>
>> 0x00000091:       DW_TAG_formal_parameter
>>                      DW_AT_location      (DW_OP_reg4 RSI)
>>                      DW_AT_abstract_origin       (0x000000aa "d")
>>
>> 0x00000098:       NULL
>>
>> 0x00000099:     NULL
>>
>> 0x0000009a:   DW_TAG_subprogram
>>                  DW_AT_name      ("foo")
>>                  DW_AT_decl_file ("/home/yhs/tests/sig-change/deadarg/test.c")
>>                  DW_AT_decl_line (3)
>>                  DW_AT_prototyped        (true)
>>                  DW_AT_type      (0x000000bb "char *")
>>                  DW_AT_inline    (DW_INL_inlined)
>>
>> 0x000000a2:     DW_TAG_formal_parameter
>>                    DW_AT_name    ("a")
>>                    DW_AT_decl_file       ("/home/yhs/tests/sig-change/deadarg/test.c")
>>                    DW_AT_decl_line       (3)
>>                    DW_AT_type    (0x000000c4 "t *")
>>
>> 0x000000aa:     DW_TAG_formal_parameter
>>                    DW_AT_name    ("d")
>>                    DW_AT_decl_file       ("/home/yhs/tests/sig-change/deadarg/test.c")
>>                    DW_AT_decl_line       (3)
>>                    DW_AT_type    (0x000000c4 "t *")
>>
>> 0x000000b2:     DW_TAG_formal_parameter
>>                    DW_AT_name    ("b")
>>                    DW_AT_decl_file       ("/home/yhs/tests/sig-change/deadarg/test.c")
>>                    DW_AT_decl_line       (3)
>>                    DW_AT_type    (0x000000d8 "int")
>>
>> 0x000000ba:     NULL
>> ====
>>
>> In the above, there are two subprograms with the same name 'foo'.
>> Currently btf encoder will consider both functions as ELF functions.
>> Since two subprograms have different signature, the funciton will
>> be ignored.
>>
>> But actually, one of function 'foo' is marked as DW_INL_inlined which means
>> we should not treat it as an elf funciton. The patch fixed this issue
>> by filtering subprograms if the corresponding function__inlined() is true.
>>
>> This will fix the issue for [1]. But it should work fine without [1] too.
>>
>>    [1] https://github.com/llvm/llvm-project/pull/157349
> The change itself looks fine on the surface but it has some odd
> consequences that we need to find a solution for.
>
> Specifically in CI I was seeing an error in BTF-to-DWARF function
> comparison:
>
> https://github.com/alan-maguire/dwarves/actions/runs/18376819644/job/52352757287#step:7:40
>
> 1: Validation of BTF encoding of functions; this may take some time:
> ERROR: mismatch : BTF '__be32 ip6_make_flowlabel(struct net *, struct
> sk_buff *, __be32, struct flowi6 *, bool);' not found; DWARF ''
>
> Further investigation reveals the problem; there is a constprop variant
> of ip6_make_flowlabel():
>
> ffffffff81ecf390 t ip6_make_flowlabel.constprop.0
>
> ..and the problem is it has a different function signature:
>
> __be32 ip6_make_flowlabel(struct net *, struct sk_buff *, __be32, struct
> flowi6 *, bool);
>
> The "real" function (that was inlined, other than the constprop variant)
> looks like this:
>
> static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff
> *skb,
>   					__be32 flowlabel, bool autolabel,
>   					struct flowi6 *fl6);
>
> i.e. the last two parameters are in a different order.

It is interesting that gcc optimization may change parameter orders...

>
> Digging into the DWARF representation, the .constprop function uses an
> abstract origin reference to the original function. In the case prior to
> your change, we would have compared function signatures across both
> representations and found the inconsistency and then avoided emitting
> BTF for the function.
>
> However with your change, we no longer add a function representation for
> the inline case to contrast with and detect that inconsistency.
>
> So that's the core problem; your change is trying to avoid comparing
> across inlined and uninlined functions with the same name/prefix, but
> sometimes we need to do exactly that to detect inconsistent
> representations when they really are inlined/uninlined instances of the
> same function. I don't see an easy answer here since it seems to me both
> are legitimate cases.

The upstream does not like llvm pull request (associated with this patch)
so it is totally ok to discard this patch. Sorry, I think generally
we should only care about the *real* functions. But I missed that
you want to compare signatures of the *read* functions and *inlined* functions.

>
> I'm hoping we can use BTF location info [1] to cover cases like this
> where we have inconsistencies between types in parameters. Rather than
> having to decide which case is correct we simply use location
> representations for the cases where we are unsure. This will make such
> cases safely traceable since we have info about where parameters are stored.

Indeed this could solve the inlined functions problem. Again, please discard
this patch for now.

>
> [1]
> https://lore.kernel.org/bpf/20251008173512.731801-1-alan.maguire@oracle.com/
>