Hi. Le mercredi 27 septembre 2023, 20:35:16 EEST Alessandro Carminati (Red Hat) a écrit : > It is not uncommon for drivers or modules related to similar peripherals > to have symbols with the exact same name. > While this is not a problem for the kernel's binary itself, it becomes an > issue when attempting to trace or probe specific functions using > infrastructure like ftrace or kprobe. > > The tracing subsystem relies on the `nm -n vmlinux` output, which provides > symbol information from the kernel's ELF binary. However, when multiple > symbols share the same name, the standard nm output does not differentiate > between them. This can lead to confusion and difficulty when trying to > probe the intended symbol. > > ~ # cat /proc/kallsyms | grep " name_show" > ffffffff8c4f76d0 t name_show > ffffffff8c9cccb0 t name_show > ffffffff8cb0ac20 t name_show > ffffffff8cc728c0 t name_show > ffffffff8ce0efd0 t name_show > ffffffff8ce126c0 t name_show > ffffffff8ce1dd20 t name_show > ffffffff8ce24e70 t name_show > ffffffff8d1104c0 t name_show > ffffffff8d1fe480 t name_show > > kas_alias addresses this challenge by enhancing symbol names with > meaningful suffixes generated from the source file and line number > during the kernel build process. > These newly generated aliases provide tracers with the ability to > comprehend the symbols they are interacting with when utilizing the > ftracefs interface. > This approach may also allow for the probing by name of previously > inaccessible symbols. > > ~ # cat /proc/kallsyms | grep gic_mask_irq > ffffd15671e505ac t gic_mask_irq > ffffd15671e505ac t gic_mask_irq@drivers_irqchip_irq_gic_c_167 > ffffd15671e532a4 t gic_mask_irq > ffffd15671e532a4 t gic_mask_irq@drivers_irqchip_irq_gic_v3_c_407 > ~ # > > Changes from v1: > - Integrated changes requested by Masami to exclude symbols with prefixes > "_cfi" and "_pfx". > - Introduced a small framework to handle patterns that need to be excluded > from the alias production. > - Excluded other symbols using the framework. > - Introduced the ability to discriminate between text and data symbols. > - Added two new config symbols in this version: CONFIG_KALLSYMS_ALIAS_DATA, > which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which > excludes all filters and provides an alias for each duplicated symbol. > > https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carminati@gm > ail.com/ > > Changes from v2: > - Alias tags are created by querying DWARF information from the vmlinux. > - The filename + line number is normalized and appended to the original > name. > - The tag begins with '@' to indicate the symbol source. > - Not a change, but worth mentioning, since the alias is added to the > existing list, the old duplicated name is preserved, and the livepatch > way of dealing with duplicates is maintained. > - Acknowledging the existence of scenarios where inlined functions > declared in header files may result in multiple copies due to compiler > behavior, though it is not actionable as it does not pose an operational > issue. > - Highlighting a single exception where the same name refers to different > functions: the case of "compat_binfmt_elf.c," which directly includes > "binfmt_elf.c" producing identical function copies in two separate > modules. > > https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carminati@gm > ail.com/ > > Changes from v3: > - kas_alias was rewritten in Python to create a more concise and > maintainable codebase. > - The previous automation process used by kas_alias to locate the vmlinux > and the addr2line has been replaced with an explicit command-line switch > for specifying these requirements. > - addr2line has been added into the main Makefile. > - A new command-line switch has been introduced, enabling users to extend > the alias to global data names. > > https://lore.kernel.org/all/20230828080423.3539686-1-alessandro.carminati@gm > ail.com/ > > Changes from v4: > - Fixed the O=<build dir> build issue > - The tool halts execution upon encountering major issues, thereby ensuring > the pipeline is interrupted. > - A cmdline option to specify the source directory added. > - Minor code adjusments. > - Tested on mips32 and i386
Thank you for sending this new version! I only found nits and I think we are near its merge! > https://lore.kernel.org/all/20230919193948.465340-1-alessandro.carminati@gma > il.com/ > > NOTE: > About the symbols name duplication that happens as consequence of the > inclusion compat_binfmt_elf.c does, it is evident that this corner is > inherently challenging the addr2line approach. > Attempting to conceal this limitation would be counterproductive. > > compat_binfmt_elf.c includes directly binfmt_elf.c, addr2line can't help > but report all functions and data declared by that file, coming from > binfmt_elf.c. > > My position is that, rather than producing a more complicated pipeline > to handle this corner case, it is better to fix the compat_binfmt_elf.c > anomaly. > > This patch does not deal with the two potentially problematic symbols > defined by compat_binfmt_elf.c > > Signed-off-by: Alessandro Carminati (Red Hat) > <alessandro.carmin...@gmail.com> --- > Makefile | 4 +- > init/Kconfig | 22 +++++++ > scripts/kas_alias.py | 136 ++++++++++++++++++++++++++++++++++++++++ > scripts/link-vmlinux.sh | 21 ++++++- > 4 files changed, 180 insertions(+), 3 deletions(-) > create mode 100755 scripts/kas_alias.py > > diff --git a/Makefile b/Makefile > index 4f283d915e54..f33c179f4cc3 100644 > --- a/Makefile > +++ b/Makefile > @@ -488,6 +488,7 @@ OBJCOPY = $(LLVM_PREFIX)llvm-objcopy$ (LLVM_SUFFIX) > OBJDUMP = $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX) > READELF = $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX) > STRIP = $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX) > +ADDR2LINE = $(LLVM_PREFIX)llvm-addr2line$(LLVM_SUFFIX) > else > CC = $(CROSS_COMPILE)gcc > LD = $(CROSS_COMPILE)ld > @@ -497,6 +498,7 @@ OBJCOPY = $(CROSS_COMPILE)objcopy > OBJDUMP = $(CROSS_COMPILE)objdump > READELF = $(CROSS_COMPILE)readelf > STRIP = $(CROSS_COMPILE)strip > +ADDR2LINE = $(CROSS_COMPILE)addr2line > endif > RUSTC = rustc > RUSTDOC = rustdoc > @@ -611,7 +613,7 @@ export RUSTC_BOOTSTRAP := 1 > export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS > CROSS_COMPILE LD CC HOSTPKG_CONFIG export RUSTC RUSTDOC RUSTFMT > RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN CARGO export HOSTRUSTC > KBUILD_HOSTRUSTFLAGS > -export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX > YACC AWK INSTALLKERNEL +export CPP AR NM STRIP OBJCOPY OBJDUMP READELF > ADDR2LINE PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL export PERL > PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX > export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD > export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS > LDFLAGS_MODULE diff --git a/init/Kconfig b/init/Kconfig > index 6d35728b94b2..d45dd423e1ec 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -1738,6 +1738,28 @@ config KALLSYMS_BASE_RELATIVE > time constants, and no relocation pass is required at runtime to fix > up the entries based on the runtime load address of the kernel. > > +config KALLSYMS_ALIAS_SRCLINE > + bool "Produces alias for duplicated text symbols" if EXPERT > + depends on KALLSYMS && DEBUG_INFO && !DEBUG_INFO_SPLIT > + help > + It is not uncommon for drivers or modules related to similar > + peripherals to have symbols with the exact same name. > + While this is not a problem for the kernel's binary itself, it > + becomes an issue when attempting to trace or probe specific > + functions using infrastructure like ftrace or kprobe. > + > + This option addresses this challenge, producing alias for text > + symbol names that include the file name and line where the symbols > + are defined in the source code. > + > +config KALLSYMS_ALIAS_SRCLINE_DATA > + bool "Produces alias also for global variables names" > + depends on KALLSYMS_ALIAS_SRCLINE > + help > + Sometimes it can be useful to refer to global vars by name. Since > + they suffer the same issue as text symbols, this config option > + allows having aliases for global variables names too. > + > # end of the "standard kernel features (expert users)" menu > > # syscall, maps, verifier > diff --git a/scripts/kas_alias.py b/scripts/kas_alias.py > new file mode 100755 > index 000000000000..7c6b7045081c > --- /dev/null > +++ b/scripts/kas_alias.py > @@ -0,0 +1,136 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Copyright (C) 2023 Red Hat, Inc. Alessandro Carminati > <alessandro.carmin...@gmail.com> +# > +# kas_alias: Adds alias to duplicate symbols in the kallsyms output. > + > +import subprocess > +import sys > +import os > +import argparse > +import re > +from collections import namedtuple > + > +regex_filter = [ > + "^__compound_literal\\.[0-9]+$", > + "^__[wm]*key\\.[0-9]+$", > + "^_*TRACE_SYSTEM.*$", > + "^__already_done\\.[0-9]+$", > + "^__msg\\.[0-9]+$", > + "^__func__\\.[0-9]+$", > + "^CSWTCH\\.[0-9]+$", > + "^_rs\\.[0-9]+$", > + "^___tp_str\\.[0-9]+$", > + "^__flags\\.[0-9]+$", > + "^___done\\.[0-9]+$", > + "^__print_once\\.[0-9]+$", > + "^___once_key\\.[0-9]+$", > + "^__pfx_.*$", > + "^__cfi_.*$" > + ] > + > +class SeparatorType: > + def __call__(self, separator): > + if len(separator) != 1: > + raise argparse.ArgumentTypeError("Separator must be a single > character") + return separator > + > +Line = namedtuple('Line', ['address', 'type', 'name']) > + > +def parse_file(filename): > + symbol_list = [] > + name_occurrences = {} > + > + with open(filename, 'r') as file: > + for line in file: > + fields = line.strip().split() > + > + if len(fields) >= 3: > + address, type, name = fields[0], fields[1], ' > '.join(fields[2:]) + symbol_list.append(Line(address, type, > name)) > + name_occurrences[name] = name_occurrences.get(name, 0) + 1 > + > + return symbol_list, name_occurrences > + > +def find_duplicate(symbol_list, name_occurrences): This function is never used. > + name_to_lines = {} > + duplicate_lines = [] > + > + for line in symbol_list: > + if line.name in name_to_lines: > + first_occurrence = name_to_lines[line.name] > + duplicate_lines.extend([first_occurrence, line]) > + else: > + name_to_lines[line.name] = line > + > + return duplicate_lines > + > +def start_addr2line_process(binary_file, addr2line_file): > + try: > + addr2line_process = subprocess.Popen([addr2line_file, '-fe', > binary_file], + > stdin=subprocess.PIPE, + > stdout=subprocess.PIPE, + > stderr=subprocess.PIPE, + > text=True) > + return addr2line_process > + except Exception as e: > + print(f"Error starting addr2line process: {str(e)}") > + sys.exit(1) As you already have a try/except block code in your main function, I think it is better to handle all exception here rather than inside each function. > + > +def addr2line_fetch_address(addr2line_process, address): > + try: > + addr2line_process.stdin.write(address + '\n') > + addr2line_process.stdin.flush() > + addr2line_process.stdout.readline().strip() > + output = addr2line_process.stdout.readline().strip() > + > + return os.path.normpath(output) > + except Exception as e: > + print(f"Error communicating with addr2line: {str(e)}") > + sys.exit(1) Same comment as above. > +def process_line(obj, config): > + if config: > + return not (any(re.match(regex, obj.name) for regex in > regex_filter)) + else: > + return obj.type in {"T", "t"} > + > +if __name__ == "__main__": > + parser = argparse.ArgumentParser(description='Add alias to multiple > occurring symbols name in kallsyms') + parser.add_argument('-a', > "--addr2line", dest="addr2line_file", required=True) + > parser.add_argument('-v', "--vmlinux", dest="vmlinux_file", required=True) > + parser.add_argument('-o', "--outfile", dest="output_file", > required=True) + parser.add_argument('-n', "--nmdata", > dest="nm_data_file", required=True) + parser.add_argument('-b', > "--basedir", dest="linux_base_dir", required=True) + > parser.add_argument('-s', "--separator", dest="separator", required=False, > default="@", type=SeparatorType()) + parser.add_argument('-d', "--data", > dest="include_data", required=False, action='store_true') + config = > parser.parse_args() > + > + try: > + config.linux_base_dir = os.path.normpath(os.getcwd() + "/" + > config.linux_base_dir) + "/" + symbol_list, name_occurrences = > parse_file(config.nm_data_file) + addr2line_process = > start_addr2line_process(config.vmlinux_file, config.addr2line_file) + > + with open(config.output_file, 'w') as file: > + for obj in symbol_list: > + file.write(f"{obj.address} {obj.type} {obj.name}\n") > + if (name_occurrences[obj.name] > 1) and process_line(obj, > config.include_data) : + output = > addr2line_fetch_address(addr2line_process, obj.address) + > decoration = config.separator + "".join( > + "_" if not c.isalnum() else c for c in > output.replace(config.linux_base_dir, "") + ) > + # The addr2line can emit the special string "?:??" when > addr2line can not find the + # specified address in the > DWARF section that after normalization it becomes "____". + > # In such cases, emitting an alias wouldn't make sense, so it is > skipped. Thank you for the comment! > + if decoration != config.separator + "____": > + file.write(f"{obj.address} {obj.type} {obj.name + > decoration}\n") + > + addr2line_process.stdin.close() > + addr2line_process.stdout.close() > + addr2line_process.stderr.close() > + addr2line_process.wait() > + > + except Exception as e: > + print(f"An error occurred: {str(e)}") > + raise SystemExit("Script terminated due to an error") > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index a432b171be82..c110b0f58a19 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -91,7 +91,12 @@ vmlinux_link() > > # The kallsyms linking does not need debug symbols included. > if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then > - ldflags="${ldflags} ${wl}--strip-debug" > + # The kallsyms linking does not need debug symbols included, > + # unless the KALLSYMS_ALIAS_SRCLINE. > + if ! is_enabled CONFIG_KALLSYMS_ALIAS_SRCLINE && \ > + [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then > + ldflags="${ldflags} ${wl}--strip-debug" > + fi > fi > > if is_enabled CONFIG_VMLINUX_MAP; then > @@ -161,7 +166,19 @@ kallsyms() > fi > > info KSYMS ${2} > - scripts/kallsyms ${kallsymopt} ${1} > ${2} > + ALIAS="" > + KAS_DATA="" > + if is_enabled CONFIG_KALLSYMS_ALIAS_SRCLINE_DATA; then > + KAS_DATA="--data" > + fi > + if is_enabled CONFIG_KALLSYMS_ALIAS_SRCLINE; then > + ALIAS=".alias" > + ${srctree}/scripts/kas_alias.py \ > + --addr2line ${ADDR2LINE} --vmlinux ${kallsyms_vmlinux} \ > + --nmdata ${1} --outfile ${1}${ALIAS} \ > + --basedir ${srctree} --separator @ ${KAS_DATA} > + fi > + scripts/kallsyms ${kallsymopt} ${1}${ALIAS} > ${2} > } > > # Perform one step in kallsyms generation, including temporary linking of Best regards.
