The add_jump_destinations() logic is a bit weird and convoluted after being incrementally tweaked over the years. Refactor it to hopefully be more logical and straightforward.
Signed-off-by: Josh Poimboeuf <jpoim...@kernel.org> --- tools/objtool/check.c | 227 +++++++++++++--------------- tools/objtool/include/objtool/elf.h | 4 +- 2 files changed, 104 insertions(+), 127 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 66cbeebd16ea..e4ca5edf73ad 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1439,9 +1439,14 @@ static void add_return_call(struct objtool_file *file, struct instruction *insn, } static bool is_first_func_insn(struct objtool_file *file, - struct instruction *insn, struct symbol *sym) + struct instruction *insn) { - if (insn->offset == sym->offset) + struct symbol *func = insn_func(insn); + + if (!func) + return false; + + if (insn->offset == func->offset) return true; /* Allow direct CALL/JMP past ENDBR */ @@ -1449,52 +1454,32 @@ static bool is_first_func_insn(struct objtool_file *file, struct instruction *prev = prev_insn_same_sym(file, insn); if (prev && prev->type == INSN_ENDBR && - insn->offset == sym->offset + prev->len) + insn->offset == func->offset + prev->len) return true; } return false; } -/* - * A sibling call is a tail-call to another symbol -- to differentiate from a - * recursive tail-call which is to the same symbol. - */ -static bool jump_is_sibling_call(struct objtool_file *file, - struct instruction *from, struct instruction *to) -{ - struct symbol *fs = from->sym; - struct symbol *ts = to->sym; - - /* Not a sibling call if from/to a symbol hole */ - if (!fs || !ts) - return false; - - /* Not a sibling call if not targeting the start of a symbol. */ - if (!is_first_func_insn(file, to, ts)) - return false; - - /* Disallow sibling calls into STT_NOTYPE */ - if (is_notype_sym(ts)) - return false; - - /* Must not be self to be a sibling */ - return fs->pfunc != ts->pfunc; -} - /* * Find the destination instructions for all jumps. */ static int add_jump_destinations(struct objtool_file *file) { - struct instruction *insn, *jump_dest; + struct instruction *insn; struct reloc *reloc; - struct section *dest_sec; - unsigned long dest_off; int ret; for_each_insn(file, insn) { struct symbol *func = insn_func(insn); + struct instruction *dest_insn; + struct section *dest_sec; + struct symbol *dest_sym; + unsigned long dest_off; + bool dest_undef = false; + + if (!is_static_jump(insn)) + continue; if (insn->jump_dest) { /* @@ -1503,129 +1488,121 @@ static int add_jump_destinations(struct objtool_file *file) */ continue; } - if (!is_static_jump(insn)) - continue; reloc = insn_reloc(file, insn); if (!reloc) { dest_sec = insn->sec; dest_off = arch_jump_destination(insn); - } else if (is_sec_sym(reloc->sym)) { + } else if (is_undef_sym(reloc->sym)) { + dest_sym = reloc->sym; + dest_undef = true; + } else { dest_sec = reloc->sym->sec; - dest_off = arch_insn_adjusted_addend(insn, reloc); - } else if (reloc->sym->retpoline_thunk) { + dest_off = reloc->sym->sym.st_value + + arch_insn_adjusted_addend(insn, reloc); + } + + if (!dest_undef) { + dest_insn = find_insn(file, dest_sec, dest_off); + if (!dest_insn) { + struct symbol *sym = find_symbol_by_offset(dest_sec, dest_off); + + /* + * retbleed_untrain_ret() jumps to + * __x86_return_thunk(), but objtool can't find + * the thunk's starting RET instruction, + * because the RET is also in the middle of + * another instruction. Objtool only knows + * about the outer instruction. + */ + if (sym && sym->embedded_insn) { + add_return_call(file, insn, false); + continue; + } + + /* + * GCOV/KCOV dead code can jump to the end of + * the function/section. + */ + if (file->ignore_unreachables && func && + dest_sec == insn->sec && + dest_off == func->offset + func->len) + continue; + + ERROR_INSN(insn, "can't find jump dest instruction at %s+0x%lx", + dest_sec->name, dest_off); + return -1; + } + + dest_sym = dest_insn->sym; + if (!dest_sym) + goto set_jump_dest; + } + + if (dest_sym->retpoline_thunk) { ret = add_retpoline_call(file, insn); if (ret) return ret; continue; - } else if (reloc->sym->return_thunk) { + } + + if (dest_sym->return_thunk) { add_return_call(file, insn, true); continue; - } else if (func) { - /* - * External sibling call or internal sibling call with - * STT_FUNC reloc. - */ - ret = add_call_dest(file, insn, reloc->sym, true); - if (ret) - return ret; - continue; - } else if (reloc->sym->sec->idx) { - dest_sec = reloc->sym->sec; - dest_off = reloc->sym->sym.st_value + - arch_dest_reloc_offset(reloc_addend(reloc)); - } else { - /* non-func asm code jumping to another file */ - continue; } - jump_dest = find_insn(file, dest_sec, dest_off); - if (!jump_dest) { - struct symbol *sym = find_symbol_by_offset(dest_sec, dest_off); - - /* - * This is a special case for retbleed_untrain_ret(). - * It jumps to __x86_return_thunk(), but objtool - * can't find the thunk's starting RET - * instruction, because the RET is also in the - * middle of another instruction. Objtool only - * knows about the outer instruction. - */ - if (sym && sym->embedded_insn) { - add_return_call(file, insn, false); - continue; - } - - /* - * GCOV/KCOV dead code can jump to the end of the - * function/section. - */ - if (file->ignore_unreachables && func && - dest_sec == insn->sec && - dest_off == func->offset + func->len) - continue; - - ERROR_INSN(insn, "can't find jump dest instruction at %s+0x%lx", - dest_sec->name, dest_off); - return -1; - } - - /* - * An intra-TU jump in retpoline.o might not have a relocation - * for its jump dest, in which case the above - * add_{retpoline,return}_call() didn't happen. - */ - if (jump_dest->sym && jump_dest->offset == jump_dest->sym->offset) { - if (jump_dest->sym->retpoline_thunk) { - ret = add_retpoline_call(file, insn); + if (dest_undef) { + /* External symbol */ + if (func) { + /* External sibling call */ + ret = add_call_dest(file, insn, dest_sym, true); if (ret) return ret; continue; } - if (jump_dest->sym->return_thunk) { - add_return_call(file, insn, true); - continue; - } + + /* Non-func asm code jumping to external symbol */ + continue; } + if (!insn->sym || insn->sym == dest_insn->sym) + goto set_jump_dest; + /* - * Cross-function jump. + * Internal cross-function jump. */ - if (func && insn_func(jump_dest) && !func->cold && - insn_func(jump_dest)->cold) { - - /* - * For GCC 8+, create parent/child links for any cold - * subfunctions. This is _mostly_ redundant with a - * similar initialization in read_symbols(). - * - * If a function has aliases, we want the *first* such - * function in the symbol table to be the subfunction's - * parent. In that case we overwrite the - * initialization done in read_symbols(). - * - * However this code can't completely replace the - * read_symbols() code because this doesn't detect the - * case where the parent function's only reference to a - * subfunction is through a jump table. - */ - func->cfunc = insn_func(jump_dest); - insn_func(jump_dest)->pfunc = func; + /* + * For GCC 8+, create parent/child links for any cold + * subfunctions. This is _mostly_ redundant with a + * similar initialization in read_symbols(). + * + * If a function has aliases, we want the *first* such + * function in the symbol table to be the subfunction's + * parent. In that case we overwrite the + * initialization done in read_symbols(). + * + * However this code can't completely replace the + * read_symbols() code because this doesn't detect the + * case where the parent function's only reference to a + * subfunction is through a jump table. + */ + if (func && dest_sym->cold) { + func->cfunc = dest_sym; + dest_sym->pfunc = func; + goto set_jump_dest; } - if (jump_is_sibling_call(file, insn, jump_dest)) { - /* - * Internal sibling call without reloc or with - * STT_SECTION reloc. - */ - ret = add_call_dest(file, insn, insn_func(jump_dest), true); + if (is_first_func_insn(file, dest_insn)) { + /* Internal sibling call */ + ret = add_call_dest(file, insn, dest_sym, true); if (ret) return ret; continue; } - insn->jump_dest = jump_dest; +set_jump_dest: + insn->jump_dest = dest_insn; } return 0; diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h index 842faec1b9a9..74c7b84b5310 100644 --- a/tools/objtool/include/objtool/elf.h +++ b/tools/objtool/include/objtool/elf.h @@ -180,9 +180,9 @@ static inline unsigned int elf_text_rela_type(struct elf *elf) return elf_addr_size(elf) == 4 ? R_TEXT32 : R_TEXT64; } -static inline bool sym_has_sec(struct symbol *sym) +static inline bool is_undef_sym(struct symbol *sym) { - return sym->sec->idx; + return !sym->sec->idx; } static inline bool is_null_sym(struct symbol *sym) -- 2.49.0