On 27/08/2025 20:13, Eduard Zingerman wrote: > On Wed, 2025-08-27 at 10:00 -0700, Yonghong Song wrote: >> >> On 8/26/25 10:02 PM, Eduard Zingerman wrote: >>> On Tue, 2025-08-26 at 13:17 -0700, Yonghong Song wrote: >>> >>> [...] >>> >>>> I tried with gcc14 and can reproduced the issue described in the above. >>>> I build the kernel like below with gcc14 >>>> make KCFLAGS='-O3' -j >>>> and get the following build error >>>> WARN: resolve_btfids: unresolved symbol bpf_strnchr >>>> make[2]: *** [/home/yhs/work/bpf-next/scripts/Makefile.vmlinux:91: vmlinux] Error 255 >>>> make[2]: *** Deleting file 'vmlinux' >>>> Checking the symbol table: >>>> 22276: ffffffff81b15260 249 FUNC LOCAL DEFAULT 1 bpf_strnchr.cons[...] >>>> 235128: ffffffff81b1f540 296 FUNC GLOBAL DEFAULT 1 bpf_strnchr >>>> and the disasm code: >>>> bpf_strnchr: >>>> ... >>>> >>>> bpf_strchr: >>>> ... >>>> bpf_strnchr.constprop.0 >>>> ... >>>> >>>> So in symbol table, we have both bpf_strnchr.constprop.0 and bpf_strnchr. >>>> For such case, pahole will skip func bpf_strnchr hence the above resolve_btfids >>>> failure. >>>> >>>> The solution in this patch can indeed resolve this issue. >>> It looks like instead of adding __noclone there is an option to >>> improve pahole's filtering of ambiguous functions. >>> Abstractly, there is nothing wrong with having a clone of a global >>> function that has undergone additional optimizations. As long as the >>> original symbol exists, everything should be fine. >> >> Right. The generated code itself is totally fine. The problem is >> currently pahole will filter out bpf_strnchr since in the symbol table >> having both bpf_strnchr and bpf_strnchr.constprop.0. It there is >> no explicit dwarf-level signature in dwarf for bpf_strnchr.constprop.0. >> (For this particular .constprop.0 case, it is possible to derive the >> signature. but it will be hard for other suffixes like .isra). >> The current pahole will have strip out suffixes so the function >> name is 'bpf_strnchr' which covers bpf_strnchr and bpf_strnchr.constprop.0. >> Since two underlying signature is different, the 'bpf_strnchr' >> will be filtered out. > > Yes, I understand the mechanics. My question is: is it really > necessary for pahole to go through this process? > > It sees two functions: 'bpf_strnchr', 'bpf_strnchr.constprop.0', > first global, second local, first with DWARF signature, second w/o > DWARF signature. So, why conflating the two? > > For non-lto build the function being global guarantees signature > correctness, and below you confirm that it is the case for lto builds > as well. So, it looks like we are just loosing 'bpf_strnchr' for no > good reason. > I'm working on a small 2-patch series at the moment to improve this. The problem is we currently have no way to associate the DWARF with the relevant ELF function; DWARF representations of functions do not have "." suffixes either so we are just matching by name prefix when we collect DWARF info about a particular function. The series I'm working on uses DWARF addresses to improve the DWARF/ELF association, ensuring that we don't toss functions that look inconsistent but just have .part or .cold suffixed components that have non-matching DWARF function signatures. ".constprop" isn't covered yet however. >> I am actually working to improve such cases in llvm to address >> like foo() and foo.<...>() functions and they will have their >> own respective functions. We will discuss with gcc folks >> about how to implement similar approaches in gcc. >> >>> >>> Since kfuncs are global, this should guarantee that the compiler does not >>> change their signature, correct? Does this also hold for LTO builds? >> >> Yes, the original signature will not changed. This holds for LTO build >> and global variables/functions will not be renamed. >> >>> If so, when pahole sees a set of symbols like [foo, foo.1, foo.2, ...], >> >> The compiler needs to emit the signature in dwarf for foo.1, foo.2, etc. and this >> is something I am working on. >> >>> with 'foo' being global and the rest local, then there is no real need >>> to filter out 'foo'. >> >> I think the current __noclone approach is okay as the full implementation >> for signature changes (foo, foo.1, ...) might takes a while for both llvm >> and gcc. >> >>> >>> Wdyt? >>> >>> [...]
