From 39cf650d68289d41d484f4c29fea0124df2e09aa Mon Sep 17 00:00:00 2001 From: Brian Johannesmeyer Date: Mon, 15 Apr 2024 16:55:32 +0200 Subject: scripts/faddr2line: Reduce number of readelf calls to three Rather than calling readelf several times for each invocation of __faddr2line, call readelf only three times at the beginning, and save its result for future use. Signed-off-by: Brian Johannesmeyer Link: https://lore.kernel.org/r/20240415145538.1938745-2-bjohannesmeyer@gmail.com Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index 587415a52b6f..bf394bfd526a 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -87,7 +87,7 @@ command -v ${ADDR2LINE} >/dev/null 2>&1 || die "${ADDR2LINE} isn't installed" find_dir_prefix() { local objfile=$1 - local start_kernel_addr=$(${READELF} --symbols --wide $objfile | sed 's/\[.*\]//' | + local start_kernel_addr=$(echo "${ELF_SYMS}" | sed 's/\[.*\]//' | ${AWK} '$8 == "start_kernel" {printf "0x%s", $2}') [[ -z $start_kernel_addr ]] && return @@ -103,6 +103,14 @@ find_dir_prefix() { return 0 } +run_readelf() { + local objfile=$1 + + ELF_FILEHEADER=$(${READELF} --file-header $objfile) + ELF_SECHEADERS=$(${READELF} --section-headers --wide $objfile) + ELF_SYMS=$(${READELF} --symbols --wide $objfile) +} + __faddr2line() { local objfile=$1 local func_addr=$2 @@ -125,7 +133,7 @@ __faddr2line() { # vmlinux uses absolute addresses in the section table rather than # section offsets. - local file_type=$(${READELF} --file-header $objfile | + local file_type=$(echo "${ELF_FILEHEADER}" | ${AWK} '$1 == "Type:" { print $2; exit }') if [[ $file_type = "EXEC" ]] || [[ $file_type == "DYN" ]]; then is_vmlinux=1 @@ -143,8 +151,7 @@ __faddr2line() { local sec_name # Get the section size: - sec_size=$(${READELF} --section-headers --wide $objfile | - sed 's/\[ /\[/' | + sec_size=$(echo "${ELF_SECHEADERS}" | sed 's/\[ /\[/' | ${AWK} -v sec=$sym_sec '$1 == "[" sec "]" { print "0x" $6; exit }') if [[ -z $sec_size ]]; then @@ -154,8 +161,7 @@ __faddr2line() { fi # Get the section name: - sec_name=$(${READELF} --section-headers --wide $objfile | - sed 's/\[ /\[/' | + sec_name=$(echo "${ELF_SECHEADERS}" | sed 's/\[ /\[/' | ${AWK} -v sec=$sym_sec '$1 == "[" sec "]" { print $2; exit }') if [[ -z $sec_name ]]; then @@ -197,7 +203,7 @@ __faddr2line() { found=2 break fi - done < <(${READELF} --symbols --wide $objfile | sed 's/\[.*\]//' | ${AWK} -v sec=$sym_sec '$7 == sec' | sort --key=2) + done < <(echo "${ELF_SYMS}" | sed 's/\[.*\]//' | ${AWK} -v sec=$sym_sec '$7 == sec' | sort --key=2) if [[ $found = 0 ]]; then warn "can't find symbol: sym_name: $sym_name sym_sec: $sym_sec sym_addr: $sym_addr sym_elf_size: $sym_elf_size" @@ -278,7 +284,7 @@ __faddr2line() { DONE=1 - done < <(${READELF} --symbols --wide $objfile | sed 's/\[.*\]//' | ${AWK} -v fn=$sym_name '$8 == fn') + done < <(echo "${ELF_SYMS}" | sed 's/\[.*\]//' | ${AWK} -v fn=$sym_name '$8 == fn') } [[ $# -lt 2 ]] && usage @@ -291,7 +297,9 @@ LIST=0 [[ ! -f $objfile ]] && die "can't find objfile $objfile" shift -${READELF} --section-headers --wide $objfile | ${GREP} -q '\.debug_info' || die "CONFIG_DEBUG_INFO not enabled" +run_readelf $objfile + +echo "${ELF_SECHEADERS}" | ${GREP} -q '\.debug_info' || die "CONFIG_DEBUG_INFO not enabled" DIR_PREFIX=supercalifragilisticexpialidocious find_dir_prefix $objfile -- cgit v1.2.3-58-ga151 From b8d9d9496c1e78a8fd89f4fe9923d12b3c9ad8a3 Mon Sep 17 00:00:00 2001 From: Brian Johannesmeyer Date: Mon, 15 Apr 2024 16:55:33 +0200 Subject: scripts/faddr2line: Combine three readelf calls into one Rather than calling readelf three separate times to collect three different types of info, call it only once, and parse out the different types of info from its output. Signed-off-by: Brian Johannesmeyer Link: https://lore.kernel.org/r/20240415145538.1938745-3-bjohannesmeyer@gmail.com Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index bf394bfd526a..f011bda4ed25 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -105,10 +105,14 @@ find_dir_prefix() { run_readelf() { local objfile=$1 - - ELF_FILEHEADER=$(${READELF} --file-header $objfile) - ELF_SECHEADERS=$(${READELF} --section-headers --wide $objfile) - ELF_SYMS=$(${READELF} --symbols --wide $objfile) + local out=$(${READELF} --file-header --section-headers --symbols --wide $objfile) + + # This assumes that readelf first prints the file header, then the section headers, then the symbols. + # Note: It seems that GNU readelf does not prefix section headers with the "There are X section headers" + # line when multiple options are given, so let's also match with the "Section Headers:" line. + ELF_FILEHEADER=$(echo "${out}" | sed -n '/There are [0-9]* section headers, starting at offset\|Section Headers:/q;p') + ELF_SECHEADERS=$(echo "${out}" | sed -n '/There are [0-9]* section headers, starting at offset\|Section Headers:/,$p' | sed -n '/Symbol table .* contains [0-9]* entries:/q;p') + ELF_SYMS=$(echo "${out}" | sed -n '/Symbol table .* contains [0-9]* entries:/,$p') } __faddr2line() { -- cgit v1.2.3-58-ga151 From 2c809186ccf0e3a4cb952da181f9c28436133081 Mon Sep 17 00:00:00 2001 From: Brian Johannesmeyer Date: Mon, 15 Apr 2024 16:55:34 +0200 Subject: scripts/faddr2line: Check vmlinux only once Rather than checking whether the object file is vmlinux for each invocation of __faddr2line, check it only once beforehand. Signed-off-by: Brian Johannesmeyer Link: https://lore.kernel.org/r/20240415145538.1938745-4-bjohannesmeyer@gmail.com Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index f011bda4ed25..bb3b5f03f4ea 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -115,6 +115,17 @@ run_readelf() { ELF_SYMS=$(echo "${out}" | sed -n '/Symbol table .* contains [0-9]* entries:/,$p') } +check_vmlinux() { + # vmlinux uses absolute addresses in the section table rather than + # section offsets. + IS_VMLINUX=0 + local file_type=$(echo "${ELF_FILEHEADER}" | + ${AWK} '$1 == "Type:" { print $2; exit }') + if [[ $file_type = "EXEC" ]] || [[ $file_type == "DYN" ]]; then + IS_VMLINUX=1 + fi +} + __faddr2line() { local objfile=$1 local func_addr=$2 @@ -125,8 +136,6 @@ __faddr2line() { local func_offset=${func_addr#*+} func_offset=${func_offset%/*} local user_size= - local file_type - local is_vmlinux=0 [[ $func_addr =~ "/" ]] && user_size=${func_addr#*/} if [[ -z $sym_name ]] || [[ -z $func_offset ]] || [[ $sym_name = $func_addr ]]; then @@ -135,14 +144,6 @@ __faddr2line() { return fi - # vmlinux uses absolute addresses in the section table rather than - # section offsets. - local file_type=$(echo "${ELF_FILEHEADER}" | - ${AWK} '$1 == "Type:" { print $2; exit }') - if [[ $file_type = "EXEC" ]] || [[ $file_type == "DYN" ]]; then - is_vmlinux=1 - fi - # Go through each of the object's symbols which match the func name. # In rare cases there might be duplicates, in which case we print all # matches. @@ -260,7 +261,7 @@ __faddr2line() { # Pass section address to addr2line and strip absolute paths # from the output: local args="--functions --pretty-print --inlines --exe=$objfile" - [[ $is_vmlinux = 0 ]] && args="$args --section=$sec_name" + [[ $IS_VMLINUX = 0 ]] && args="$args --section=$sec_name" local output=$(${ADDR2LINE} $args $addr | sed "s; $dir_prefix\(\./\)*; ;") [[ -z $output ]] && continue @@ -305,6 +306,8 @@ run_readelf $objfile echo "${ELF_SECHEADERS}" | ${GREP} -q '\.debug_info' || die "CONFIG_DEBUG_INFO not enabled" +check_vmlinux + DIR_PREFIX=supercalifragilisticexpialidocious find_dir_prefix $objfile -- cgit v1.2.3-58-ga151 From 5b280de46d2bcea9def0dd84b1e86f8b42ca70b9 Mon Sep 17 00:00:00 2001 From: Brian Johannesmeyer Date: Mon, 15 Apr 2024 16:55:35 +0200 Subject: scripts/faddr2line: Pass --addresses argument to addr2line In preparation for identifying an addr2line sentinel. See previous work [0], which applies a similar change to perf. [0] commit 8dc26b6f718a ("perf srcline: Make sentinel reading for binutils addr2line more robust") Signed-off-by: Brian Johannesmeyer Link: https://lore.kernel.org/r/20240415145538.1938745-5-bjohannesmeyer@gmail.com Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index bb3b5f03f4ea..820680c59a39 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -260,9 +260,12 @@ __faddr2line() { # Pass section address to addr2line and strip absolute paths # from the output: - local args="--functions --pretty-print --inlines --exe=$objfile" + local args="--functions --pretty-print --inlines --addresses --exe=$objfile" [[ $IS_VMLINUX = 0 ]] && args="$args --section=$sec_name" - local output=$(${ADDR2LINE} $args $addr | sed "s; $dir_prefix\(\./\)*; ;") + local output_with_addr=$(${ADDR2LINE} $args $addr | sed "s; $dir_prefix\(\./\)*; ;") + [[ -z $output_with_addr ]] && continue + + local output=$(echo "${output_with_addr}" | sed 's/^0x[0-9a-fA-F]*: //') [[ -z $output ]] && continue # Default output (non --list): -- cgit v1.2.3-58-ga151 From e36b69e918112430ee53e24238bb87f5146f9acf Mon Sep 17 00:00:00 2001 From: Brian Johannesmeyer Date: Mon, 15 Apr 2024 16:55:36 +0200 Subject: scripts/faddr2line: Invoke addr2line as a single long-running process Rather than invoking a separate addr2line process for each address, invoke a single addr2line coprocess, and pass each address to its stdin. Previous work [0] applied a similar change to perf, leading to a ~60x speed-up [1]. If using an object file that is _not_ vmlinux, faddr2line passes a section name argument to addr2line. Because we do not know until runtime which section names will be passed to addr2line, we cannot apply this change to non-vmlinux object files. Hence, it only applies to vmlinux. [0] commit be8ecc57f180 ("perf srcline: Use long-running addr2line per DSO") [1] Link: https://eighty-twenty.org/2021/09/09/perf-addr2line-speed-improvement Signed-off-by: Brian Johannesmeyer Link: https://lore.kernel.org/r/20240415145538.1938745-6-bjohannesmeyer@gmail.com Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 52 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index 820680c59a39..48fc8cfc80df 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -126,6 +126,48 @@ check_vmlinux() { fi } +init_addr2line() { + local objfile=$1 + + check_vmlinux + + ADDR2LINE_ARGS="--functions --pretty-print --inlines --addresses --exe=$objfile" + if [[ $IS_VMLINUX = 1 ]]; then + # If the executable file is vmlinux, we don't pass section names to + # addr2line, so we can launch it now as a single long-running process. + coproc ADDR2LINE_PROC (${ADDR2LINE} ${ADDR2LINE_ARGS}) + fi +} + +run_addr2line() { + local addr=$1 + local sec_name=$2 + + if [[ $IS_VMLINUX = 1 ]]; then + # We send to the addr2line process: (1) the address, then (2) a sentinel + # value, i.e., something that can't be interpreted as a valid address + # (i.e., ","). This causes addr2line to write out: (1) the answer for + # our address, then (2) either "?? ??:0" or "0x0...0: ..." (if + # using binutils' addr2line), or "," (if using LLVM's addr2line). + echo ${addr} >& "${ADDR2LINE_PROC[1]}" + echo "," >& "${ADDR2LINE_PROC[1]}" + local first_line + read -r first_line <& "${ADDR2LINE_PROC[0]}" + ADDR2LINE_OUT=$(echo "${first_line}" | sed 's/^0x[0-9a-fA-F]*: //') + while read -r line <& "${ADDR2LINE_PROC[0]}"; do + if [[ "$line" == "?? ??:0" ]] || [[ "$line" == "," ]] || [[ $(echo "$line" | ${GREP} "^0x00*: ") ]]; then + break + fi + ADDR2LINE_OUT+=$'\n'$(echo "$line" | sed 's/^0x[0-9a-fA-F]*: //') + done + else + # Run addr2line as a single invocation. + local sec_arg + [[ -z $sec_name ]] && sec_arg="" || sec_arg="--section=${sec_name}" + ADDR2LINE_OUT=$(${ADDR2LINE} ${ADDR2LINE_ARGS} ${sec_arg} ${addr} | sed 's/^0x[0-9a-fA-F]*: //') + fi +} + __faddr2line() { local objfile=$1 local func_addr=$2 @@ -260,12 +302,8 @@ __faddr2line() { # Pass section address to addr2line and strip absolute paths # from the output: - local args="--functions --pretty-print --inlines --addresses --exe=$objfile" - [[ $IS_VMLINUX = 0 ]] && args="$args --section=$sec_name" - local output_with_addr=$(${ADDR2LINE} $args $addr | sed "s; $dir_prefix\(\./\)*; ;") - [[ -z $output_with_addr ]] && continue - - local output=$(echo "${output_with_addr}" | sed 's/^0x[0-9a-fA-F]*: //') + run_addr2line $addr $sec_name + local output=$(echo "${ADDR2LINE_OUT}" | sed "s; $dir_prefix\(\./\)*; ;") [[ -z $output ]] && continue # Default output (non --list): @@ -309,7 +347,7 @@ run_readelf $objfile echo "${ELF_SECHEADERS}" | ${GREP} -q '\.debug_info' || die "CONFIG_DEBUG_INFO not enabled" -check_vmlinux +init_addr2line $objfile DIR_PREFIX=supercalifragilisticexpialidocious find_dir_prefix $objfile -- cgit v1.2.3-58-ga151 From 406b5c12aad8110e1b1f9355f176cac43cd1fecb Mon Sep 17 00:00:00 2001 From: Brian Johannesmeyer Date: Mon, 15 Apr 2024 16:55:37 +0200 Subject: scripts/faddr2line: Remove call to addr2line from find_dir_prefix() Use the single long-running faddr2line process from find_dir_prefix(). Signed-off-by: Brian Johannesmeyer Link: https://lore.kernel.org/r/20240415145538.1938745-7-bjohannesmeyer@gmail.com Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index 48fc8cfc80df..1fa6beef9f97 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -85,15 +85,17 @@ command -v ${ADDR2LINE} >/dev/null 2>&1 || die "${ADDR2LINE} isn't installed" # init/main.c! This only works for vmlinux. Otherwise it falls back to # printing the absolute path. find_dir_prefix() { - local objfile=$1 - local start_kernel_addr=$(echo "${ELF_SYMS}" | sed 's/\[.*\]//' | ${AWK} '$8 == "start_kernel" {printf "0x%s", $2}') [[ -z $start_kernel_addr ]] && return - local file_line=$(${ADDR2LINE} -e $objfile $start_kernel_addr) - [[ -z $file_line ]] && return + run_addr2line ${start_kernel_addr} "" + [[ -z $ADDR2LINE_OUT ]] && return + local file_line=${ADDR2LINE_OUT#* at } + if [[ -z $file_line ]] || [[ $file_line = $ADDR2LINE_OUT ]]; then + return + fi local prefix=${file_line%init/main.c:*} if [[ -z $prefix ]] || [[ $prefix = $file_line ]]; then return @@ -350,7 +352,7 @@ echo "${ELF_SECHEADERS}" | ${GREP} -q '\.debug_info' || die "CONFIG_DEBUG_INFO n init_addr2line $objfile DIR_PREFIX=supercalifragilisticexpialidocious -find_dir_prefix $objfile +find_dir_prefix FIRST=1 while [[ $# -gt 0 ]]; do -- cgit v1.2.3-58-ga151 From c02904f05ff805d6c0631634d5751ebd338f75ec Mon Sep 17 00:00:00 2001 From: Brian Johannesmeyer Date: Mon, 15 Apr 2024 16:55:38 +0200 Subject: scripts/faddr2line: Check only two symbols when calculating symbol size Rather than looping through each symbol in a particular section to calculate a symbol's size, grep for the symbol and its immediate successor, and only use those two symbols. Signed-off-by: Brian Johannesmeyer Link: https://lore.kernel.org/r/20240415145538.1938745-8-bjohannesmeyer@gmail.com Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index 1fa6beef9f97..fe0cc45f03be 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -252,7 +252,7 @@ __faddr2line() { found=2 break fi - done < <(echo "${ELF_SYMS}" | sed 's/\[.*\]//' | ${AWK} -v sec=$sym_sec '$7 == sec' | sort --key=2) + done < <(echo "${ELF_SYMS}" | sed 's/\[.*\]//' | ${AWK} -v sec=$sym_sec '$7 == sec' | sort --key=2 | ${GREP} -A1 --no-group-separator " ${sym_name}$") if [[ $found = 0 ]]; then warn "can't find symbol: sym_name: $sym_name sym_sec: $sym_sec sym_addr: $sym_addr sym_elf_size: $sym_elf_size" -- cgit v1.2.3-58-ga151 From b13e9f6da4cc34240dae05418b9876b2758ebe35 Mon Sep 17 00:00:00 2001 From: Siddh Raman Pant Date: Fri, 10 May 2024 15:02:57 +0530 Subject: objtool: Use "action" in error message to be consistent with help The help message mentions the main options as "actions", which is different from the optional "options". But the check error messages outputs "option" or "command" for referring to actions. Make the error messages consistent with help. Signed-off-by: Siddh Raman Pant Signed-off-by: Josh Poimboeuf --- tools/objtool/builtin-check.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index 5e21cfb7661d..387d56a7f5fb 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -144,7 +144,7 @@ static bool opts_valid(void) opts.static_call || opts.uaccess) { if (opts.dump_orc) { - ERROR("--dump can't be combined with other options"); + ERROR("--dump can't be combined with other actions"); return false; } @@ -159,7 +159,7 @@ static bool opts_valid(void) if (opts.dump_orc) return true; - ERROR("At least one command required"); + ERROR("At least one action required"); return false; } -- cgit v1.2.3-58-ga151 From 8e366d83edce3065ff3372bedc281c5e217c0550 Mon Sep 17 00:00:00 2001 From: Alexandre Chartre Date: Thu, 20 Jun 2024 16:47:47 +0200 Subject: objtool/x86: objtool can confuse memory and stack access The encoding of an x86 instruction can include a ModR/M and a SIB (Scale-Index-Base) byte to describe the addressing mode of the instruction. objtool processes all addressing mode with a SIB base of 5 as having %rbp as the base register. However, a SIB base of 5 means that the effective address has either no base (if ModR/M mod is zero) or %rbp as the base (if ModR/M mod is 1 or 2). This can cause objtool to confuse an absolute address access with a stack operation. For example, objtool will see the following instruction: 4c 8b 24 25 e0 ff ff mov 0xffffffffffffffe0,%r12 as a stack operation (i.e. similar to: mov -0x20(%rbp), %r12). [Note that this kind of weird absolute address access is added by the compiler when using KASAN.] If this perceived stack operation happens to reference the location where %r12 was pushed on the stack then the objtool validation will think that %r12 is being restored and this can cause a stack state mismatch. This kind behavior was seen on xfs code, after a minor change (convert kmem_alloc() to kmalloc()): >> fs/xfs/xfs.o: warning: objtool: xfs_da_grow_inode_int+0x6c1: stack state mismatch: reg1[12]=-2-48 reg2[12]=-1+0 Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202402220435.MGN0EV6l-lkp@intel.com/ Signed-off-by: Alexandre Chartre Link: https://lore.kernel.org/r/20240620144747.2524805-1-alexandre.chartre@oracle.com Signed-off-by: Josh Poimboeuf --- tools/objtool/arch/x86/decode.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index 3a1d80a7878d..ed6bff0e01dc 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -125,8 +125,14 @@ bool arch_pc_relative_reloc(struct reloc *reloc) #define is_RIP() ((modrm_rm & 7) == CFI_BP && modrm_mod == 0) #define have_SIB() ((modrm_rm & 7) == CFI_SP && mod_is_mem()) +/* + * Check the ModRM register. If there is a SIB byte then check with + * the SIB base register. But if the SIB base is 5 (i.e. CFI_BP) and + * ModRM mod is 0 then there is no base register. + */ #define rm_is(reg) (have_SIB() ? \ - sib_base == (reg) && sib_index == CFI_SP : \ + sib_base == (reg) && sib_index == CFI_SP && \ + (sib_base != CFI_BP || modrm_mod != 0) : \ modrm_rm == (reg)) #define rm_is_mem(reg) (mod_is_mem() && !is_RIP() && rm_is(reg)) -- cgit v1.2.3-58-ga151