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

[Alan Maguire <alan.maguire@oracle.com>]

On 20/10/2025 17:01, Yonghong Song wrote:
> 
> 
> 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...
> 

Yeah, I'm checking into this because I sort of wonder if it's a bug in
pahole processing and that the bool was in fact constant-propagated and
the struct fl6 * was actually the last ip6_make_flowlabel.constprop
parameter. Might be an issue in how we handle abstract origin cases.

>>
>> 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.
> 

Yeah, it's all a symptom of the fact we are trying to reconstruct things
with incomplete info; these are all tradeoffs but I'm hoping with the
location info we can at least provide data about the tricky cases rather
than simply skip them.

>>
>> 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.
>

Will do; sorry it took me a while to get around to this one! Thanks!

Alan