On Sat Jun 14, 2025 at 12:35 AM CEST, Alexei Starovoitov wrote:
On Fri, Jun 13, 2025 at 1:59 AM Alexis Lothoré alexis.lothore@bootlin.com wrote:
On Fri Jun 13, 2025 at 10:32 AM CEST, Peter Zijlstra wrote:
On Fri, Jun 13, 2025 at 10:26:37AM +0200, Alexis Lothoré wrote:
[...]
If I need to respin, I'll rewrite the commit message to include the details above.
No need to respin. The cover letter is quite detailed already.
But looking at the patch and this thread I think we need to agree on the long term approach to BTF, since people assume that it's a more compact dwarf and any missing information should be added to it. Like in this case special alignment case and packed attributes are not expressed in BTF and I believe they should not be. BTF is not a debug format and not a substitute for dwarf. There is no goal to express everything possible in C. It's minimal, because BTF is _practical_ description of types and data present in the kernel. I don't think the special case of packing and alignment exists in the kernel today, so the current format is sufficient. It doesn't miss anything. I think we made arm64 JIT unnecessary restrictive and now considering to make all other JITs restrictive too for hypothetical case of some future kernel functions. I feel we're going in the wrong direction. Instead we should teach pahole to sanitize BTF where functions are using this fancy alignment and packed structs. pahole can see it in dwarf and can skip emitting BTF for such functions. Then the kernel JITs on all architectures won't even see such cases.
The issue was initially discovered by a selftest: https://lore.kernel.org/bpf/20250411-many_args_arm64-v1-3-0a32fe72339e@bootl... that attempted to support these two types: +struct bpf_testmod_struct_arg_4 {
- __u64 a;
- __u64 b;
+};
+struct bpf_testmod_struct_arg_5 {
- __int128 a;
+};
The former is present in the kernel. It's more or less sockptr_t, and people want to access it for observability in tracing. The latter doesn't exist in the kernel and we cannot represent it properly in BTF without losing alignment.
So I think we should go back to that series: https://lore.kernel.org/bpf/20250411-many_args_arm64-v1-0-0a32fe72339e@bootl...
remove __int128 selftest, but also teach pahole to recognize types that cannot be represented in BTF and don't emit them either into vmlinux or in kernel module (like in this case it was bpf_testmod.ko) I think that would be a better path forward aligned with the long term goal of BTF.
And before people ask... pahole is a trusted component of the build system. We trust it just as we trust gcc, clang, linker, objtool.
So if I understand correctly your point, it would be better to just move out those constraints from the JIT compilers, and just not represent those special cases in BTF, so that it becomes impossible to hook programs on those functions, since they are not event present in BTF info. And so: - cancel this series - revert the small ARM64 check about struct passed on stack - update pahole to make sure that it does not encode info about this specific kind of functions.
I still expect some challenges with this. AFAIU pahole uses DWARF to generate BTF, and discussions in [1] highlighted the fact that the attributes altering the structs alignment are not reliably encoded in DWARF. Maybe pahole can "guess" if a struct has been altered, by doing something like btf_is_struct_packed in libbpf ? As Andrii mentioned in [2], it may not be able to cover all cases, but that could be a start. If that's indeed the desired direction, I can take a further look at this.
+ CC dwarves ML
Alexis
[1] https://lore.kernel.org/bpf/9a2ba0ad-b34d-42f8-89a6-d9a44f007bdc@linux.dev/ [2] https://lore.kernel.org/bpf/CAEf4BzZHMYyGDZ4c4eNXG7Fm=ecxCCbKhKbQTbCjvWmKtdw...