* [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
@ 2025-10-03 17:36 Yonghong Song
2025-10-20 10:53 ` Alan Maguire
2025-10-21 12:32 ` Jakub Sitnicki
0 siblings, 2 replies; 11+ messages in thread
From: Yonghong Song @ 2025-10-03 17:36 UTC (permalink / raw)
To: Alan Maguire, Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team
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
---
btf_encoder.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/btf_encoder.c b/btf_encoder.c
index 0bc2334..18f0162 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -2652,6 +2652,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
*/
if (fn->declaration)
continue;
+ if (function__inlined(fn))
+ continue;
if (!ftype__has_arg_names(&fn->proto))
continue;
if (funcs->cnt) {
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-03 17:36 [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF Yonghong Song
@ 2025-10-20 10:53 ` Alan Maguire
2025-10-20 16:01 ` Yonghong Song
2025-10-21 12:32 ` Jakub Sitnicki
1 sibling, 1 reply; 11+ messages in thread
From: Alan Maguire @ 2025-10-20 10:53 UTC (permalink / raw)
To: Yonghong Song, Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team
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/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-20 10:53 ` Alan Maguire
@ 2025-10-20 16:01 ` Yonghong Song
2025-10-20 20:11 ` Alan Maguire
0 siblings, 1 reply; 11+ messages in thread
From: Yonghong Song @ 2025-10-20 16:01 UTC (permalink / raw)
To: Alan Maguire, Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team
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/
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-20 16:01 ` Yonghong Song
@ 2025-10-20 20:11 ` Alan Maguire
2025-10-20 20:44 ` David Faust
0 siblings, 1 reply; 11+ messages in thread
From: Alan Maguire @ 2025-10-20 20:11 UTC (permalink / raw)
To: Yonghong Song, Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-20 20:11 ` Alan Maguire
@ 2025-10-20 20:44 ` David Faust
2025-10-22 9:23 ` Alan Maguire
0 siblings, 1 reply; 11+ messages in thread
From: David Faust @ 2025-10-20 20:44 UTC (permalink / raw)
To: Alan Maguire, Yonghong Song, Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team
On 10/20/25 13:11, Alan Maguire wrote:
> 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.
Yeah, I think most likely 'autolabel' was const-propagated and *fl6 is
the last real arg as you suggest.
I'm not an expert on the IPA optimization passes, but I don't know of
any that would reorder parameters like that.
OTOH, I see a few places in kernel sources where ip6_make_flowlabel is
passed a simple 'true' for autolabel. That sort of thing will almost
certainly be optimized by the IPA-cprop pass.
Note that you may have _both_ the "real" version and the .constprop
version of the function. IPA-cprop can create specialized versions
of functions so places where a parameter is a known constant can use
the .constprop version (where 'autolabel' has been dropped) while
other places where it may be variable use the original.
IPA-SRA (.isra suffix) can also change function parameters and return
values, but afaiu it does not reorder existing parameters.
>
>>>
>>> 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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-03 17:36 [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF Yonghong Song
2025-10-20 10:53 ` Alan Maguire
@ 2025-10-21 12:32 ` Jakub Sitnicki
2025-10-21 14:32 ` Alan Maguire
1 sibling, 1 reply; 11+ messages in thread
From: Jakub Sitnicki @ 2025-10-21 12:32 UTC (permalink / raw)
To: Yonghong Song
Cc: Alan Maguire, Arnaldo Carvalho de Melo, dwarves,
Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team, Matt Fleming, kernel-team
On Fri, Oct 03, 2025 at 10:36 AM -07, Yonghong Song wrote:
> 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.
I have a semi-related question: are there any plans for BTF to indicate
when a function has been inlined? Not necessarily where it has been
inlined, just that it has, somewhere, at least once.
When tracing with bpftrace or perf without a vmlinux available, it's
easy to assume you're tracing all calls to a function, when in fact some
calls may be inlined within the same compilation unit.
A good example is tracing the rtnl_lock - there are multiple inlined
copies, but neither bpftrace nor perf can warn you about it when debug
info is absent.
$ sudo perf probe -a rtnl_lock
Added new event:
probe:rtnl_lock (on rtnl_lock)
You can now use it in all perf tools, such as:
perf record -e probe:rtnl_lock -aR sleep 1
$ sudo apt install linux-image-`uname -r`-dbg
Installing:
linux-image-6.12.53-cloudflare-2025.10.4-dbg
[…]
$ sudo perf probe -d rtnl_lock
Removed event: probe:rtnl_lock
$ sudo perf probe -a rtnl_lock
Added new events:
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
probe:rtnl_lock (on rtnl_lock)
You can now use it in all perf tools, such as:
perf record -e probe:rtnl_lock -aR sleep 1
$
Thanks,
-jkbs
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-21 12:32 ` Jakub Sitnicki
@ 2025-10-21 14:32 ` Alan Maguire
2025-10-21 14:54 ` Arnaldo Carvalho de Melo
2025-10-21 19:06 ` Jakub Sitnicki
0 siblings, 2 replies; 11+ messages in thread
From: Alan Maguire @ 2025-10-21 14:32 UTC (permalink / raw)
To: Jakub Sitnicki, Yonghong Song
Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team, Matt Fleming,
kernel-team
On 21/10/2025 13:32, Jakub Sitnicki wrote:
> On Fri, Oct 03, 2025 at 10:36 AM -07, Yonghong Song wrote:
>> 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.
>
> I have a semi-related question: are there any plans for BTF to indicate
> when a function has been inlined? Not necessarily where it has been
> inlined, just that it has, somewhere, at least once.
>
> When tracing with bpftrace or perf without a vmlinux available, it's
> easy to assume you're tracing all calls to a function, when in fact some
> calls may be inlined within the same compilation unit.
>
> A good example is tracing the rtnl_lock - there are multiple inlined
> copies, but neither bpftrace nor perf can warn you about it when debug
> info is absent.
>
hi Jakub, see the RFC series at [1]. The goal is to represent inline
sites in BTF such that we can see when a function has been partially or
fully inlined, or indeed when optimizations have been applied to its ,
parameters which result in it being unsafe for fprobe()ing - in these
cases we skip representing such functions in BTF today.
In the case of inlined/optimized functions the proposal is to represent
them via BTF location data; not all of these locations will have all
parameters available due to optimization etc. However even absent that
it is still valuable to know such inlining has occurred as you say.
[1]
https://lore.kernel.org/bpf/20251008173512.731801-1-alan.maguire@oracle.com/
> $ sudo perf probe -a rtnl_lock
> Added new event:
> probe:rtnl_lock (on rtnl_lock)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:rtnl_lock -aR sleep 1
>
> $ sudo apt install linux-image-`uname -r`-dbg
> Installing:
> linux-image-6.12.53-cloudflare-2025.10.4-dbg
> […]
> $ sudo perf probe -d rtnl_lock
> Removed event: probe:rtnl_lock
> $ sudo perf probe -a rtnl_lock
> Added new events:
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
> probe:rtnl_lock (on rtnl_lock)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:rtnl_lock -aR sleep 1
>
> $
>
> Thanks,
> -jkbs
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-21 14:32 ` Alan Maguire
@ 2025-10-21 14:54 ` Arnaldo Carvalho de Melo
2025-10-21 19:06 ` Jakub Sitnicki
1 sibling, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-10-21 14:54 UTC (permalink / raw)
To: Alan Maguire, Jakub Sitnicki, Yonghong Song
Cc: dwarves, Alexei Starovoitov, Andrii Nakryiko, bpf,
Daniel Borkmann, kernel-team, Matt Fleming, kernel-team
On October 21, 2025 11:32:08 AM GMT-03:00, Alan Maguire <alan.maguire@oracle.com> wrote:
>On 21/10/2025 13:32, Jakub Sitnicki wrote:
>> On Fri, Oct 03, 2025 at 10:36 AM -07, Yonghong Song wrote:
>>> 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.
>>
>> I have a semi-related question: are there any plans for BTF to indicate
>> when a function has been inlined? Not necessarily where it has been
>> inlined, just that it has, somewhere, at least once.
>>
>> When tracing with bpftrace or perf without a vmlinux available, it's
>> easy to assume you're tracing all calls to a function, when in fact some
>> calls may be inlined within the same compilation unit.
>>
>> A good example is tracing the rtnl_lock - there are multiple inlined
>> copies, but neither bpftrace nor perf can warn you about it when debug
>> info is absent.
>>
>
>hi Jakub, see the RFC series at [1]. The goal is to represent inline
>sites in BTF such that we can see when a function has been partially or
>fully inlined, or indeed when optimizations have been applied to its ,
>parameters which result in it being unsafe for fprobe()ing - in these
>cases we skip representing such functions in BTF today.
I wonder if at least telling the user that there are such optimized cases around line N on function F, etc it could help with workarounds while tracing.
I.e. represent it in BTF and let tools decide it's unsafe and use it just for these warnings.
>In the case of inlined/optimized functions the proposal is to represent
>them via BTF location data; not all of these locations will have all
>parameters available due to optimization etc. However even absent that
>it is still valuable to know such inlining has occurred as you say.
Oh well, that's what you propose 8-)
>
>[1]
>https://lore.kernel.org/bpf/20251008173512.731801-1-alan.maguire@oracle.com/
>
>> $ sudo perf probe -a rtnl_lock
>> Added new event:
>> probe:rtnl_lock (on rtnl_lock)
>>
>> You can now use it in all perf tools, such as:
>>
>> perf record -e probe:rtnl_lock -aR sleep 1
>>
>> $ sudo apt install linux-image-`uname -r`-dbg
>> Installing:
>> linux-image-6.12.53-cloudflare-2025.10.4-dbg
>> […]
>> $ sudo perf probe -d rtnl_lock
>> Removed event: probe:rtnl_lock
>> $ sudo perf probe -a rtnl_lock
>> Added new events:
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>> probe:rtnl_lock (on rtnl_lock)
>>
>> You can now use it in all perf tools, such as:
>>
>> perf record -e probe:rtnl_lock -aR sleep 1
>>
>> $
>>
>> Thanks,
>> -jkbs
>
- Arnaldo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-21 14:32 ` Alan Maguire
2025-10-21 14:54 ` Arnaldo Carvalho de Melo
@ 2025-10-21 19:06 ` Jakub Sitnicki
1 sibling, 0 replies; 11+ messages in thread
From: Jakub Sitnicki @ 2025-10-21 19:06 UTC (permalink / raw)
To: Alan Maguire
Cc: Yonghong Song, Arnaldo Carvalho de Melo, dwarves,
Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team, Matt Fleming, kernel-team
On Tue, Oct 21, 2025 at 03:32 PM +01, Alan Maguire wrote:
> On 21/10/2025 13:32, Jakub Sitnicki wrote:
>> On Fri, Oct 03, 2025 at 10:36 AM -07, Yonghong Song wrote:
>>> 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.
>>
>> I have a semi-related question: are there any plans for BTF to indicate
>> when a function has been inlined? Not necessarily where it has been
>> inlined, just that it has, somewhere, at least once.
>>
>> When tracing with bpftrace or perf without a vmlinux available, it's
>> easy to assume you're tracing all calls to a function, when in fact some
>> calls may be inlined within the same compilation unit.
>>
>> A good example is tracing the rtnl_lock - there are multiple inlined
>> copies, but neither bpftrace nor perf can warn you about it when debug
>> info is absent.
>>
>
> hi Jakub, see the RFC series at [1]. The goal is to represent inline
> sites in BTF such that we can see when a function has been partially or
> fully inlined, or indeed when optimizations have been applied to its ,
> parameters which result in it being unsafe for fprobe()ing - in these
> cases we skip representing such functions in BTF today.
>
> In the case of inlined/optimized functions the proposal is to represent
> them via BTF location data; not all of these locations will have all
> parameters available due to optimization etc. However even absent that
> it is still valuable to know such inlining has occurred as you say.
>
> [1]
> https://lore.kernel.org/bpf/20251008173512.731801-1-alan.maguire@oracle.com/
Fresh stuff! Thanks for the link, Alan.
(I could have used the search box harder. Shame on me.)
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-20 20:44 ` David Faust
@ 2025-10-22 9:23 ` Alan Maguire
2025-10-22 20:19 ` David Faust
0 siblings, 1 reply; 11+ messages in thread
From: Alan Maguire @ 2025-10-22 9:23 UTC (permalink / raw)
To: David Faust, Yonghong Song, Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team
On 20/10/2025 21:44, David Faust wrote:
>
>
> On 10/20/25 13:11, Alan Maguire wrote:
>> 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.
>
> Yeah, I think most likely 'autolabel' was const-propagated and *fl6 is
> the last real arg as you suggest.
>
> I'm not an expert on the IPA optimization passes, but I don't know of
> any that would reorder parameters like that.
>
> OTOH, I see a few places in kernel sources where ip6_make_flowlabel is
> passed a simple 'true' for autolabel. That sort of thing will almost
> certainly be optimized by the IPA-cprop pass.
>
> Note that you may have _both_ the "real" version and the .constprop
> version of the function. IPA-cprop can create specialized versions
> of functions so places where a parameter is a known constant can use
> the .constprop version (where 'autolabel' has been dropped) while
> other places where it may be variable use the original.
>
> IPA-SRA (.isra suffix) can also change function parameters and return
> values, but afaiu it does not reorder existing parameters.
>
Thanks for the additional info!
Looking at the specific case, here's one instance of the inlined
function's representation:
<1><be25126>: Abbrev Number: 35 (DW_TAG_subprogram)
<be25127> DW_AT_name : (indirect string, offset: 0x3bce6f):
ip6_make_flowlabel
<be2512b> DW_AT_decl_file : 3
<be2512c> DW_AT_decl_line : 952
<be2512e> DW_AT_decl_column : 22
<be2512f> DW_AT_prototyped : 1
<be2512f> DW_AT_type : <0xbdef11c>
<be25133> DW_AT_inline : 3 (declared as inline and inlined)
<be25134> DW_AT_sibling : <0xbe25187>
<2><be25138>: Abbrev Number: 20 (DW_TAG_formal_parameter)
<be25139> DW_AT_name : net
<be2513d> DW_AT_decl_file : 3
<be2513e> DW_AT_decl_line : 952
<be25140> DW_AT_decl_column : 53
<be25141> DW_AT_type : <0xbe019b0>
<2><be25145>: Abbrev Number: 20 (DW_TAG_formal_parameter)
<be25146> DW_AT_name : skb
<be2514a> DW_AT_decl_file : 3
<be2514b> DW_AT_decl_line : 952
<be2514d> DW_AT_decl_column : 74
<be2514e> DW_AT_type : <0xbdfd253>
<2><be25152>: Abbrev Number: 40 (DW_TAG_formal_parameter)
<be25153> DW_AT_name : (indirect string, offset: 0x10853):
flowlabel
<be25157> DW_AT_decl_file : 3
<be25158> DW_AT_decl_line : 953
<be2515a> DW_AT_decl_column : 13
<be2515b> DW_AT_type : <0xbdef11c>
<2><be2515f>: Abbrev Number: 40 (DW_TAG_formal_parameter)
<be25160> DW_AT_name : (indirect string, offset: 0x3bcc9e):
autolabel
<be25164> DW_AT_decl_file : 3
<be25165> DW_AT_decl_line : 953
<be25167> DW_AT_decl_column : 29
<be25168> DW_AT_type : <0xbdef194>
<2><be2516c>: Abbrev Number: 20 (DW_TAG_formal_parameter)
<be2516d> DW_AT_name : fl6
<be25171> DW_AT_decl_file : 3
<be25172> DW_AT_decl_line : 954
<be25174> DW_AT_decl_column : 21
<be25175> DW_AT_type : <0xbe100ac>
And here's the abstract origin reference to it which I believe causes
the trouble:
<1><be2708c>: Abbrev Number: 205 (DW_TAG_subprogram)
<be2708e> DW_AT_abstract_origin: <0xbe25126>
<be27092> DW_AT_low_pc : 0xffffffff81ecf390
<be2709a> DW_AT_high_pc : 0xa2
<be270a2> DW_AT_frame_base : 1 byte block: 9c
(DW_OP_call_frame_cfa)
<be270a4> DW_AT_call_all_calls: 1
<be270a4> DW_AT_sibling : <0xbe27268>
<2><be270a8>: Abbrev Number: 7 (DW_TAG_formal_parameter)
<be270a9> DW_AT_abstract_origin: <0xbe25138>
<be270ad> DW_AT_location : 0x18ed328 (location list)
<be270b1> DW_AT_GNU_locviews: 0x18ed31c
<2><be270b5>: Abbrev Number: 7 (DW_TAG_formal_parameter)
<be270b6> DW_AT_abstract_origin: <0xbe25145>
<be270ba> DW_AT_location : 0x18ed363 (location list)
<be270be> DW_AT_GNU_locviews: 0x18ed359
<2><be270c2>: Abbrev Number: 7 (DW_TAG_formal_parameter)
<be270c3> DW_AT_abstract_origin: <0xbe25152>
<be270c7> DW_AT_location : 0x18ed399 (location list)
<be270cb> DW_AT_GNU_locviews: 0x18ed38f
<2><be270cf>: Abbrev Number: 7 (DW_TAG_formal_parameter)
<be270d0> DW_AT_abstract_origin: <0xbe2516c>
<be270d4> DW_AT_location : 0x18ed3cb (location list)
<be270d8> DW_AT_GNU_locviews: 0x18ed3c3
<2><be270dc>: Abbrev Number: 16 (DW_TAG_variable)
<be270dd> DW_AT_abstract_origin: <0xbe25179>
<be270e1> DW_AT_location : 0x18ed3f6 (location list)
<be270e5> DW_AT_GNU_locviews: 0x18ed3f0
<2><be270e9>: Abbrev Number: 55 (DW_TAG_formal_parameter)
<be270ea> DW_AT_abstract_origin: <0xbe2515f>
So what you see above is two things. First the order of parameters is
not preserved; specifically the original function and inlined function
representation it is
net, skb, flowlabel, autolabel, fl6
...while the non-inlined references via abstract origin has order
net, skb, flowlabel, fl6, and finally autolabel (with a DW_TAG_variable
inbetween).
And secondly what's interesting here is that the other parameters all
specify locations while autolabel does not.
The problem we have is that
1. pahole does not attach any significance to reordering like this and
does not detect it as far as I can see (I've also observed similar
patterns in inline site representations where order differs from the
original abstract origin function)
2. pahole also does not enforce the need for location info for a
parameter (implicit assumption being that if no location is present it
is in the usual calling-convention-dictated place)
The combination of 1 and 2 leads to the problem observed.
The DWARF spec appears to mandate source code order for parameters but I
couldn't find any equivalent mention of abstract origin parameter
references.
From the above empirical case and others it _seems_ like the ordering
_is_ meaningful in cases like this. How we extract that meaning without
breaking other things is always the challenge though.
I've started experimenting with detecting location misordering in
abstract origin references in the work-in-progress location code since
it does more extensive parameter location handling. There are a fair few
instances of misordering detected, especially for inline expansions it
seems (likely due to more frequent argument omissions at inline sites).
I'm hoping detecting misordering combined with enforcing location info
for misordered cases might be enough to detect and handle cases like
this, but as always the worry is other stuff gets broken as a
consequence. I'll report back when I have more data.
Alan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF
2025-10-22 9:23 ` Alan Maguire
@ 2025-10-22 20:19 ` David Faust
0 siblings, 0 replies; 11+ messages in thread
From: David Faust @ 2025-10-22 20:19 UTC (permalink / raw)
To: Alan Maguire, Yonghong Song, Arnaldo Carvalho de Melo, dwarves
Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann,
kernel-team
On 10/22/25 02:23, Alan Maguire wrote:
> On 20/10/2025 21:44, David Faust wrote:
>>
>>
>> On 10/20/25 13:11, Alan Maguire wrote:
>>> 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.
>>
>> Yeah, I think most likely 'autolabel' was const-propagated and *fl6 is
>> the last real arg as you suggest.
>>
>> I'm not an expert on the IPA optimization passes, but I don't know of
>> any that would reorder parameters like that.
>>
>> OTOH, I see a few places in kernel sources where ip6_make_flowlabel is
>> passed a simple 'true' for autolabel. That sort of thing will almost
>> certainly be optimized by the IPA-cprop pass.
>>
>> Note that you may have _both_ the "real" version and the .constprop
>> version of the function. IPA-cprop can create specialized versions
>> of functions so places where a parameter is a known constant can use
>> the .constprop version (where 'autolabel' has been dropped) while
>> other places where it may be variable use the original.
>>
>> IPA-SRA (.isra suffix) can also change function parameters and return
>> values, but afaiu it does not reorder existing parameters.
>>
>
> Thanks for the additional info!
>
> Looking at the specific case, here's one instance of the inlined
> function's representation:
>
> <1><be25126>: Abbrev Number: 35 (DW_TAG_subprogram)
> <be25127> DW_AT_name : (indirect string, offset: 0x3bce6f):
> ip6_make_flowlabel
> <be2512b> DW_AT_decl_file : 3
> <be2512c> DW_AT_decl_line : 952
> <be2512e> DW_AT_decl_column : 22
> <be2512f> DW_AT_prototyped : 1
> <be2512f> DW_AT_type : <0xbdef11c>
> <be25133> DW_AT_inline : 3 (declared as inline and inlined)
> <be25134> DW_AT_sibling : <0xbe25187>
> <2><be25138>: Abbrev Number: 20 (DW_TAG_formal_parameter)
> <be25139> DW_AT_name : net
> <be2513d> DW_AT_decl_file : 3
> <be2513e> DW_AT_decl_line : 952
> <be25140> DW_AT_decl_column : 53
> <be25141> DW_AT_type : <0xbe019b0>
> <2><be25145>: Abbrev Number: 20 (DW_TAG_formal_parameter)
> <be25146> DW_AT_name : skb
> <be2514a> DW_AT_decl_file : 3
> <be2514b> DW_AT_decl_line : 952
> <be2514d> DW_AT_decl_column : 74
> <be2514e> DW_AT_type : <0xbdfd253>
> <2><be25152>: Abbrev Number: 40 (DW_TAG_formal_parameter)
> <be25153> DW_AT_name : (indirect string, offset: 0x10853):
> flowlabel
> <be25157> DW_AT_decl_file : 3
> <be25158> DW_AT_decl_line : 953
> <be2515a> DW_AT_decl_column : 13
> <be2515b> DW_AT_type : <0xbdef11c>
> <2><be2515f>: Abbrev Number: 40 (DW_TAG_formal_parameter)
> <be25160> DW_AT_name : (indirect string, offset: 0x3bcc9e):
> autolabel
> <be25164> DW_AT_decl_file : 3
> <be25165> DW_AT_decl_line : 953
> <be25167> DW_AT_decl_column : 29
> <be25168> DW_AT_type : <0xbdef194>
> <2><be2516c>: Abbrev Number: 20 (DW_TAG_formal_parameter)
> <be2516d> DW_AT_name : fl6
> <be25171> DW_AT_decl_file : 3
> <be25172> DW_AT_decl_line : 954
> <be25174> DW_AT_decl_column : 21
> <be25175> DW_AT_type : <0xbe100ac>
>
>
> And here's the abstract origin reference to it which I believe causes
> the trouble:
>
> <1><be2708c>: Abbrev Number: 205 (DW_TAG_subprogram)
> <be2708e> DW_AT_abstract_origin: <0xbe25126>
> <be27092> DW_AT_low_pc : 0xffffffff81ecf390
> <be2709a> DW_AT_high_pc : 0xa2
> <be270a2> DW_AT_frame_base : 1 byte block: 9c
> (DW_OP_call_frame_cfa)
> <be270a4> DW_AT_call_all_calls: 1
> <be270a4> DW_AT_sibling : <0xbe27268>
> <2><be270a8>: Abbrev Number: 7 (DW_TAG_formal_parameter)
> <be270a9> DW_AT_abstract_origin: <0xbe25138>
> <be270ad> DW_AT_location : 0x18ed328 (location list)
> <be270b1> DW_AT_GNU_locviews: 0x18ed31c
> <2><be270b5>: Abbrev Number: 7 (DW_TAG_formal_parameter)
> <be270b6> DW_AT_abstract_origin: <0xbe25145>
> <be270ba> DW_AT_location : 0x18ed363 (location list)
> <be270be> DW_AT_GNU_locviews: 0x18ed359
> <2><be270c2>: Abbrev Number: 7 (DW_TAG_formal_parameter)
> <be270c3> DW_AT_abstract_origin: <0xbe25152>
> <be270c7> DW_AT_location : 0x18ed399 (location list)
> <be270cb> DW_AT_GNU_locviews: 0x18ed38f
> <2><be270cf>: Abbrev Number: 7 (DW_TAG_formal_parameter)
> <be270d0> DW_AT_abstract_origin: <0xbe2516c>
> <be270d4> DW_AT_location : 0x18ed3cb (location list)
> <be270d8> DW_AT_GNU_locviews: 0x18ed3c3
> <2><be270dc>: Abbrev Number: 16 (DW_TAG_variable)
> <be270dd> DW_AT_abstract_origin: <0xbe25179>
> <be270e1> DW_AT_location : 0x18ed3f6 (location list)
> <be270e5> DW_AT_GNU_locviews: 0x18ed3f0
> <2><be270e9>: Abbrev Number: 55 (DW_TAG_formal_parameter)
> <be270ea> DW_AT_abstract_origin: <0xbe2515f>
>
> So what you see above is two things. First the order of parameters is
> not preserved; specifically the original function and inlined function
> representation it is
>
> net, skb, flowlabel, autolabel, fl6
>
> ...while the non-inlined references via abstract origin has order
>
> net, skb, flowlabel, fl6, and finally autolabel (with a DW_TAG_variable
> inbetween).
>
> And secondly what's interesting here is that the other parameters all
> specify locations while autolabel does not.
I think it's important to be a little careful, the actual inline
sites shall be reflected by DW_TAG_inlined_subroutine DIEs as direct
children of the blocks where the inlining occurs.
The first subprogram DIE above (be25126) is the "abstract instance"
reflecting the source code, and the second (be2708c) is a concrete
instance reflecting the ".constprop"-suffixed specialization of
ip6_make_flowlabel.
The autolabel param in the concrete out-of-line instance lacking
location information indicates that it has been const-propagated;
it is no longer passed at all (see note on AT_location below).
I think this case falls under what is described in the DWARF5 spec
as "out-of-line instances of inlined subroutines" sec. 3.3.8.3.
>
> The problem we have is that
>
> 1. pahole does not attach any significance to reordering like this and
> does not detect it as far as I can see (I've also observed similar
> patterns in inline site representations where order differs from the
> original abstract origin function)
>
> 2. pahole also does not enforce the need for location info for a
> parameter (implicit assumption being that if no location is present it
> is in the usual calling-convention-dictated place)
This assumption seems incorrect to me. Based on DWARF5 sec. 4.1:
4. AT_location "describes the location of a variable or parameter
at run-time.
If no location attribute is present... or if the location attribute is
present but has an empty location... the variable is assumed to exist
in the source code but not in the executable program."
Note the second para uses "variable" but based on the context I think
that it applies to parameters as well. i.e., missing AT_location
does not mean "in the default CC-dictated place" it means the
param is not actually being passed at all for this concrete instance.
This is also reflected in the formal_parameter lists of
DW_TAG_inlined_subroutine DIEs representing where the actual inlines
occur. I would think pahole should check every inlined_subroutine
to see whether that inlining has elided formals.
DW_AT_calling_convention indicates whether the subprogram does not
obey standard calling conventions. Not present is equivalent to
"CC_normal" - i.e. obeys standard calling conventions (3.3.1.1).
>
> The combination of 1 and 2 leads to the problem observed.
>
> The DWARF spec appears to mandate source code order for parameters but I
> couldn't find any equivalent mention of abstract origin parameter
> references.
>
> From the above empirical case and others it _seems_ like the ordering
> _is_ meaningful in cases like this. How we extract that meaning without
> breaking other things is always the challenge though.
I think the ordering and absence of AT_location are interrelated.
In general:
"1. An entry in the concrete instance tree may be omitted if it contains
only a DW_AT_abstract_origin attribute and either has no children, or
its children are omitted." (last page of 3.3.8.2)
If a param like 'autolabel' is optimized out so it has no AT_location,
then there may be nothing left except the AT_abstract_origin and it
could be omitted.
I do find it odd that 'autolabel' is present out-of-order with only
AT_abstract_origin here. I think the more reliable info is that
it has no AT_location in the concrete instance, meaning it's been
optimized away.
>
> I've started experimenting with detecting location misordering in
> abstract origin references in the work-in-progress location code since
> it does more extensive parameter location handling. There are a fair few
> instances of misordering detected, especially for inline expansions it
> seems (likely due to more frequent argument omissions at inline sites).
> I'm hoping detecting misordering combined with enforcing location info
> for misordered cases might be enough to detect and handle cases like
> this, but as always the worry is other stuff gets broken as a
> consequence. I'll report back when I have more data.
>
> Alan
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-10-22 20:19 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-03 17:36 [PATCH dwarves] pahole: Avoid generating artificial inlined functions for BTF Yonghong Song
2025-10-20 10:53 ` Alan Maguire
2025-10-20 16:01 ` Yonghong Song
2025-10-20 20:11 ` Alan Maguire
2025-10-20 20:44 ` David Faust
2025-10-22 9:23 ` Alan Maguire
2025-10-22 20:19 ` David Faust
2025-10-21 12:32 ` Jakub Sitnicki
2025-10-21 14:32 ` Alan Maguire
2025-10-21 14:54 ` Arnaldo Carvalho de Melo
2025-10-21 19:06 ` Jakub Sitnicki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox