On Fri, Jun 13, 2025 at 1:59 AM Alexis Lothoré <alexis.loth...@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: > >> Hi Peter, > >> > >> On Fri Jun 13, 2025 at 10:11 AM CEST, Peter Zijlstra wrote: > >> > On Fri, Jun 13, 2025 at 09:37:11AM +0200, Alexis Lothoré (eBPF > >> > Foundation) wrote: > > [...] > > >> Maybe my commit wording is not precise enough, but indeed, there's not > >> doubt about whether the struct value is passed on the stack or through a > >> register/a pair of registers. The doubt is rather about the struct location > >> when it is passed _by value_ and _on the stack_: the ABI indeed clearly > >> states that "Structures and unions assume the alignment of their most > >> strictly aligned component" (p.13), but this rule is "silently broken" when > >> a struct has an __attribute__((packed)) or and __attribute__((aligned(X))), > >> and AFAICT this case can not be detected at runtime with current BTF info. > > > > Ah, okay. So it is a failure of BTF. That was indeed not clear. > > 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-0a32fe723...@bootlin.com/ 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-0a32fe723...@bootlin.com/ 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.
