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

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

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.

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.

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.

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