On Fri, Jan 03, 2025 at 12:10:08PM +0100, Jiri Olsa wrote: > On Thu, Jan 02, 2025 at 01:58:59PM -0500, Steven Rostedt wrote: > > From: Steven Rostedt <rostedt@xxxxxxxxxxx> > > > > When a function is annotated as "weak" and is overridden, the code is not > > removed. If it is traced, the fentry/mcount location in the weak function > > will be referenced by the "__mcount_loc" section. This will then be added > > to the available_filter_functions list. Since only the address of the > > functions are listed, to find the name to show, a search of kallsyms is > > used. > > > > Since kallsyms will return the function by simply finding the function > > that the address is after but before the next function, an address of a > > weak function will show up as the function before it. This is because > > kallsyms does not save names of weak functions. This has caused issues in > > the past, as now the traced weak function will be listed in > > available_filter_functions with the name of the function before it. > > > > At best, this will cause the previous function's name to be listed twice. > > At worse, if the previous function was marked notrace, it will now show up > > as a function that can be traced. Note that it only shows up that it can > > be traced but will not be if enabled, which causes confusion. > > > > https://lore.kernel.org/all/20220412094923.0abe90955e5db486b7bca279@xxxxxxxxxx/ > > > > The commit b39181f7c6907 ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid > > adding weak function") was a workaround to this by checking the function > > address before printing its name. If the address was too far from the > > function given by the name then instead of printing the name it would > > print: __ftrace_invalid_address___<invalid-offset> > > > > The real issue is that these invalid addresses are listed in the ftrace > > table look up which available_filter_functions is derived from. A place > > holder must be listed in that file because set_ftrace_filter may take a > > series of indexes into that file instead of names to be able to do O(1) > > lookups to enable filtering (many tools use this method). > > > > Even if kallsyms saved the size of the function, it does not remove the > > need of having these place holders. The real solution is to not add a weak > > function into the ftrace table in the first place. > > > > To solve this, the sorttable.c code that sorts the mcount regions during > > the build is modified to take a "nm -S vmlinux" input, sort it, and any > > function listed in the mcount_loc section that is not within a boundary of > > the function list given by nm is considered a weak function and is zeroed > > out. Note, this does not mean they will remain zero when booting as KASLR > > will still shift those addresses. > > hi, > fyi this seems to remove several functions from available_filter_functions, > that bpf relay on.. like update_socket_protocol or bpf_rstat_flush: > > __bpf_hook_start(); > > __weak noinline int update_socket_protocol(int family, int type, int protocol) > { > return protocol; > } > > __bpf_hook_end(); > > > [root@qemu-1 tracing]# cat available_filter_functions | grep update_socket_protocol > [root@qemu-1 tracing]# cat /proc/kallsyms | grep update_socket_protocol > ffffffff821d58b0 W __pfx_update_socket_protocol > ffffffff821d58c0 W update_socket_protocol > > not sure why that fits the condition above for removal Check your build, if update_socket_protocol() is no longer in the symbol table for your vmlinux.o then the linker deleted the symbol and things work as advertised. If its still there, these patches have a wobbly.
