From fef44ebaf61b57a71ab818058926a3f9a0ac81e6 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 20 Sep 2023 12:41:41 +0100 Subject: x86/unwind/orc: Remove redundant initialization of 'mid' pointer in __orc_find() The 'mid' pointer is being initialized with a value that is never read, it is being re-assigned and used inside a for-loop. Remove the redundant initialization. Cleans up clang scan build warning: arch/x86/kernel/unwind_orc.c:88:7: warning: Value stored to 'mid' during its initialization is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King Signed-off-by: Ingo Molnar Cc: Josh Poimboeuf Link: https://lore.kernel.org/r/20230920114141.118919-1-colin.i.king@gmail.com --- arch/x86/kernel/unwind_orc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c index 7e574cf3bf8a..d00c28aaa5be 100644 --- a/arch/x86/kernel/unwind_orc.c +++ b/arch/x86/kernel/unwind_orc.c @@ -85,7 +85,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table, { int *first = ip_table; int *last = ip_table + num_entries - 1; - int *mid = first, *found = first; + int *mid, *found = first; if (!num_entries) return NULL; -- cgit From b8ec60e1186cdcfce41e7db4c827cb107e459002 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Tue, 19 Sep 2023 17:17:28 -0700 Subject: x86/speculation, objtool: Use absolute relocations for annotations .discard.retpoline_safe sections do not have the SHF_ALLOC flag. These sections referencing text sections' STT_SECTION symbols with PC-relative relocations like R_386_PC32 [0] is conceptually not suitable. Newer LLD will report warnings for REL relocations even for relocatable links [1]: ld.lld: warning: vmlinux.a(drivers/i2c/busses/i2c-i801.o):(.discard.retpoline_safe+0x120): has non-ABS relocation R_386_PC32 against symbol '' Switch to absolute relocations instead, which indicate link-time addresses. In a relocatable link, these addresses are also output section offsets, used by checks in tools/objtool/check.c. When linking vmlinux, these .discard.* sections will be discarded, therefore it is not a problem that R_X86_64_32 cannot represent a kernel address. Alternatively, we could set the SHF_ALLOC flag for .discard.* sections, but I think non-SHF_ALLOC for sections to be discarded makes more sense. Note: if we decide to never support REL architectures (e.g. arm, i386), we can utilize R_*_NONE relocations (.reloc ., BFD_RELOC_NONE, sym), making .discard.* sections zero-sized. That said, the section content waste is 4 bytes per entry, much smaller than sizeof(Elf{32,64}_Rel). [0] commit 1c0c1faf5692 ("objtool: Use relative pointers for annotations") [1] https://github.com/ClangBuiltLinux/linux/issues/1937 Signed-off-by: Fangrui Song Signed-off-by: Ingo Molnar Acked-by: Peter Zijlstra (Intel) Cc: Josh Poimboeuf Link: https://lore.kernel.org/r/20230920001728.1439947-1-maskray@google.com --- arch/x86/include/asm/alternative.h | 4 ++-- arch/x86/include/asm/nospec-branch.h | 4 ++-- include/linux/objtool.h | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 9c4da699e11a..65f79092c9d9 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -58,7 +58,7 @@ #define ANNOTATE_IGNORE_ALTERNATIVE \ "999:\n\t" \ ".pushsection .discard.ignore_alts\n\t" \ - ".long 999b - .\n\t" \ + ".long 999b\n\t" \ ".popsection\n\t" /* @@ -352,7 +352,7 @@ static inline int alternatives_text_reserved(void *start, void *end) .macro ANNOTATE_IGNORE_ALTERNATIVE .Lannotate_\@: .pushsection .discard.ignore_alts - .long .Lannotate_\@ - . + .long .Lannotate_\@ .popsection .endm diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index c55cc243592e..4952b73d944e 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -196,7 +196,7 @@ .macro ANNOTATE_RETPOLINE_SAFE .Lhere_\@: .pushsection .discard.retpoline_safe - .long .Lhere_\@ - . + .long .Lhere_\@ .popsection .endm @@ -334,7 +334,7 @@ #define ANNOTATE_RETPOLINE_SAFE \ "999:\n\t" \ ".pushsection .discard.retpoline_safe\n\t" \ - ".long 999b - .\n\t" \ + ".long 999b\n\t" \ ".popsection\n\t" typedef u8 retpoline_thunk_t[RETPOLINE_THUNK_SIZE]; diff --git a/include/linux/objtool.h b/include/linux/objtool.h index 03f82c2c2ebf..6f6da95fe7f9 100644 --- a/include/linux/objtool.h +++ b/include/linux/objtool.h @@ -48,13 +48,13 @@ #define ANNOTATE_NOENDBR \ "986: \n\t" \ ".pushsection .discard.noendbr\n\t" \ - ".long 986b - .\n\t" \ + ".long 986b\n\t" \ ".popsection\n\t" #define ASM_REACHABLE \ "998:\n\t" \ ".pushsection .discard.reachable\n\t" \ - ".long 998b - .\n\t" \ + ".long 998b\n\t" \ ".popsection\n\t" #else /* __ASSEMBLY__ */ @@ -66,7 +66,7 @@ #define ANNOTATE_INTRA_FUNCTION_CALL \ 999: \ .pushsection .discard.intra_function_calls; \ - .long 999b - .; \ + .long 999b; \ .popsection; /* @@ -118,7 +118,7 @@ .macro ANNOTATE_NOENDBR .Lhere_\@: .pushsection .discard.noendbr - .long .Lhere_\@ - . + .long .Lhere_\@ .popsection .endm @@ -141,7 +141,7 @@ .macro REACHABLE .Lhere_\@: .pushsection .discard.reachable - .long .Lhere_\@ - . + .long .Lhere_\@ .popsection .endm -- cgit From 758a74306f1076b50cb9872af18cb900bafd9497 Mon Sep 17 00:00:00 2001 From: Ruan Jinjie Date: Tue, 1 Aug 2023 11:52:30 +0800 Subject: objtool: Use 'the fallthrough' pseudo-keyword Replace the existing /* fallthrough */ comments with the new 'fallthrough' pseudo-keyword macro: https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through Signed-off-by: Ruan Jinjie Signed-off-by: Ingo Molnar Cc: Josh Poimboeuf Cc: Peter Zijlstra Cc: linux-kernel@vger.kernel.org --- tools/objtool/arch/x86/decode.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c index c0f25d00181e..e327cd827135 100644 --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -291,7 +291,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec switch (modrm_reg & 7) { case 5: imm = -imm; - /* fallthrough */ + fallthrough; case 0: /* add/sub imm, %rsp */ ADD_OP(op) { @@ -375,7 +375,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec break; } - /* fallthrough */ + fallthrough; case 0x88: if (!rex_w) break; @@ -656,7 +656,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec break; } - /* fallthrough */ + fallthrough; case 0xca: /* retf */ case 0xcb: /* retf */ -- cgit From e959c279d391c10b35ce300fb4b0fe3b98e86bd2 Mon Sep 17 00:00:00 2001 From: Aaron Plattner Date: Wed, 4 Oct 2023 17:08:18 -0700 Subject: objtool: Propagate early errors If objtool runs into a problem that causes it to exit early, the overall tool still returns a status code of 0, which causes the build to continue as if nothing went wrong. Note this only affects early errors, as later errors are still ignored by check(). Fixes: b51277eb9775 ("objtool: Ditch subcommands") Signed-off-by: Aaron Plattner Link: https://lore.kernel.org/r/cb6a28832d24b2ebfafd26da9abb95f874c83045.1696355111.git.aplattner@nvidia.com Signed-off-by: Josh Poimboeuf --- tools/objtool/objtool.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c index c54f7235c5d9..f40febdd6e36 100644 --- a/tools/objtool/objtool.c +++ b/tools/objtool/objtool.c @@ -146,7 +146,5 @@ int main(int argc, const char **argv) exec_cmd_init("objtool", UNUSED, UNUSED, UNUSED); pager_init(UNUSED); - objtool_run(argc, argv); - - return 0; + return objtool_run(argc, argv); } -- cgit From f404a58dcf0c862b05602f641ce5fdd8b98fbc3a Mon Sep 17 00:00:00 2001 From: Aaron Plattner Date: Wed, 4 Oct 2023 17:08:19 -0700 Subject: objtool: Remove max symbol name length limitation If one of the symbols processed by read_symbols() happens to have a .cold variant with a name longer than objtool's MAX_NAME_LEN limit, the build fails. Avoid this problem by just using strndup() to copy the parent function's name, rather than strncpy()ing it onto the stack. Signed-off-by: Aaron Plattner Link: https://lore.kernel.org/r/41e94cfea1d9131b758dd637fecdeacd459d4584.1696355111.git.aplattner@nvidia.com Signed-off-by: Josh Poimboeuf --- tools/objtool/elf.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 081befa4674b..3d27983dc908 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -22,8 +22,6 @@ #include #include -#define MAX_NAME_LEN 128 - static inline u32 str_hash(const char *str) { return jhash(str, strlen(str), 0); @@ -515,7 +513,7 @@ static int read_symbols(struct elf *elf) /* Create parent/child links for any cold subfunctions */ list_for_each_entry(sec, &elf->sections, list) { sec_for_each_sym(sec, sym) { - char pname[MAX_NAME_LEN + 1]; + char *pname; size_t pnamelen; if (sym->type != STT_FUNC) continue; @@ -531,15 +529,15 @@ static int read_symbols(struct elf *elf) continue; pnamelen = coldstr - sym->name; - if (pnamelen > MAX_NAME_LEN) { - WARN("%s(): parent function name exceeds maximum length of %d characters", - sym->name, MAX_NAME_LEN); + pname = strndup(sym->name, pnamelen); + if (!pname) { + WARN("%s(): failed to allocate memory", + sym->name); return -1; } - strncpy(pname, sym->name, pnamelen); - pname[pnamelen] = '\0'; pfunc = find_symbol_by_name(elf, pname); + free(pname); if (!pfunc) { WARN("%s(): can't find parent function", -- cgit From 180af1a5bdaf8d4964837a46a9fce8c3a7fd2d97 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Mon, 2 Oct 2023 17:57:47 +0100 Subject: scripts/faddr2line: Don't filter out non-function symbols from readelf As Josh points out in 20230724234734.zy67gm674vl3p3wv@treble: > Problem is, I think the kernel's symbol printing code prints the > nearest kallsyms symbol, and there are some valid non-FUNC code > symbols. For example, syscall_return_via_sysret. so we shouldn't be considering only 'FUNC'-type symbols in the output from readelf. Drop the function symbol type filtering from the faddr2line outer loop. Suggested-by: Josh Poimboeuf Reviewed-by: Nick Desaulniers Link: https://lore.kernel.org/r/20230724234734.zy67gm674vl3p3wv@treble Signed-off-by: Will Deacon Link: https://lore.kernel.org/r/20231002165750.1661-2-will@kernel.org Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index 0e73aca4f908..a35a420d0f26 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -260,7 +260,7 @@ __faddr2line() { DONE=1 - done < <(${READELF} --symbols --wide $objfile | sed 's/\[.*\]//' | ${AWK} -v fn=$sym_name '$4 == "FUNC" && $8 == fn') + done < <(${READELF} --symbols --wide $objfile | sed 's/\[.*\]//' | ${AWK} -v fn=$sym_name '$8 == fn') } [[ $# -lt 2 ]] && usage -- cgit From 86bf86e19d308a1dba41e5f1f7e8cc105a5efa49 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Mon, 2 Oct 2023 17:57:48 +0100 Subject: scripts/faddr2line: Use LLVM addr2line and readelf if LLVM=1 GNU utilities cannot necessarily parse objects built by LLVM, which can result in confusing errors when using 'faddr2line': $ CROSS_COMPILE=aarch64-linux-gnu- ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260 aarch64-linux-gnu-addr2line: vmlinux: unknown type [0x13] section `.relr.dyn' aarch64-linux-gnu-addr2line: DWARF error: invalid or unhandled FORM value: 0x25 do_one_initcall+0xf4/0x260: aarch64-linux-gnu-addr2line: vmlinux: unknown type [0x13] section `.relr.dyn' aarch64-linux-gnu-addr2line: DWARF error: invalid or unhandled FORM value: 0x25 $x.73 at main.c:? Although this can be worked around by setting CROSS_COMPILE to "llvm=-", it's cleaner to follow the same syntax as the top-level Makefile and accept LLVM= as an indication to use the llvm- tools, optionally specifying their location or specific version number. Suggested-by: Masahiro Yamada Signed-off-by: Will Deacon Reviewed-by: Nick Desaulniers Link: https://lore.kernel.org/r/20231002165750.1661-3-will@kernel.org Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/scripts/faddr2line b/scripts/faddr2line index a35a420d0f26..6b8206802157 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -58,8 +58,21 @@ die() { exit 1 } -READELF="${CROSS_COMPILE:-}readelf" -ADDR2LINE="${CROSS_COMPILE:-}addr2line" +UTIL_SUFFIX="" +if [[ "${LLVM:-}" == "" ]]; then + UTIL_PREFIX=${CROSS_COMPILE:-} +else + UTIL_PREFIX=llvm- + + if [[ "${LLVM}" == *"/" ]]; then + UTIL_PREFIX=${LLVM}${UTIL_PREFIX} + elif [[ "${LLVM}" == "-"* ]]; then + UTIL_SUFFIX=${LLVM} + fi +fi + +READELF="${UTIL_PREFIX}readelf${UTIL_SUFFIX}" +ADDR2LINE="${UTIL_PREFIX}addr2line${UTIL_SUFFIX}" AWK="awk" GREP="grep" -- cgit From 60fd39af33d3f63c4c94bd06784ebdf0d883f5c9 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Mon, 2 Oct 2023 17:57:49 +0100 Subject: scripts/faddr2line: Skip over mapping symbols in output from readelf Mapping symbols emitted in the readelf output can confuse the 'faddr2line' symbol size calculation, resulting in the erroneous rejection of valid offsets. This is especially prevalent when building an arm64 kernel with CONFIG_CFI_CLANG=y, where most functions are prefixed with a 32-bit data value in a '$d.n' section. For example: 447538: ffff800080014b80 548 FUNC GLOBAL DEFAULT 2 do_one_initcall 104: ffff800080014c74 0 NOTYPE LOCAL DEFAULT 2 $x.73 106: ffff800080014d30 0 NOTYPE LOCAL DEFAULT 2 $x.75 111: ffff800080014da4 0 NOTYPE LOCAL DEFAULT 2 $d.78 112: ffff800080014da8 0 NOTYPE LOCAL DEFAULT 2 $x.79 36: ffff800080014de0 200 FUNC LOCAL DEFAULT 2 run_init_process Adding a warning to do_one_initcall() results in: | WARNING: CPU: 0 PID: 1 at init/main.c:1236 do_one_initcall+0xf4/0x260 Which 'faddr2line' refuses to accept: $ ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260 skipping do_one_initcall address at 0xffff800080014c74 due to size mismatch (0x260 != 0x224) no match for do_one_initcall+0xf4/0x260 Filter out these entries from readelf using a shell reimplementation of is_mapping_symbol(), so that the size of a symbol is calculated as a delta to the next symbol present in ksymtab. Suggested-by: Masahiro Yamada Signed-off-by: Will Deacon Reviewed-by: Nick Desaulniers Link: https://lore.kernel.org/r/20231002165750.1661-4-will@kernel.org Signed-off-by: Josh Poimboeuf --- scripts/faddr2line | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/faddr2line b/scripts/faddr2line index 6b8206802157..587415a52b6f 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -179,6 +179,11 @@ __faddr2line() { local cur_sym_elf_size=${fields[2]} local cur_sym_name=${fields[7]:-} + # is_mapping_symbol(cur_sym_name) + if [[ ${cur_sym_name} =~ ^(\.L|L0|\$) ]]; then + continue + fi + if [[ $cur_sym_addr = $sym_addr ]] && [[ $cur_sym_elf_size = $sym_elf_size ]] && [[ $cur_sym_name = $sym_name ]]; then -- cgit