objtool: warning improvements
Josh Poimboeuf
maintainerv2 cover objtool: warning improvements
v2:
- keep --backtrace
- hard-code awk logic (don't use objdump-func script)
- add OBJTOOL_VERBOSE=1
- add WARN_INSN()
- several other improvements/fixes - best to review from scratch ;-)
Josh Poimboeuf (9):
scripts/objdump-func: Support multiple functions
objtool: Add WARN_INSN()
objtool: Limit unreachable warnings to once per function
objtool: Add symbol iteration helpers
objtool: Add verbose option for disassembling affected functions
objtool: Include backtrace in verbose mode
objtool: Remove superfluous dead_end_function() check
objtool: Detect missing __noreturn annotations
objtool: Ignore exc_double_fault() __noreturn warnings
scripts/objdump-func | 34 +- tools/objtool/Documentation/objtool.txt | 11 + tools/objtool/builtin-check.c | 5 + tools/objtool/check.c | 421 +++++++++++++----------- tools/objtool/elf.c | 2 +- tools/objtool/include/objtool/builtin.h | 1 + tools/objtool/include/objtool/elf.h | 10 + tools/objtool/include/objtool/warn.h | 22 +- tools/objtool/orc_gen.c | 9 +- 9 files changed, 307 insertions(+), 208 deletions(-)
Signature · 1 line
2.39.2
Josh Poimboeuf
maintainerPATCH v2 1/9 scripts/objdump-func: Support multiple functions
Allow specifying multiple functions on the cmdline. Note this removes the secret EXTRA_ARGS feature.
While at it, spread out the awk to make it more readable.
Signed-off-by Josh Poimboeuf <[email protected]>
--- scripts/objdump-func | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
scripts/objdump-func +25 -9
@@ -3,7 +3,7 @@ # # Disassemble a single function. # -# usage: objdump-func <file> <func> +# usage: objdump-func <file> <func> [<func> ...] set -o errexit set -o nounset
@@ -13,17 +13,33 @@ OBJDUMP="${CROSS_COMPILE:-}objdump" command -v gawk >/dev/null 2>&1 || die "gawk isn't installed" usage() { - echo "usage: objdump-func <file> <func>" >&2 + echo "usage: objdump-func <file> <func> [<func> ...]" >&2 exit 1 } [[ $# -lt 2 ]] && usage OBJ=$1; shift -FUNC=$1; shift - -# Secret feature to allow adding extra objdump args at the end -EXTRA_ARGS=$@ - -# Note this also matches compiler-added suffixes like ".cold", etc -${OBJDUMP} -wdr $EXTRA_ARGS $OBJ | gawk -M -v f=$FUNC '/^$/ { P=0; } $0 ~ "<" f "(\\..*)?>:" { P=1; O=strtonum("0x" $1); } { if (P) { o=strtonum("0x" $1); printf("%04x ", o-O); print $0; } }' +FUNCS=("$@") + +${OBJDUMP} -wdr $OBJ | gawk -M -v _funcs="${FUNCS[*]}" ' + BEGIN { split(_funcs, funcs); } + /^$/ { func_match=0; } + /<.*>:/ { + f = gensub(/.*<(.*)>:/, "\\1", 1); + for (i in funcs) { + # match compiler-added suffixes like ".cold", etc + if (f ~ "^" funcs[i] "(\\..*)?") { + func_match = 1; + base = strtonum("0x" $1); + break; + } + } + } + { + if (func_match) { + addr = strtonum("0x" $1); + printf("%04x ", addr - base); + print; + } + }'
Signature · 1 line
2.39.2
[tip: objtool/core] scripts/objdump-func: Support multiple functions
The following commit has been merged into the objtool/core branch of tip:
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: 27d000d635ce48b579988e9b3240352a2a0306e0 Gitweb: https://git.kernel.org/tip/27d000d635ce48b579988e9b3240352a2a0306e0 Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 12 Apr 2023 12:03:16 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 14 Apr 2023 16:08:28 +02:00
scripts/objdump-func: Support multiple functions
Allow specifying multiple functions on the cmdline. Note this removes the secret EXTRA_ARGS feature.
While at it, spread out the awk to make it more readable.
Signed-off-by Josh Poimboeuf <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
--- scripts/objdump-func | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
scripts/objdump-func +25 -9
@@ -3,7 +3,7 @@ # # Disassemble a single function. # -# usage: objdump-func <file> <func> +# usage: objdump-func <file> <func> [<func> ...] set -o errexit set -o nounset
@@ -13,17 +13,33 @@ OBJDUMP="${CROSS_COMPILE:-}objdump" command -v gawk >/dev/null 2>&1 || die "gawk isn't installed" usage() { - echo "usage: objdump-func <file> <func>" >&2 + echo "usage: objdump-func <file> <func> [<func> ...]" >&2 exit 1 } [[ $# -lt 2 ]] && usage OBJ=$1; shift -FUNC=$1; shift - -# Secret feature to allow adding extra objdump args at the end -EXTRA_ARGS=$@ - -# Note this also matches compiler-added suffixes like ".cold", etc -${OBJDUMP} -wdr $EXTRA_ARGS $OBJ | gawk -M -v f=$FUNC '/^$/ { P=0; } $0 ~ "<" f "(\\..*)?>:" { P=1; O=strtonum("0x" $1); } { if (P) { o=strtonum("0x" $1); printf("%04x ", o-O); print $0; } }' +FUNCS=("$@") + +${OBJDUMP} -wdr $OBJ | gawk -M -v _funcs="${FUNCS[*]}" ' + BEGIN { split(_funcs, funcs); } + /^$/ { func_match=0; } + /<.*>:/ { + f = gensub(/.*<(.*)>:/, "\\1", 1); + for (i in funcs) { + # match compiler-added suffixes like ".cold", etc + if (f ~ "^" funcs[i] "(\\..*)?") { + func_match = 1; + base = strtonum("0x" $1); + break; + } + } + } + { + if (func_match) { + addr = strtonum("0x" $1); + printf("%04x ", addr - base); + print; + } + }'
Josh Poimboeuf
maintainerPATCH v2 2/9 objtool: Add WARN_INSN()
In reply to Josh PoimboeufIt's easier to use and also gives easy access to the instruction's containing function, which is useful for printing that function's symbol. It will also be useful in the future for rate-limiting and disassembly of warned functions.
Signed-off-by Josh Poimboeuf <[email protected]>
--- tools/objtool/check.c | 171 ++++++++++----------------- tools/objtool/include/objtool/warn.h | 5 + tools/objtool/orc_gen.c | 9 +- 3 files changed, 70 insertions(+), 115 deletions(-)
tools/objtool/check.c +62 -109
@@ -1444,7 +1444,7 @@ static void annotate_call_site(struct objtool_file *file,if (opts.mcount && sym->fentry) {if (sibling) -WARN_FUNC ("Tail call to __fentry__ !?!?" , insn->sec, insn->offset); +WARN_INSN (insn,"tail call to __fentry__ !?!?" );if (opts.mnop) {if (reloc) { reloc->type =R_NONE ;
@@ -1646,9 +1646,8 @@ static int add_jump_destinations(struct objtool_file *file)continue ; } -WARN_FUNC ("can't find jump dest instruction at %s+0x%lx" , - insn->sec, insn->offset, dest_sec->name, - dest_off); +WARN_INSN (insn,"can't find jump dest instruction at %s+0x%lx" , + dest_sec->name, dest_off);return -1 ; }
@@ -1731,13 +1730,12 @@ static int add_call_destinations(struct objtool_file *file)continue ;if (!insn_call_dest (insn)) { -WARN_FUNC ("unannotated intra-function call" , insn->sec, insn->offset); +WARN_INSN (insn,"unannotated intra-function call" );return -1 ; }if (insn_func (insn) &&insn_call_dest (insn)->type !=STT_FUNC ) { -WARN_FUNC ("unsupported call to non-function" , - insn->sec, insn->offset); +WARN_INSN (insn,"unsupported call to non-function" );return -1 ; }
@@ -1745,10 +1743,8 @@ static int add_call_destinations(struct objtool_file *file) dest_off =arch_dest_reloc_offset (reloc->addend); dest =find_call_destination (reloc->sym->sec, dest_off);if (!dest) { -WARN_FUNC ("can't find call dest symbol at %s+0x%lx" , - insn->sec, insn->offset, - reloc->sym->sec->name, - dest_off); +WARN_INSN (insn,"can't find call dest symbol at %s+0x%lx" , + reloc->sym->sec->name, dest_off);return -1 ; }
@@ -1808,8 +1804,7 @@ static int handle_group_alt(struct objtool_file *file, }else {if (orig_alt_group->last_insn->offset + orig_alt_group->last_insn->len - orig_alt_group->first_insn->offset != special_alt->orig_len) { - WARN_FUNC("weirdly overlapping alternative! %ld != %d", - orig_insn->sec, orig_insn->offset, + WARN_INSN(orig_insn, "weirdly overlapping alternative! %ld != %d", orig_alt_group->last_insn->offset + orig_alt_group->last_insn->len - orig_alt_group->first_insn->offset,
@@ -1878,8 +1873,7 @@ static int handle_group_alt(struct objtool_file *file,if (alt_reloc &&arch_pc_relative_reloc (alt_reloc) && !arch_support_alt_relocation (special_alt, insn, alt_reloc)) { -WARN_FUNC ("unsupported relocation in alternatives section" , - insn->sec, insn->offset); +WARN_INSN (insn,"unsupported relocation in alternatives section" );return -1 ; }
@@ -1893,8 +1887,7 @@ static int handle_group_alt(struct objtool_file *file,if (dest_off == special_alt->new_off + special_alt->new_len) { insn->jump_dest =next_insn_same_sec (file, orig_alt_group->last_insn);if (!insn->jump_dest) { -WARN_FUNC ("can't find alternative jump destination" , - insn->sec, insn->offset); +WARN_INSN (insn,"can't find alternative jump destination" );return -1 ; } }
@@ -1928,8 +1921,7 @@ static int handle_jump_alt(struct objtool_file *file,if (orig_insn->type !=INSN_JUMP_UNCONDITIONAL && orig_insn->type !=INSN_NOP ) { -WARN_FUNC ("unsupported instruction at jump label" , - orig_insn->sec, orig_insn->offset); +WARN_INSN (orig_insn,"unsupported instruction at jump label" );return -1 ; }
@@ -2008,8 +2000,7 @@ static int add_special_section_alts(struct objtool_file *file)if (special_alt->group) {if (!special_alt->orig_len) { -WARN_FUNC ("empty alternative entry" , - orig_insn->sec, orig_insn->offset); +WARN_INSN (orig_insn,"empty alternative entry" );continue ; }
@@ -2100,8 +2091,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn, }if (!prev_offset) { -WARN_FUNC ("can't find switch jump table" , - insn->sec, insn->offset); +WARN_INSN (insn,"can't find switch jump table" );return -1 ; }
@@ -2305,8 +2295,7 @@ static int read_unwind_hints(struct objtool_file *file)if (sym && sym->bind ==STB_GLOBAL ) {if (opts.ibt && insn->type !=INSN_ENDBR && !insn->noendbr) { -WARN_FUNC ("UNWIND_HINT_IRET_REGS without ENDBR" , - insn->sec, insn->offset); +WARN_INSN (insn,"UNWIND_HINT_IRET_REGS without ENDBR" ); } } }
@@ -2320,8 +2309,7 @@ static int read_unwind_hints(struct objtool_file *file) cfi = *(insn->cfi);if (arch_decode_hint_reg (hint->sp_reg, &cfi.cfa.base)) { -WARN_FUNC ("unsupported unwind_hint sp base reg %d" , - insn->sec, insn->offset, hint->sp_reg); +WARN_INSN (insn,"unsupported unwind_hint sp base reg %d" , hint->sp_reg);return -1 ; }
@@ -2384,8 +2372,7 @@ static int read_retpoline_hints(struct objtool_file *file) insn->type != INSN_CALL_DYNAMIC && insn->type != INSN_RETURN && insn->type != INSN_NOP) { -WARN_FUNC ("retpoline_safe hint not an indirect jump/call/ret/nop" , - insn->sec, insn->offset); +WARN_INSN (insn,"retpoline_safe hint not an indirect jump/call/ret/nop" );return -1 ; }
@@ -2496,8 +2483,7 @@ static int read_intra_function_calls(struct objtool_file *file) }if (insn->type !=INSN_CALL ) { -WARN_FUNC ("intra_function_call not a direct call" , - insn->sec, insn->offset); +WARN_INSN (insn,"intra_function_call not a direct call" );return -1 ; }
@@ -2511,8 +2497,7 @@ static int read_intra_function_calls(struct objtool_file *file) dest_off =arch_jump_destination (insn); insn->jump_dest =find_insn (file, insn->sec, dest_off);if (!insn->jump_dest) { -WARN_FUNC ("can't find call dest at %s+0x%lx" , - insn->sec, insn->offset, +WARN_INSN (insn,"can't find call dest at %s+0x%lx" , insn->sec->name, dest_off);return -1 ; }
@@ -2853,7 +2838,7 @@ static int update_cfi_state(struct instruction *insn,/* stack operations don't make sense with an undefined CFA */ if (cfa->base ==CFI_UNDEFINED ) {if (insn_func (insn)) { -WARN_FUNC ("undefined stack state" , insn->sec, insn->offset); +WARN_INSN (insn,"undefined stack state" );return -1 ; }return 0 ;
@@ -3047,8 +3032,7 @@ static int update_cfi_state(struct instruction *insn, }if (op->dest.reg == cfi->cfa.base && !(next_insn && next_insn->hint)) { -WARN_FUNC ("unsupported stack register modification" , - insn->sec, insn->offset); +WARN_INSN (insn,"unsupported stack register modification" );return -1 ; }
@@ -3058,8 +3042,7 @@ static int update_cfi_state(struct instruction *insn,if (op->dest.reg !=CFI_SP || (cfi->drap_reg !=CFI_UNDEFINED && cfa->base !=CFI_SP ) || (cfi->drap_reg ==CFI_UNDEFINED && cfa->base !=CFI_BP )) { -WARN_FUNC ("unsupported stack pointer realignment" , - insn->sec, insn->offset); +WARN_INSN (insn,"unsupported stack pointer realignment" );return -1 ; }
@@ -3154,8 +3137,7 @@ static int update_cfi_state(struct instruction *insn,break ;default : -WARN_FUNC ("unknown stack-related instruction" , - insn->sec, insn->offset); +WARN_INSN (insn,"unknown stack-related instruction" );return -1 ; }
@@ -3244,8 +3226,7 @@ static int update_cfi_state(struct instruction *insn,case OP_DEST_MEM :if (op->src.type !=OP_SRC_POP && op->src.type !=OP_SRC_POPF ) { -WARN_FUNC ("unknown stack-related memory operation" , - insn->sec, insn->offset); +WARN_INSN (insn,"unknown stack-related memory operation" );return -1 ; }
@@ -3257,8 +3238,7 @@ static int update_cfi_state(struct instruction *insn,break ;default : -WARN_FUNC ("unknown stack-related instruction" , - insn->sec, insn->offset); +WARN_INSN (insn,"unknown stack-related instruction" );return -1 ; }
@@ -3297,8 +3277,7 @@ static int propagate_alt_cfi(struct objtool_file *file, struct instruction *insnstruct alt_group *orig_group = insn->alt_group->orig_group ?: insn->alt_group;struct instruction *orig = orig_group->first_insn;char *where =offstr (insn->sec, insn->offset); -WARN_FUNC ("stack layout conflict in alternatives: %s" , - orig->sec, orig->offset, where); +WARN_INSN (orig,"stack layout conflict in alternatives: %s" , where);free (where);return -1 ; }
@@ -3325,8 +3304,7 @@ static int handle_insn_ops(struct instruction *insn,if (!state->uaccess_stack) { state->uaccess_stack =1 ; }else if (state->uaccess_stack >>31 ) { -WARN_FUNC ("PUSHF stack exhausted" , - insn->sec, insn->offset); +WARN_INSN (insn,"PUSHF stack exhausted" );return 1 ; } state->uaccess_stack <<=1 ;
@@ -3358,8 +3336,7 @@ static bool insn_cfi_match(struct instruction *insn, struct cfi_state *cfi2)if (memcmp (&cfi1->cfa, &cfi2->cfa,sizeof (cfi1->cfa))) { -WARN_FUNC ("stack state mismatch: cfa1=%d%+d cfa2=%d%+d" , - insn->sec, insn->offset, +WARN_INSN (insn,"stack state mismatch: cfa1=%d%+d cfa2=%d%+d" , cfi1->cfa.base, cfi1->cfa.offset, cfi2->cfa.base, cfi2->cfa.offset);
@@ -3369,8 +3346,7 @@ static bool insn_cfi_match(struct instruction *insn, struct cfi_state *cfi2) sizeof(struct cfi_reg)))continue ; -WARN_FUNC ("stack state mismatch: reg1[%d]=%d%+d reg2[%d]=%d%+d" , - insn->sec, insn->offset, +WARN_INSN (insn,"stack state mismatch: reg1[%d]=%d%+d reg2[%d]=%d%+d" , i, cfi1->regs[i].base, cfi1->regs[i].offset, i, cfi2->regs[i].base, cfi2->regs[i].offset);break ;
@@ -3378,15 +3354,14 @@ static bool insn_cfi_match(struct instruction *insn, struct cfi_state *cfi2) }else if (cfi1->type != cfi2->type) { -WARN_FUNC ("stack state mismatch: type1=%d type2=%d" , - insn->sec, insn->offset, cfi1->type, cfi2->type); +WARN_INSN (insn,"stack state mismatch: type1=%d type2=%d" , + cfi1->type, cfi2->type); }else if (cfi1->drap != cfi2->drap || (cfi1->drap && cfi1->drap_reg != cfi2->drap_reg) || (cfi1->drap && cfi1->drap_offset != cfi2->drap_offset)) { -WARN_FUNC ("stack state mismatch: drap1=%d(%d,%d) drap2=%d(%d,%d)" , - insn->sec, insn->offset, +WARN_INSN (insn,"stack state mismatch: drap1=%d(%d,%d) drap2=%d(%d,%d)" , cfi1->drap, cfi1->drap_reg, cfi1->drap_offset, cfi2->drap, cfi2->drap_reg, cfi2->drap_offset);
@@ -3494,20 +3469,17 @@ static int validate_call(struct objtool_file *file, {if (state->noinstr && state->instr <=0 && !noinstr_call_dest (file, insn,insn_call_dest (insn))) { -WARN_FUNC ("call to %s() leaves .noinstr.text section" , - insn->sec, insn->offset,call_dest_name (insn)); +WARN_INSN (insn,"call to %s() leaves .noinstr.text section" ,call_dest_name (insn));return 1 ; }if (state->uaccess && !func_uaccess_safe (insn_call_dest (insn))) { -WARN_FUNC ("call to %s() with UACCESS enabled" , - insn->sec, insn->offset,call_dest_name (insn)); +WARN_INSN (insn,"call to %s() with UACCESS enabled" ,call_dest_name (insn));return 1 ; }if (state->df) { -WARN_FUNC ("call to %s() with DF set" , - insn->sec, insn->offset,call_dest_name (insn)); +WARN_INSN (insn,"call to %s() with DF set" ,call_dest_name (insn));return 1 ; }
@@ -3519,8 +3491,7 @@ static int validate_sibling_call(struct objtool_file *file, struct insn_state *state) {if (insn_func (insn) &&has_modified_stack_frame (insn, state)) { -WARN_FUNC ("sibling call from callable instruction with modified stack frame" , - insn->sec, insn->offset); +WARN_INSN (insn,"sibling call from callable instruction with modified stack frame" );return 1 ; }
@@ -3530,38 +3501,32 @@ static int validate_sibling_call(struct objtool_file *file,static int validate_return (struct symbol *func,struct instruction *insn,struct insn_state *state) {if (state->noinstr && state->instr >0 ) { -WARN_FUNC ("return with instrumentation enabled" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with instrumentation enabled" );return 1 ; }if (state->uaccess && !func_uaccess_safe (func)) { -WARN_FUNC ("return with UACCESS enabled" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with UACCESS enabled" );return 1 ; }if (!state->uaccess &&func_uaccess_safe (func)) { -WARN_FUNC ("return with UACCESS disabled from a UACCESS-safe function" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with UACCESS disabled from a UACCESS-safe function" );return 1 ; }if (state->df) { -WARN_FUNC ("return with DF set" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with DF set" );return 1 ; }if (func &&has_modified_stack_frame (insn, state)) { -WARN_FUNC ("return with modified stack frame" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with modified stack frame" );return 1 ; }if (state->cfi.bp_scratch) { -WARN_FUNC ("BP used as a scratch register" , - insn->sec, insn->offset); +WARN_INSN (insn,"BP used as a scratch register" );return 1 ; }
@@ -3633,8 +3598,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, }if (func && insn->ignore) { -WARN_FUNC ("BUG: why am I validating an ignored function?" , - sec, insn->offset); +WARN_INSN (insn,"BUG: why am I validating an ignored function?" );return 1 ; }
@@ -3667,14 +3631,12 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, }if (!save_insn) { -WARN_FUNC ("no corresponding CFI save for CFI restore" , - sec, insn->offset); +WARN_INSN (insn,"no corresponding CFI save for CFI restore" );return 1 ; }if (!save_insn->visited) { -WARN_FUNC ("objtool isn't smart enough to handle this CFI save/restore combo" , - sec, insn->offset); +WARN_INSN (insn,"objtool isn't smart enough to handle this CFI save/restore combo" );return 1 ; }
@@ -3734,8 +3696,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,if (opts.stackval && func && !is_fentry_call (insn) && !has_valid_stack_frame (&state)) { -WARN_FUNC ("call without frame pointer save/setup" , - sec, insn->offset); +WARN_INSN (insn,"call without frame pointer save/setup" );return 1 ; }
@@ -3781,15 +3742,14 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,case INSN_CONTEXT_SWITCH :if (func && (!next_insn || !next_insn->hint)) { -WARN_FUNC ("unsupported instruction in callable function" , - sec, insn->offset); +WARN_INSN (insn,"unsupported instruction in callable function" );return 1 ; }return 0 ;case INSN_STAC :if (state.uaccess) { -WARN_FUNC ("recursive UACCESS enable" , sec, insn->offset); +WARN_INSN (insn,"recursive UACCESS enable" );return 1 ; }
@@ -3798,12 +3758,12 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,case INSN_CLAC :if (!state.uaccess && func) { -WARN_FUNC ("redundant UACCESS disable" , sec, insn->offset); +WARN_INSN (insn,"redundant UACCESS disable" );return 1 ; }if (func_uaccess_safe (func) && !state.uaccess_stack) { -WARN_FUNC ("UACCESS-safe disables UACCESS" , sec, insn->offset); +WARN_INSN (insn,"UACCESS-safe disables UACCESS" );return 1 ; }
@@ -3812,7 +3772,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,case INSN_STD :if (state.df) { -WARN_FUNC ("recursive STD" , sec, insn->offset); +WARN_INSN (insn,"recursive STD" );return 1 ; }
@@ -3821,7 +3781,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,case INSN_CLD :if (!state.df && func) { -WARN_FUNC ("redundant CLD" , sec, insn->offset); +WARN_INSN (insn,"redundant CLD" );return 1 ; }
@@ -3929,15 +3889,14 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)case INSN_CALL_DYNAMIC :case INSN_JUMP_DYNAMIC :case INSN_JUMP_DYNAMIC_CONDITIONAL : -WARN_FUNC ("early indirect call" , insn->sec, insn->offset); +WARN_INSN (insn,"early indirect call" );return 1 ;case INSN_JUMP_UNCONDITIONAL :case INSN_JUMP_CONDITIONAL :if (!is_sibling_call (insn)) {if (!insn->jump_dest) { -WARN_FUNC ("unresolved jump target after linking?!?" , - insn->sec, insn->offset); +WARN_INSN (insn,"unresolved jump target after linking?!?" );return -1 ; } ret =validate_unret (file, insn->jump_dest);
@@ -3978,7 +3937,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)return 0 ;case INSN_RETURN : -WARN_FUNC ("RET before UNTRAIN" , insn->sec, insn->offset); +WARN_INSN (insn,"RET before UNTRAIN" );return 1 ;case INSN_NOP :
@@ -3991,7 +3950,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn) }if (!next) { -WARN_FUNC ("teh end!" , insn->sec, insn->offset); +WARN_INSN (insn,"teh end!" );return -1 ; } insn = next;
@@ -4015,7 +3974,7 @@ static int validate_unrets(struct objtool_file *file) ret =validate_unret (file, insn);if (ret <0 ) { -WARN_FUNC ("Failed UNRET validation" , insn->sec, insn->offset); +WARN_INSN (insn,"Failed UNRET validation" );return ret; } warnings += ret;
@@ -4043,13 +4002,11 @@ static int validate_retpoline(struct objtool_file *file)if (insn->type ==INSN_RETURN ) {if (opts.rethunk) { -WARN_FUNC ("'naked' return found in RETHUNK build" , - insn->sec, insn->offset); +WARN_INSN (insn,"'naked' return found in RETHUNK build" ); }else continue ; }else { -WARN_FUNC ("indirect %s found in RETPOLINE build" , - insn->sec, insn->offset, +WARN_INSN (insn,"indirect %s found in RETPOLINE build" , insn->type ==INSN_JUMP_DYNAMIC ?"jump" :"call" ); }
@@ -4428,9 +4385,7 @@ static int validate_ibt_insn(struct objtool_file *file, struct instruction *insnif (noendbr_range (file, dest))continue ; -WARN_FUNC ("relocation to !ENDBR: %s" , - insn->sec, insn->offset, -offstr (dest->sec, dest->offset)); +WARN_INSN (insn,"relocation to !ENDBR: %s" ,offstr (dest->sec, dest->offset)); warnings++; }
@@ -4532,16 +4487,14 @@ static int validate_sls(struct objtool_file *file)switch (insn->type) {case INSN_RETURN :if (!next_insn || next_insn->type !=INSN_TRAP ) { -WARN_FUNC ("missing int3 after ret" , - insn->sec, insn->offset); +WARN_INSN (insn,"missing int3 after ret" ); warnings++; }break ;case INSN_JUMP_DYNAMIC :if (!next_insn || next_insn->type !=INSN_TRAP ) { -WARN_FUNC ("missing int3 after indirect jump" , - insn->sec, insn->offset); +WARN_INSN (insn,"missing int3 after indirect jump" ); warnings++; }break ;
@@ -4564,7 +4517,7 @@ static int validate_reachable_instructions(struct objtool_file *file)if (insn->visited ||ignore_unreachable_insn (file, insn))continue ; -WARN_FUNC ("unreachable instruction" , insn->sec, insn->offset); +WARN_INSN (insn,"unreachable instruction" );return 1 ; }
tools/objtool/include/objtool/warn.h +5 -0
@@ -53,6 +53,11 @@ static inline char *offstr(struct section *sec, unsigned long offset)free (_str); \ }) +#define WARN_INSN (insn, format, ...) \ +({ \ + WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__); \ +}) +#define BT_FUNC (format, insn, ...) \ ({ \ struct instruction *_insn = (insn); \
tools/objtool/orc_gen.c +3 -6
@@ -47,8 +47,7 @@ static int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, orc->type =ORC_TYPE_REGS_PARTIAL ;break ;default : -WARN_FUNC ("unknown unwind hint type %d" , - insn->sec, insn->offset, cfi->type); +WARN_INSN (insn,"unknown unwind hint type %d" , cfi->type);return -1 ; }
@@ -80,8 +79,7 @@ static int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, orc->sp_reg =ORC_REG_DX ;break ;default : -WARN_FUNC ("unknown CFA base reg %d" , - insn->sec, insn->offset, cfi->cfa.base); +WARN_INSN (insn,"unknown CFA base reg %d" , cfi->cfa.base);return -1 ; }
@@ -96,8 +94,7 @@ static int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, orc->bp_reg =ORC_REG_BP ;break ;default : -WARN_FUNC ("unknown BP base reg %d" , - insn->sec, insn->offset, bp->base); +WARN_INSN (insn,"unknown BP base reg %d" , bp->base);return -1 ; }
Signature · 1 line
2.39.2
[tip: objtool/core] objtool: Add WARN_INSN()
The following commit has been merged into the objtool/core branch of tip:
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: 246b2c85487a7bc5f6a09098e18a96506b1b55df Gitweb: https://git.kernel.org/tip/246b2c85487a7bc5f6a09098e18a96506b1b55df Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 12 Apr 2023 12:03:17 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 14 Apr 2023 16:08:28 +02:00
objtool: Add WARN_INSN()
It's easier to use and also gives easy access to the instruction's containing function, which is useful for printing that function's symbol. It will also be useful in the future for rate-limiting and disassembly of warned functions.
Signed-off-by Josh Poimboeuf <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
--- tools/objtool/check.c | 171 +++++++++----------------- tools/objtool/include/objtool/warn.h | 5 +- tools/objtool/orc_gen.c | 9 +- 3 files changed, 70 insertions(+), 115 deletions(-)
tools/objtool/check.c +62 -109
@@ -1446,7 +1446,7 @@ static void annotate_call_site(struct objtool_file *file,if (opts.mcount && sym->fentry) {if (sibling) -WARN_FUNC ("Tail call to __fentry__ !?!?" , insn->sec, insn->offset); +WARN_INSN (insn,"tail call to __fentry__ !?!?" );if (opts.mnop) {if (reloc) { reloc->type =R_NONE ;
@@ -1648,9 +1648,8 @@ static int add_jump_destinations(struct objtool_file *file)continue ; } -WARN_FUNC ("can't find jump dest instruction at %s+0x%lx" , - insn->sec, insn->offset, dest_sec->name, - dest_off); +WARN_INSN (insn,"can't find jump dest instruction at %s+0x%lx" , + dest_sec->name, dest_off);return -1 ; }
@@ -1733,13 +1732,12 @@ static int add_call_destinations(struct objtool_file *file)continue ;if (!insn_call_dest (insn)) { -WARN_FUNC ("unannotated intra-function call" , insn->sec, insn->offset); +WARN_INSN (insn,"unannotated intra-function call" );return -1 ; }if (insn_func (insn) &&insn_call_dest (insn)->type !=STT_FUNC ) { -WARN_FUNC ("unsupported call to non-function" , - insn->sec, insn->offset); +WARN_INSN (insn,"unsupported call to non-function" );return -1 ; }
@@ -1747,10 +1745,8 @@ static int add_call_destinations(struct objtool_file *file) dest_off =arch_dest_reloc_offset (reloc->addend); dest =find_call_destination (reloc->sym->sec, dest_off);if (!dest) { -WARN_FUNC ("can't find call dest symbol at %s+0x%lx" , - insn->sec, insn->offset, - reloc->sym->sec->name, - dest_off); +WARN_INSN (insn,"can't find call dest symbol at %s+0x%lx" , + reloc->sym->sec->name, dest_off);return -1 ; }
@@ -1810,8 +1806,7 @@ static int handle_group_alt(struct objtool_file *file, }else {if (orig_alt_group->last_insn->offset + orig_alt_group->last_insn->len - orig_alt_group->first_insn->offset != special_alt->orig_len) { - WARN_FUNC("weirdly overlapping alternative! %ld != %d", - orig_insn->sec, orig_insn->offset, + WARN_INSN(orig_insn, "weirdly overlapping alternative! %ld != %d", orig_alt_group->last_insn->offset + orig_alt_group->last_insn->len - orig_alt_group->first_insn->offset,
@@ -1880,8 +1875,7 @@ static int handle_group_alt(struct objtool_file *file,if (alt_reloc &&arch_pc_relative_reloc (alt_reloc) && !arch_support_alt_relocation (special_alt, insn, alt_reloc)) { -WARN_FUNC ("unsupported relocation in alternatives section" , - insn->sec, insn->offset); +WARN_INSN (insn,"unsupported relocation in alternatives section" );return -1 ; }
@@ -1895,8 +1889,7 @@ static int handle_group_alt(struct objtool_file *file,if (dest_off == special_alt->new_off + special_alt->new_len) { insn->jump_dest =next_insn_same_sec (file, orig_alt_group->last_insn);if (!insn->jump_dest) { -WARN_FUNC ("can't find alternative jump destination" , - insn->sec, insn->offset); +WARN_INSN (insn,"can't find alternative jump destination" );return -1 ; } }
@@ -1930,8 +1923,7 @@ static int handle_jump_alt(struct objtool_file *file,if (orig_insn->type !=INSN_JUMP_UNCONDITIONAL && orig_insn->type !=INSN_NOP ) { -WARN_FUNC ("unsupported instruction at jump label" , - orig_insn->sec, orig_insn->offset); +WARN_INSN (orig_insn,"unsupported instruction at jump label" );return -1 ; }
@@ -2010,8 +2002,7 @@ static int add_special_section_alts(struct objtool_file *file)if (special_alt->group) {if (!special_alt->orig_len) { -WARN_FUNC ("empty alternative entry" , - orig_insn->sec, orig_insn->offset); +WARN_INSN (orig_insn,"empty alternative entry" );continue ; }
@@ -2102,8 +2093,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn, }if (!prev_offset) { -WARN_FUNC ("can't find switch jump table" , - insn->sec, insn->offset); +WARN_INSN (insn,"can't find switch jump table" );return -1 ; }
@@ -2307,8 +2297,7 @@ static int read_unwind_hints(struct objtool_file *file)if (sym && sym->bind ==STB_GLOBAL ) {if (opts.ibt && insn->type !=INSN_ENDBR && !insn->noendbr) { -WARN_FUNC ("UNWIND_HINT_IRET_REGS without ENDBR" , - insn->sec, insn->offset); +WARN_INSN (insn,"UNWIND_HINT_IRET_REGS without ENDBR" ); } } }
@@ -2322,8 +2311,7 @@ static int read_unwind_hints(struct objtool_file *file) cfi = *(insn->cfi);if (arch_decode_hint_reg (hint->sp_reg, &cfi.cfa.base)) { -WARN_FUNC ("unsupported unwind_hint sp base reg %d" , - insn->sec, insn->offset, hint->sp_reg); +WARN_INSN (insn,"unsupported unwind_hint sp base reg %d" , hint->sp_reg);return -1 ; }
@@ -2386,8 +2374,7 @@ static int read_retpoline_hints(struct objtool_file *file) insn->type != INSN_CALL_DYNAMIC && insn->type != INSN_RETURN && insn->type != INSN_NOP) { -WARN_FUNC ("retpoline_safe hint not an indirect jump/call/ret/nop" , - insn->sec, insn->offset); +WARN_INSN (insn,"retpoline_safe hint not an indirect jump/call/ret/nop" );return -1 ; }
@@ -2498,8 +2485,7 @@ static int read_intra_function_calls(struct objtool_file *file) }if (insn->type !=INSN_CALL ) { -WARN_FUNC ("intra_function_call not a direct call" , - insn->sec, insn->offset); +WARN_INSN (insn,"intra_function_call not a direct call" );return -1 ; }
@@ -2513,8 +2499,7 @@ static int read_intra_function_calls(struct objtool_file *file) dest_off =arch_jump_destination (insn); insn->jump_dest =find_insn (file, insn->sec, dest_off);if (!insn->jump_dest) { -WARN_FUNC ("can't find call dest at %s+0x%lx" , - insn->sec, insn->offset, +WARN_INSN (insn,"can't find call dest at %s+0x%lx" , insn->sec->name, dest_off);return -1 ; }
@@ -2855,7 +2840,7 @@ static int update_cfi_state(struct instruction *insn,/* stack operations don't make sense with an undefined CFA */ if (cfa->base ==CFI_UNDEFINED ) {if (insn_func (insn)) { -WARN_FUNC ("undefined stack state" , insn->sec, insn->offset); +WARN_INSN (insn,"undefined stack state" );return -1 ; }return 0 ;
@@ -3038,8 +3023,7 @@ static int update_cfi_state(struct instruction *insn, }if (op->dest.reg == cfi->cfa.base && !(next_insn && next_insn->hint)) { -WARN_FUNC ("unsupported stack register modification" , - insn->sec, insn->offset); +WARN_INSN (insn,"unsupported stack register modification" );return -1 ; }
@@ -3049,8 +3033,7 @@ static int update_cfi_state(struct instruction *insn,if (op->dest.reg !=CFI_SP || (cfi->drap_reg !=CFI_UNDEFINED && cfa->base !=CFI_SP ) || (cfi->drap_reg ==CFI_UNDEFINED && cfa->base !=CFI_BP )) { -WARN_FUNC ("unsupported stack pointer realignment" , - insn->sec, insn->offset); +WARN_INSN (insn,"unsupported stack pointer realignment" );return -1 ; }
@@ -3145,8 +3128,7 @@ static int update_cfi_state(struct instruction *insn,break ;default : -WARN_FUNC ("unknown stack-related instruction" , - insn->sec, insn->offset); +WARN_INSN (insn,"unknown stack-related instruction" );return -1 ; }
@@ -3235,8 +3217,7 @@ static int update_cfi_state(struct instruction *insn,case OP_DEST_MEM :if (op->src.type !=OP_SRC_POP && op->src.type !=OP_SRC_POPF ) { -WARN_FUNC ("unknown stack-related memory operation" , - insn->sec, insn->offset); +WARN_INSN (insn,"unknown stack-related memory operation" );return -1 ; }
@@ -3248,8 +3229,7 @@ static int update_cfi_state(struct instruction *insn,break ;default : -WARN_FUNC ("unknown stack-related instruction" , - insn->sec, insn->offset); +WARN_INSN (insn,"unknown stack-related instruction" );return -1 ; }
@@ -3288,8 +3268,7 @@ static int propagate_alt_cfi(struct objtool_file *file, struct instruction *insnstruct alt_group *orig_group = insn->alt_group->orig_group ?: insn->alt_group;struct instruction *orig = orig_group->first_insn;char *where =offstr (insn->sec, insn->offset); -WARN_FUNC ("stack layout conflict in alternatives: %s" , - orig->sec, orig->offset, where); +WARN_INSN (orig,"stack layout conflict in alternatives: %s" , where);free (where);return -1 ; }
@@ -3316,8 +3295,7 @@ static int handle_insn_ops(struct instruction *insn,if (!state->uaccess_stack) { state->uaccess_stack =1 ; }else if (state->uaccess_stack >>31 ) { -WARN_FUNC ("PUSHF stack exhausted" , - insn->sec, insn->offset); +WARN_INSN (insn,"PUSHF stack exhausted" );return 1 ; } state->uaccess_stack <<=1 ;
@@ -3349,8 +3327,7 @@ static bool insn_cfi_match(struct instruction *insn, struct cfi_state *cfi2)if (memcmp (&cfi1->cfa, &cfi2->cfa,sizeof (cfi1->cfa))) { -WARN_FUNC ("stack state mismatch: cfa1=%d%+d cfa2=%d%+d" , - insn->sec, insn->offset, +WARN_INSN (insn,"stack state mismatch: cfa1=%d%+d cfa2=%d%+d" , cfi1->cfa.base, cfi1->cfa.offset, cfi2->cfa.base, cfi2->cfa.offset);
@@ -3360,8 +3337,7 @@ static bool insn_cfi_match(struct instruction *insn, struct cfi_state *cfi2) sizeof(struct cfi_reg)))continue ; -WARN_FUNC ("stack state mismatch: reg1[%d]=%d%+d reg2[%d]=%d%+d" , - insn->sec, insn->offset, +WARN_INSN (insn,"stack state mismatch: reg1[%d]=%d%+d reg2[%d]=%d%+d" , i, cfi1->regs[i].base, cfi1->regs[i].offset, i, cfi2->regs[i].base, cfi2->regs[i].offset);break ;
@@ -3369,15 +3345,14 @@ static bool insn_cfi_match(struct instruction *insn, struct cfi_state *cfi2) }else if (cfi1->type != cfi2->type) { -WARN_FUNC ("stack state mismatch: type1=%d type2=%d" , - insn->sec, insn->offset, cfi1->type, cfi2->type); +WARN_INSN (insn,"stack state mismatch: type1=%d type2=%d" , + cfi1->type, cfi2->type); }else if (cfi1->drap != cfi2->drap || (cfi1->drap && cfi1->drap_reg != cfi2->drap_reg) || (cfi1->drap && cfi1->drap_offset != cfi2->drap_offset)) { -WARN_FUNC ("stack state mismatch: drap1=%d(%d,%d) drap2=%d(%d,%d)" , - insn->sec, insn->offset, +WARN_INSN (insn,"stack state mismatch: drap1=%d(%d,%d) drap2=%d(%d,%d)" , cfi1->drap, cfi1->drap_reg, cfi1->drap_offset, cfi2->drap, cfi2->drap_reg, cfi2->drap_offset);
@@ -3485,20 +3460,17 @@ static int validate_call(struct objtool_file *file, {if (state->noinstr && state->instr <=0 && !noinstr_call_dest (file, insn,insn_call_dest (insn))) { -WARN_FUNC ("call to %s() leaves .noinstr.text section" , - insn->sec, insn->offset,call_dest_name (insn)); +WARN_INSN (insn,"call to %s() leaves .noinstr.text section" ,call_dest_name (insn));return 1 ; }if (state->uaccess && !func_uaccess_safe (insn_call_dest (insn))) { -WARN_FUNC ("call to %s() with UACCESS enabled" , - insn->sec, insn->offset,call_dest_name (insn)); +WARN_INSN (insn,"call to %s() with UACCESS enabled" ,call_dest_name (insn));return 1 ; }if (state->df) { -WARN_FUNC ("call to %s() with DF set" , - insn->sec, insn->offset,call_dest_name (insn)); +WARN_INSN (insn,"call to %s() with DF set" ,call_dest_name (insn));return 1 ; }
@@ -3510,8 +3482,7 @@ static int validate_sibling_call(struct objtool_file *file, struct insn_state *state) {if (insn_func (insn) &&has_modified_stack_frame (insn, state)) { -WARN_FUNC ("sibling call from callable instruction with modified stack frame" , - insn->sec, insn->offset); +WARN_INSN (insn,"sibling call from callable instruction with modified stack frame" );return 1 ; }
@@ -3521,38 +3492,32 @@ static int validate_sibling_call(struct objtool_file *file,static int validate_return (struct symbol *func,struct instruction *insn,struct insn_state *state) {if (state->noinstr && state->instr >0 ) { -WARN_FUNC ("return with instrumentation enabled" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with instrumentation enabled" );return 1 ; }if (state->uaccess && !func_uaccess_safe (func)) { -WARN_FUNC ("return with UACCESS enabled" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with UACCESS enabled" );return 1 ; }if (!state->uaccess &&func_uaccess_safe (func)) { -WARN_FUNC ("return with UACCESS disabled from a UACCESS-safe function" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with UACCESS disabled from a UACCESS-safe function" );return 1 ; }if (state->df) { -WARN_FUNC ("return with DF set" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with DF set" );return 1 ; }if (func &&has_modified_stack_frame (insn, state)) { -WARN_FUNC ("return with modified stack frame" , - insn->sec, insn->offset); +WARN_INSN (insn,"return with modified stack frame" );return 1 ; }if (state->cfi.bp_scratch) { -WARN_FUNC ("BP used as a scratch register" , - insn->sec, insn->offset); +WARN_INSN (insn,"BP used as a scratch register" );return 1 ; }
@@ -3624,8 +3589,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, }if (func && insn->ignore) { -WARN_FUNC ("BUG: why am I validating an ignored function?" , - sec, insn->offset); +WARN_INSN (insn,"BUG: why am I validating an ignored function?" );return 1 ; }
@@ -3658,14 +3622,12 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, }if (!save_insn) { -WARN_FUNC ("no corresponding CFI save for CFI restore" , - sec, insn->offset); +WARN_INSN (insn,"no corresponding CFI save for CFI restore" );return 1 ; }if (!save_insn->visited) { -WARN_FUNC ("objtool isn't smart enough to handle this CFI save/restore combo" , - sec, insn->offset); +WARN_INSN (insn,"objtool isn't smart enough to handle this CFI save/restore combo" );return 1 ; }
@@ -3725,8 +3687,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,if (opts.stackval && func && !is_fentry_call (insn) && !has_valid_stack_frame (&state)) { -WARN_FUNC ("call without frame pointer save/setup" , - sec, insn->offset); +WARN_INSN (insn,"call without frame pointer save/setup" );return 1 ; }
@@ -3772,15 +3733,14 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,case INSN_CONTEXT_SWITCH :if (func && (!next_insn || !next_insn->hint)) { -WARN_FUNC ("unsupported instruction in callable function" , - sec, insn->offset); +WARN_INSN (insn,"unsupported instruction in callable function" );return 1 ; }return 0 ;case INSN_STAC :if (state.uaccess) { -WARN_FUNC ("recursive UACCESS enable" , sec, insn->offset); +WARN_INSN (insn,"recursive UACCESS enable" );return 1 ; }
@@ -3789,12 +3749,12 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,case INSN_CLAC :if (!state.uaccess && func) { -WARN_FUNC ("redundant UACCESS disable" , sec, insn->offset); +WARN_INSN (insn,"redundant UACCESS disable" );return 1 ; }if (func_uaccess_safe (func) && !state.uaccess_stack) { -WARN_FUNC ("UACCESS-safe disables UACCESS" , sec, insn->offset); +WARN_INSN (insn,"UACCESS-safe disables UACCESS" );return 1 ; }
@@ -3803,7 +3763,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,case INSN_STD :if (state.df) { -WARN_FUNC ("recursive STD" , sec, insn->offset); +WARN_INSN (insn,"recursive STD" );return 1 ; }
@@ -3812,7 +3772,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,case INSN_CLD :if (!state.df && func) { -WARN_FUNC ("redundant CLD" , sec, insn->offset); +WARN_INSN (insn,"redundant CLD" );return 1 ; }
@@ -3920,15 +3880,14 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)case INSN_CALL_DYNAMIC :case INSN_JUMP_DYNAMIC :case INSN_JUMP_DYNAMIC_CONDITIONAL : -WARN_FUNC ("early indirect call" , insn->sec, insn->offset); +WARN_INSN (insn,"early indirect call" );return 1 ;case INSN_JUMP_UNCONDITIONAL :case INSN_JUMP_CONDITIONAL :if (!is_sibling_call (insn)) {if (!insn->jump_dest) { -WARN_FUNC ("unresolved jump target after linking?!?" , - insn->sec, insn->offset); +WARN_INSN (insn,"unresolved jump target after linking?!?" );return -1 ; } ret =validate_unret (file, insn->jump_dest);
@@ -3969,7 +3928,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn)return 0 ;case INSN_RETURN : -WARN_FUNC ("RET before UNTRAIN" , insn->sec, insn->offset); +WARN_INSN (insn,"RET before UNTRAIN" );return 1 ;case INSN_NOP :
@@ -3982,7 +3941,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn) }if (!next) { -WARN_FUNC ("teh end!" , insn->sec, insn->offset); +WARN_INSN (insn,"teh end!" );return -1 ; } insn = next;
@@ -4006,7 +3965,7 @@ static int validate_unrets(struct objtool_file *file) ret =validate_unret (file, insn);if (ret <0 ) { -WARN_FUNC ("Failed UNRET validation" , insn->sec, insn->offset); +WARN_INSN (insn,"Failed UNRET validation" );return ret; } warnings += ret;
@@ -4034,13 +3993,11 @@ static int validate_retpoline(struct objtool_file *file)if (insn->type ==INSN_RETURN ) {if (opts.rethunk) { -WARN_FUNC ("'naked' return found in RETHUNK build" , - insn->sec, insn->offset); +WARN_INSN (insn,"'naked' return found in RETHUNK build" ); }else continue ; }else { -WARN_FUNC ("indirect %s found in RETPOLINE build" , - insn->sec, insn->offset, +WARN_INSN (insn,"indirect %s found in RETPOLINE build" , insn->type ==INSN_JUMP_DYNAMIC ?"jump" :"call" ); }
@@ -4419,9 +4376,7 @@ static int validate_ibt_insn(struct objtool_file *file, struct instruction *insnif (noendbr_range (file, dest))continue ; -WARN_FUNC ("relocation to !ENDBR: %s" , - insn->sec, insn->offset, -offstr (dest->sec, dest->offset)); +WARN_INSN (insn,"relocation to !ENDBR: %s" ,offstr (dest->sec, dest->offset)); warnings++; }
@@ -4523,16 +4478,14 @@ static int validate_sls(struct objtool_file *file)switch (insn->type) {case INSN_RETURN :if (!next_insn || next_insn->type !=INSN_TRAP ) { -WARN_FUNC ("missing int3 after ret" , - insn->sec, insn->offset); +WARN_INSN (insn,"missing int3 after ret" ); warnings++; }break ;case INSN_JUMP_DYNAMIC :if (!next_insn || next_insn->type !=INSN_TRAP ) { -WARN_FUNC ("missing int3 after indirect jump" , - insn->sec, insn->offset); +WARN_INSN (insn,"missing int3 after indirect jump" ); warnings++; }break ;
@@ -4555,7 +4508,7 @@ static int validate_reachable_instructions(struct objtool_file *file)if (insn->visited ||ignore_unreachable_insn (file, insn))continue ; -WARN_FUNC ("unreachable instruction" , insn->sec, insn->offset); +WARN_INSN (insn,"unreachable instruction" );return 1 ; }
tools/objtool/include/objtool/warn.h +5 -0
@@ -53,6 +53,11 @@ static inline char *offstr(struct section *sec, unsigned long offset)free (_str); \ }) +#define WARN_INSN (insn, format, ...) \ +({ \ + WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__); \ +}) +#define BT_FUNC (format, insn, ...) \ ({ \ struct instruction *_insn = (insn); \
tools/objtool/orc_gen.c +3 -6
@@ -47,8 +47,7 @@ static int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, orc->type =ORC_TYPE_REGS_PARTIAL ;break ;default : -WARN_FUNC ("unknown unwind hint type %d" , - insn->sec, insn->offset, cfi->type); +WARN_INSN (insn,"unknown unwind hint type %d" , cfi->type);return -1 ; }
@@ -80,8 +79,7 @@ static int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, orc->sp_reg =ORC_REG_DX ;break ;default : -WARN_FUNC ("unknown CFA base reg %d" , - insn->sec, insn->offset, cfi->cfa.base); +WARN_INSN (insn,"unknown CFA base reg %d" , cfi->cfa.base);return -1 ; }
@@ -96,8 +94,7 @@ static int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, orc->bp_reg =ORC_REG_BP ;break ;default : -WARN_FUNC ("unknown BP base reg %d" , - insn->sec, insn->offset, bp->base); +WARN_INSN (insn,"unknown BP base reg %d" , bp->base);return -1 ; }
Josh Poimboeuf
maintainerPATCH v2 3/9 objtool: Limit unreachable warnings to once per function
In reply to Josh PoimboeufUnreachable instruction warnings are limited to once per object file. That no longer makes sense for vmlinux validation, which might have more unreachable instructions lurking in other places. Change it to once per function.
Note this affects some other (much rarer) non-fatal warnings as well. In general I think one-warning-per-function makes sense, as related warnings can accumulate quickly and we want to eventually get back to failing the build with -Werror anyway.
Signed-off-by Josh Poimboeuf <[email protected]>
--- tools/objtool/check.c | 5 +++-- tools/objtool/include/objtool/elf.h | 1 + tools/objtool/include/objtool/warn.h | 5 ++++- 3 files changed, 8 insertions(+), 3 deletions(-)
tools/objtool/check.c +3 -2
@@ -4509,6 +4509,7 @@ static int validate_sls(struct objtool_file *file)static int validate_reachable_instructions (struct objtool_file *file) {struct instruction *insn; +int warnings =0 ;if (file->ignore_unreachables)return 0 ;
@@ -4518,10 +4519,10 @@ static int validate_reachable_instructions(struct objtool_file *file)continue ;WARN_INSN (insn,"unreachable instruction" ); -return 1 ; + warnings++; } -return 0 ; +return warnings; }int check (struct objtool_file *file)
tools/objtool/include/objtool/elf.h +1 -0
@@ -61,6 +61,7 @@ struct symbol { u8 return_thunk : 1; u8 fentry : 1; u8 profiling_func : 1; + u8 warned : 1;struct list_head pv_target;struct list_head reloc_list; };
tools/objtool/include/objtool/warn.h +4 -1
@@ -55,7 +55,10 @@ static inline char *offstr(struct section *sec, unsigned long offset)#define WARN_INSN (insn, format, ...) \ ({ \ - WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__); \ + if (!insn->sym || !insn->sym->warned) \ + WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__);\ + if (insn->sym) \ + insn->sym->warned = 1; \ })#define BT_FUNC (format, insn, ...) \
Signature · 1 line
2.39.2
Peter Zijlstra
maintainerRe: [PATCH v2 3/9] objtool: Limit unreachable warnings to once per function
Quoted 5 lines · Josh Poimboeuf
tools/objtool/include/objtool/warn.h +0
-0
tools/objtool/include/objtool/warn.h static inline char *offstr(struct section *sec, unsigned long offset) · line 62
Quoted 9 lines
tools/objtool/include/objtool/warn.h +4
-1@@ -55,7 +55,10 @@ static inline char *offstr(struct section *sec, unsigned long offset)#define WARN_INSN (insn, format, ...) \ ({ \ - WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__); \ + if (!insn->sym || !insn->sym->warned) \ + WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__);\ + if (insn->sym) \ + insn->sym->warned = 1; \ })
Do we want to write that like:
#define WARN_INSN(insn, format, ...) \
({ \
struct instruction *_insn = (insn); \
if (!_insn->sym || !_insn->sym->warned) \
WARN_FUNC(format, _insn->sec, _insn->offset, ##__VA_ARGS__);\
if (_insn->sym) \
_insn->sym->warned = 1; \
})instead ?
Josh Poimboeuf
maintainerRe: [PATCH v2 3/9] objtool: Limit unreachable warnings to once per function
Quoted 28 lines · Peter Zijlstra
Quoted 5 lines · Josh Poimboeuf
tools/objtool/include/objtool/warn.h +0
-0tools/objtool/include/objtool/warn.h static inline char *offstr(struct section *sec, unsigned long offset) · line 62
Quoted 9 lines
tools/objtool/include/objtool/warn.h +4
-1@@ -55,7 +55,10 @@ static inline char *offstr(struct section *sec, unsigned long offset)#define WARN_INSN (insn, format, ...) \ ({ \ - WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__); \ + if (!insn->sym || !insn->sym->warned) \ + WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__);\ + if (insn->sym) \ + insn->sym->warned = 1; \ })Do we want to write that like:
#define
WARN_INSN(insn, format, ...)\({ \ struct instruction *_insn = (insn); \ if (!_insn->sym || !_insn->sym->warned) \ WARN_FUNC(format, _insn->sec, _insn->offset, ##__VA_ARGS__);\ if (_insn->sym) \ _insn->sym->warned = 1; \ })
Quoted 1 line
instead ?
Yes indeed.
Signature · 1 line
Josh
Josh Poimboeuf
maintainerPATCH v2 4/9 objtool: Add symbol iteration helpers
In reply to Josh PoimboeufAdd [sec_]for_each_sym() and use them.
Signed-off-by Josh Poimboeuf <[email protected]>
--- tools/objtool/check.c | 98 ++++++++++++----------------- tools/objtool/elf.c | 2 +- tools/objtool/include/objtool/elf.h | 9 +++ 3 files changed, 51 insertions(+), 58 deletions(-)
tools/objtool/check.c +41 -57
@@ -470,7 +470,7 @@ static int decode_instructions(struct objtool_file *file)// printf("%s: last chunk used: %d\n", sec->name, (int)idx); -list_for_each_entry (func, &sec->symbol_list, list) { +sec_for_each_sym (sec, func) {if (func->type !=STT_NOTYPE && func->type !=STT_FUNC )continue ;
@@ -924,7 +924,7 @@ static int create_ibt_endbr_seal_sections(struct objtool_file *file)static int create_cfi_sections (struct objtool_file *file) { -struct section *sec, *s; +struct section *sec;struct symbol *sym;unsigned int *loc;int idx;
@@ -937,19 +937,14 @@ static int create_cfi_sections(struct objtool_file *file) } idx =0 ; -for_each_sec (file, s) { -if (!s->text) +for_each_sym (file, sym) { +if (sym->type !=STT_FUNC )continue ; -list_for_each_entry (sym, &s->symbol_list, list) { -if (sym->type !=STT_FUNC ) -continue ; - -if (strncmp (sym->name,"__cfi_" ,6 )) -continue ; +if (strncmp (sym->name,"__cfi_" ,6 )) +continue ; - idx++; - } + idx++; } sec =elf_create_section (file->elf,".cfi_sites" ,0 ,sizeof (unsigned int ), idx);
@@ -957,28 +952,23 @@ static int create_cfi_sections(struct objtool_file *file)return -1 ; idx =0 ; -for_each_sec (file, s) { -if (!s->text) +for_each_sym (file, sym) { +if (sym->type !=STT_FUNC )continue ; -list_for_each_entry (sym, &s->symbol_list, list) { -if (sym->type !=STT_FUNC ) -continue ; - -if (strncmp (sym->name,"__cfi_" ,6 )) -continue ; +if (strncmp (sym->name,"__cfi_" ,6 )) +continue ; - loc = (unsigned int *)sec->data->d_buf + idx; -memset (loc,0 ,sizeof (unsigned int )); + loc = (unsigned int *)sec->data->d_buf + idx; +memset (loc,0 ,sizeof (unsigned int )); -if (elf_add_reloc_to_insn (file->elf, sec, - idx *sizeof (unsigned int ), -R_X86_64_PC32 , - s, sym->offset)) -return -1 ; +if (elf_add_reloc_to_insn (file->elf, sec, + idx *sizeof (unsigned int ), +R_X86_64_PC32 , + sym->sec, sym->offset)) +return -1 ; - idx++; - } + idx++; }return 0 ;
@@ -2205,23 +2195,20 @@ static int add_func_jump_tables(struct objtool_file *file,*/ static int add_jump_table_alts (struct objtool_file *file) { -struct section *sec;struct symbol *func;int ret;if (!file->rodata)return 0 ; -for_each_sec (file, sec) { -list_for_each_entry (func, &sec->symbol_list, list) { -if (func->type !=STT_FUNC ) -continue ; +for_each_sym (file, func) { +if (func->type !=STT_FUNC ) +continue ; -mark_func_jump_tables (file, func); - ret =add_func_jump_tables (file, func); -if (ret) -return ret; - } +mark_func_jump_tables (file, func); + ret =add_func_jump_tables (file, func); +if (ret) +return ret; }return 0 ;
@@ -2533,30 +2520,27 @@ static bool is_profiling_func(const char *name)static int classify_symbols (struct objtool_file *file) { -struct section *sec;struct symbol *func; -for_each_sec (file, sec) { -list_for_each_entry (func, &sec->symbol_list, list) { -if (func->bind !=STB_GLOBAL ) -continue ; +for_each_sym (file, func) { +if (func->bind !=STB_GLOBAL ) +continue ; -if (!strncmp (func->name,STATIC_CALL_TRAMP_PREFIX_STR , -strlen (STATIC_CALL_TRAMP_PREFIX_STR ))) - func->static_call_tramp = true; +if (!strncmp (func->name,STATIC_CALL_TRAMP_PREFIX_STR , +strlen (STATIC_CALL_TRAMP_PREFIX_STR ))) + func->static_call_tramp = true; -if (arch_is_retpoline (func)) - func->retpoline_thunk = true; +if (arch_is_retpoline (func)) + func->retpoline_thunk = true; -if (arch_is_rethunk (func)) - func->return_thunk = true; +if (arch_is_rethunk (func)) + func->return_thunk = true; -if (arch_ftrace_match (func->name)) - func->fentry = true; +if (arch_ftrace_match (func->name)) + func->fentry = true; -if (is_profiling_func (func->name)) - func->profiling_func = true; - } +if (is_profiling_func (func->name)) + func->profiling_func = true; }return 0 ;
@@ -4222,7 +4206,7 @@ static int validate_section(struct objtool_file *file, struct section *sec)struct symbol *func;int warnings =0 ; -list_for_each_entry (func, &sec->symbol_list, list) { +sec_for_each_sym (sec, func) {if (func->type !=STT_FUNC )continue ;
tools/objtool/elf.c +1 -1
@@ -474,7 +474,7 @@ static int read_symbols(struct elf *elf)/* Create parent/child links for any cold subfunctions */ list_for_each_entry (sec, &elf->sections, list) { -list_for_each_entry (sym, &sec->symbol_list, list) { +sec_for_each_sym (sec, sym) {char pname[MAX_NAME_LEN +1 ];size_t pnamelen;if (sym->type !=STT_FUNC )
tools/objtool/include/objtool/elf.h +9 -0
@@ -189,4 +189,13 @@ struct symbol *find_func_containing(struct section *sec, unsigned long offset);#define for_each_sec (file, sec) \ list_for_each_entry(sec, &file->elf->sections, list) +#define sec_for_each_sym (sec, sym) \ + list_for_each_entry(sym, &sec->symbol_list, list) + +#define for_each_sym (file, sym) \ + for (struct section *__sec, *__fake = (struct section *)1; \ + __fake; __fake = NULL) \ + for_each_sec(file, __sec) \ + sec_for_each_sym(__sec, sym) +#endif /* _OBJTOOL_ELF_H */
Signature · 1 line
2.39.2
[tip: objtool/core] objtool: Add symbol iteration helpers
The following commit has been merged into the objtool/core branch of tip:
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: 9290e772baccecec324ae9f2e0b470f870c097de Gitweb: https://git.kernel.org/tip/9290e772baccecec324ae9f2e0b470f870c097de Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 12 Apr 2023 12:03:19 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 14 Apr 2023 16:08:29 +02:00
objtool: Add symbol iteration helpers
Add [sec_]for_each_sym() and use them.
Signed-off-by Josh Poimboeuf <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
--- tools/objtool/check.c | 98 +++++++++++----------------- tools/objtool/elf.c | 2 +- tools/objtool/include/objtool/elf.h | 9 +++- 3 files changed, 51 insertions(+), 58 deletions(-)
tools/objtool/check.c +41 -57
@@ -470,7 +470,7 @@ static int decode_instructions(struct objtool_file *file)// printf("%s: last chunk used: %d\n", sec->name, (int)idx); -list_for_each_entry (func, &sec->symbol_list, list) { +sec_for_each_sym (sec, func) {if (func->type !=STT_NOTYPE && func->type !=STT_FUNC )continue ;
@@ -924,7 +924,7 @@ static int create_ibt_endbr_seal_sections(struct objtool_file *file)static int create_cfi_sections (struct objtool_file *file) { -struct section *sec, *s; +struct section *sec;struct symbol *sym;unsigned int *loc;int idx;
@@ -937,19 +937,14 @@ static int create_cfi_sections(struct objtool_file *file) } idx =0 ; -for_each_sec (file, s) { -if (!s->text) +for_each_sym (file, sym) { +if (sym->type !=STT_FUNC )continue ; -list_for_each_entry (sym, &s->symbol_list, list) { -if (sym->type !=STT_FUNC ) -continue ; - -if (strncmp (sym->name,"__cfi_" ,6 )) -continue ; +if (strncmp (sym->name,"__cfi_" ,6 )) +continue ; - idx++; - } + idx++; } sec =elf_create_section (file->elf,".cfi_sites" ,0 ,sizeof (unsigned int ), idx);
@@ -957,28 +952,23 @@ static int create_cfi_sections(struct objtool_file *file)return -1 ; idx =0 ; -for_each_sec (file, s) { -if (!s->text) +for_each_sym (file, sym) { +if (sym->type !=STT_FUNC )continue ; -list_for_each_entry (sym, &s->symbol_list, list) { -if (sym->type !=STT_FUNC ) -continue ; - -if (strncmp (sym->name,"__cfi_" ,6 )) -continue ; +if (strncmp (sym->name,"__cfi_" ,6 )) +continue ; - loc = (unsigned int *)sec->data->d_buf + idx; -memset (loc,0 ,sizeof (unsigned int )); + loc = (unsigned int *)sec->data->d_buf + idx; +memset (loc,0 ,sizeof (unsigned int )); -if (elf_add_reloc_to_insn (file->elf, sec, - idx *sizeof (unsigned int ), -R_X86_64_PC32 , - s, sym->offset)) -return -1 ; +if (elf_add_reloc_to_insn (file->elf, sec, + idx *sizeof (unsigned int ), +R_X86_64_PC32 , + sym->sec, sym->offset)) +return -1 ; - idx++; - } + idx++; }return 0 ;
@@ -2207,23 +2197,20 @@ static int add_func_jump_tables(struct objtool_file *file,*/ static int add_jump_table_alts (struct objtool_file *file) { -struct section *sec;struct symbol *func;int ret;if (!file->rodata)return 0 ; -for_each_sec (file, sec) { -list_for_each_entry (func, &sec->symbol_list, list) { -if (func->type !=STT_FUNC ) -continue ; +for_each_sym (file, func) { +if (func->type !=STT_FUNC ) +continue ; -mark_func_jump_tables (file, func); - ret =add_func_jump_tables (file, func); -if (ret) -return ret; - } +mark_func_jump_tables (file, func); + ret =add_func_jump_tables (file, func); +if (ret) +return ret; }return 0 ;
@@ -2535,30 +2522,27 @@ static bool is_profiling_func(const char *name)static int classify_symbols (struct objtool_file *file) { -struct section *sec;struct symbol *func; -for_each_sec (file, sec) { -list_for_each_entry (func, &sec->symbol_list, list) { -if (func->bind !=STB_GLOBAL ) -continue ; +for_each_sym (file, func) { +if (func->bind !=STB_GLOBAL ) +continue ; -if (!strncmp (func->name,STATIC_CALL_TRAMP_PREFIX_STR , -strlen (STATIC_CALL_TRAMP_PREFIX_STR ))) - func->static_call_tramp = true; +if (!strncmp (func->name,STATIC_CALL_TRAMP_PREFIX_STR , +strlen (STATIC_CALL_TRAMP_PREFIX_STR ))) + func->static_call_tramp = true; -if (arch_is_retpoline (func)) - func->retpoline_thunk = true; +if (arch_is_retpoline (func)) + func->retpoline_thunk = true; -if (arch_is_rethunk (func)) - func->return_thunk = true; +if (arch_is_rethunk (func)) + func->return_thunk = true; -if (arch_ftrace_match (func->name)) - func->fentry = true; +if (arch_ftrace_match (func->name)) + func->fentry = true; -if (is_profiling_func (func->name)) - func->profiling_func = true; - } +if (is_profiling_func (func->name)) + func->profiling_func = true; }return 0 ;
@@ -4213,7 +4197,7 @@ static int validate_section(struct objtool_file *file, struct section *sec)struct symbol *func;int warnings =0 ; -list_for_each_entry (func, &sec->symbol_list, list) { +sec_for_each_sym (sec, func) {if (func->type !=STT_FUNC )continue ;
tools/objtool/elf.c +1 -1
@@ -474,7 +474,7 @@ static int read_symbols(struct elf *elf)/* Create parent/child links for any cold subfunctions */ list_for_each_entry (sec, &elf->sections, list) { -list_for_each_entry (sym, &sec->symbol_list, list) { +sec_for_each_sym (sec, sym) {char pname[MAX_NAME_LEN +1 ];size_t pnamelen;if (sym->type !=STT_FUNC )
tools/objtool/include/objtool/elf.h +9 -0
@@ -188,4 +188,13 @@ struct symbol *find_func_containing(struct section *sec, unsigned long offset);#define for_each_sec (file, sec) \ list_for_each_entry(sec, &file->elf->sections, list) +#define sec_for_each_sym (sec, sym) \ + list_for_each_entry(sym, &sec->symbol_list, list) + +#define for_each_sym (file, sym) \ + for (struct section *__sec, *__fake = (struct section *)1; \ + __fake; __fake = NULL) \ + for_each_sec(file, __sec) \ + sec_for_each_sym(__sec, sym) +#endif /* _OBJTOOL_ELF_H */
Josh Poimboeuf
maintainerPATCH v2 5/9 objtool: Add verbose option for disassembling affected functions
In reply to Josh PoimboeufWhen a warning is associated with a function, add an option to disassemble that function.
This makes it easier for reporters to submit the information needed to diagnose objtool warnings.
Signed-off-by Josh Poimboeuf <[email protected]>
--- tools/objtool/Documentation/objtool.txt | 5 ++ tools/objtool/builtin-check.c | 5 ++ tools/objtool/check.c | 77 +++++++++++++++++++++++++ tools/objtool/include/objtool/builtin.h | 1 + 4 files changed, 88 insertions(+)
tools/objtool/Documentation/objtool.txt +5 -0
@@ -244,6 +244,11 @@ To achieve the validation, objtool enforces the following rules: Objtool warnings ---------------- +NOTE: When requesting help with an objtool warning, please recreate with +OBJTOOL_VERBOSE=1 (e.g., "make OBJTOOL_VERBOSE=1") and send the full +output, including any disassembly below the warning, to the objtool +maintainers. + For asm files, if you're getting an error which doesn't make sense, first make sure that the affected code follows the above rules.
tools/objtool/builtin-check.c +5 -0
@@ -93,6 +93,7 @@ static const struct option check_options[] = {OPT_BOOLEAN (0 ,"no-unreachable" , &opts.no_unreachable,"skip 'unreachable instruction' warnings" ),OPT_BOOLEAN (0 ,"sec-address" , &opts.sec_address,"print section addresses in warnings" ),OPT_BOOLEAN (0 ,"stats" , &opts.stats,"print statistics" ), +OPT_BOOLEAN ('v' ,"verbose" , &opts.verbose,"verbose warnings" ), OPT_END(), };
@@ -118,6 +119,10 @@ int cmd_parse_options(int argc, const char **argv, const char * const usage[])parse_options (envc, envv, check_options, env_usage,0 ); } + env =getenv ("OBJTOOL_VERBOSE" ); +if (env && !strcmp (env,"1" )) + opts.verbose = true; + argc =parse_options (argc, argv, check_options, usage,0 );if (argc !=1 )usage_with_options (usage, check_options);
tools/objtool/check.c +77 -0
89 lines not on this page · raw message
tools/objtool/include/objtool/builtin.h +1 -0
@@ -37,6 +37,7 @@ struct opts {bool no_unreachable;bool sec_address;bool stats; +bool verbose; };extern struct opts opts;
Signature · 1 line
2.39.2
Peter Zijlstra
maintainerRe: [PATCH v2 5/9] objtool: Add verbose option for disassembling affected functions
Quoted 43 lines · Josh Poimboeuf
When a warning is associated with a function, add an option to disassemble that function.
This makes it easier for reporters to submit the information needed to diagnose objtool warnings.
Signed-off-by Josh Poimboeuf <[email protected]>--- tools/objtool/Documentation/objtool.txt | 5 ++ tools/objtool/builtin-check.c | 5 ++ tools/objtool/check.c | 77 +++++++++++++++++++++++++ tools/objtool/include/objtool/builtin.h | 1 + 4 files changed, 88 insertions(+)tools/objtool/Documentation/objtool.txt +5
-0@@ -244,6 +244,11 @@ To achieve the validation, objtool enforces the following rules: Objtool warnings ---------------- +NOTE: When requesting help with an objtool warning, please recreate with +OBJTOOL_VERBOSE=1 (e.g., "make OBJTOOL_VERBOSE=1") and send the full +output, including any disassembly below the warning, to the objtool +maintainers. + For asm files, if you're getting an error which doesn't make sense, first make sure that the affected code follows the above rules.tools/objtool/builtin-check.c +1
-0@@ -93,6 +93,7 @@ static const struct option check_options[] = {OPT_BOOLEAN (0 ,"no-unreachable" , &opts.no_unreachable,"skip 'unreachable instruction' warnings" ),OPT_BOOLEAN (0 ,"sec-address" , &opts.sec_address,"print section addresses in warnings" ),OPT_BOOLEAN (0 ,"stats" , &opts.stats,"print statistics" ), +OPT_BOOLEAN ('v' ,"verbose" , &opts.verbose,"verbose warnings" ), OPT_END(), };
tools/objtool/builtin-check.c int cmd_parse_options(int argc, const char **argv, const char * const usage[]) · line 128
Quoted 10 lines
tools/objtool/builtin-check.c +4
-0@@ -118,6 +119,10 @@ int cmd_parse_options(int argc, const char **argv, const char * const usage[])parse_options (envc, envv, check_options, env_usage,0 ); } + env =getenv ("OBJTOOL_VERBOSE" ); +if (env && !strcmp (env,"1" )) + opts.verbose = true; + argc =parse_options (argc, argv, check_options, usage,0 );if (argc !=1 )usage_with_options (usage, check_options);
No real objection; but I do feel obliged to point out that:
OBJTOOL_ARGS="-v" achieves much the same.
Josh Poimboeuf
maintainerRe: [PATCH v2 5/9] objtool: Add verbose option for disassembling affected functions
Quoted 4 lines · Peter Zijlstra
Quoted 4 lines
tools/objtool/builtin-check.c +0
-0@@ -118,6 +119,10 @@ int cmd_parse_options(int argc, const char **argv, const char * const usage[])parse_options (envc, envv, check_options, env_usage,0 ); }
Quoted 10 lines
tools/objtool/builtin-check.c int cmd_parse_options(int argc, const char **argv, const char * const usage[]) · line 128
Quoted 7 lines
tools/objtool/builtin-check.c +4
-0+ env =getenv ("OBJTOOL_VERBOSE" ); +if (env && !strcmp (env,"1" )) + opts.verbose = true; + argc =parse_options (argc, argv, check_options, usage,0 );if (argc !=1 )usage_with_options (usage, check_options);No real objection; but I do feel obliged to point out that:
OBJTOOL_ARGS="-v" achieves much the same.
But OBJTOOL_VERBOSE=1 just rolls off the fingers :-)
Signature · 1 line
Josh
Peter Zijlstra
maintainerRe: [PATCH v2 5/9] objtool: Add verbose option for disassembling affected functions
In reply to Josh PoimboeufQuoted 10 lines · Josh Poimboeuf
tools/objtool/check.c +10
-0+ objdump_str ="%sobjdump -wdr %s | gawk -M -v _funcs='%s' '" +"BEGIN { split(_funcs, funcs); }" +"/^$/ { func_match = 0; }" +"/<.*>:/ { " +"f = gensub(/.*<(.*)>:/, \"\\\\1\", 1);" +"for (i in funcs) {" +"if (funcs[i] == f) {" +"func_match = 1;" +"base = strtonum(\"0x\" $1);" +"break;"
tools/objtool/check.c static int validate_reachable_instructions(struct objtool_file *file) · line 4540
Quoted 10 lines
tools/objtool/check.c +10
-0+"}" +"}" +"}" +"{" +"if (func_match) {" +"addr = strtonum(\"0x\" $1);" +"printf(\"%%04x \", addr - base);" +"print;" +"}" +"}' 1>&2" ;
Do we want to have scripts/objdump-func.awk and use that to avoid the duplication and eventual divergence of these awk thingies?
Re: [PATCH v2 5/9] objtool: Add verbose option for disassembling affected functions
Quoted 16 lines · Peter Zijlstra
Quoted 15 lines · Josh Poimboeuf
tools/objtool/check.c +15
-0+ objdump_str ="%sobjdump -wdr %s | gawk -M -v _funcs='%s' '" +"BEGIN { split(_funcs, funcs); }" +"/^$/ { func_match = 0; }" +"/<.*>:/ { " +"f = gensub(/.*<(.*)>:/, \"\\\\1\", 1);" +"for (i in funcs) {" +"if (funcs[i] == f) {" +"func_match = 1;" +"base = strtonum(\"0x\" $1);" +"break;" +"}" +"}" +"}" +"{" +"if (func_match) {"
Quoted 8 lines
tools/objtool/check.c static int validate_reachable_instructions(struct objtool_file *file) · line 4540
Quoted 5 lines
tools/objtool/check.c +5
-0+"addr = strtonum(\"0x\" $1);" +"printf(\"%%04x \", addr - base);" +"print;" +"}" +"}' 1>&2" ;Do we want to have
scripts/objdump-func.awkand use that to avoid the duplication and eventual divergence of these awk thingies?
I vote for that as well. To keep everything in sync can be a nightmare.
Miroslav
Josh Poimboeuf
maintainerRe: [PATCH v2 5/9] objtool: Add verbose option for disassembling affected functions
Quoted 27 lines · Miroslav Benes
Quoted 16 lines · Peter Zijlstra
Quoted 15 lines · Josh Poimboeuf
tools/objtool/check.c +15
-0+ objdump_str ="%sobjdump -wdr %s | gawk -M -v _funcs='%s' '" +"BEGIN { split(_funcs, funcs); }" +"/^$/ { func_match = 0; }" +"/<.*>:/ { " +"f = gensub(/.*<(.*)>:/, \"\\\\1\", 1);" +"for (i in funcs) {" +"if (funcs[i] == f) {" +"func_match = 1;" +"base = strtonum(\"0x\" $1);" +"break;" +"}" +"}" +"}" +"{" +"if (func_match) {" Quoted 8 lines
tools/objtool/check.c static int validate_reachable_instructions(struct objtool_file *file) · line 4540
Quoted 5 lines
tools/objtool/check.c +5
-0+"addr = strtonum(\"0x\" $1);" +"printf(\"%%04x \", addr - base);" +"print;" +"}" +"}' 1>&2" ;Do we want to have
scripts/objdump-func.awkand use that to avoid the duplication and eventual divergence of these awk thingies?
Quoted 1 line
I vote for that as well. To keep everything in sync can be a nightmare.
Reasons I did it this way for v2:
1) The awk implementation is slightly different (this one doesn't match *.cold, etc)
2) Objtool is self-sufficient (doesn't need any other files)
3) Any changes to the objdump-func interface might break objtool
4) It's clearly bug-free and self-contained and I don't expect the code to change much in the future
Signature · 1 line
Josh
Josh Poimboeuf
maintainerPATCH v2 6/9 objtool: Include backtrace in verbose mode
In reply to Josh PoimboeufInclude backtrace in verbose mode. This makes it easy to gather all the information needed for diagnosing objtool warnings.
Signed-off-by Josh Poimboeuf <[email protected]>
--- tools/objtool/Documentation/objtool.txt | 4 ++-- tools/objtool/check.c | 26 ++++++++++--------------- tools/objtool/include/objtool/warn.h | 14 +++++++------ 3 files changed, 20 insertions(+), 24 deletions(-)
tools/objtool/Documentation/objtool.txt +2 -2
@@ -246,8 +246,8 @@ Objtool warnings NOTE: When requesting help with an objtool warning, please recreate with OBJTOOL_VERBOSE=1 (e.g., "make OBJTOOL_VERBOSE=1") and send the full -output, including any disassembly below the warning, to the objtool -maintainers. +output, including any disassembly or backtrace below the warning, to the +objtool maintainers. For asm files, if you're getting an error which doesn't make sense, first make sure that the affected code follows the above rules.
tools/objtool/check.c +10 -16
@@ -3654,8 +3654,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, ret =validate_branch (file, func, alt->insn, state);if (ret) { -if (opts.backtrace) -BT_FUNC ("(alt)" , insn); +BT_INSN (insn,"(alt)" );return ret; } }
@@ -3700,8 +3699,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, ret =validate_branch (file, func, insn->jump_dest, state);if (ret) { -if (opts.backtrace) -BT_FUNC ("(branch)" , insn); +BT_INSN (insn,"(branch)" );return ret; } }
@@ -3799,8 +3797,8 @@ static int validate_unwind_hint(struct objtool_file *file, {if (insn->hint && !insn->visited && !insn->ignore) {int ret =validate_branch (file,insn_func (insn), insn, *state); -if (ret && opts.backtrace) -BT_FUNC ("<=== (hint)" , insn); +if (ret) +BT_INSN (insn,"<=== (hint)" );return ret; }
@@ -3858,8 +3856,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn) ret =validate_unret (file, alt->insn);if (ret) { -if (opts.backtrace) -BT_FUNC ("(alt)" , insn); +BT_INSN (insn,"(alt)" );return ret; } }
@@ -3885,10 +3882,8 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn) } ret =validate_unret (file, insn->jump_dest);if (ret) { -if (opts.backtrace) { -BT_FUNC ("(branch%s)" , insn, - insn->type ==INSN_JUMP_CONDITIONAL ?"-cond" :"" ); - } +BT_INSN (insn,"(branch%s)" , + insn->type ==INSN_JUMP_CONDITIONAL ?"-cond" :"" );return ret; }
@@ -3910,8 +3905,7 @@ static int validate_unret(struct objtool_file *file, struct instruction *insn) ret =validate_unret (file, dest);if (ret) { -if (opts.backtrace) -BT_FUNC ("(call)" , insn); +BT_INSN (insn,"(call)" );return ret; }/*
@@ -4195,8 +4189,8 @@ static int validate_symbol(struct objtool_file *file, struct section *sec, state->uaccess = sym->uaccess_safe; ret =validate_branch (file,insn_func (insn), insn, *state); -if (ret && opts.backtrace) -BT_FUNC ("<=== (sym)" , insn); +if (ret) +BT_INSN (insn,"<=== (sym)" );return ret; }
tools/objtool/include/objtool/warn.h +8 -6
@@ -61,12 +61,14 @@ static inline char *offstr(struct section *sec, unsigned long offset) insn->sym->warned =1 ; \ }) -#define BT_FUNC (format, insn, ...) \ -({ \ - struct instruction *_insn = (insn); \ - char *_str = offstr(_insn->sec, _insn->offset); \ - WARN(" %s: " format, _str, ##__VA_ARGS__); \ - free(_str); \ +#define BT_INSN (insn, format, ...) \ +({ \ + if (opts.verbose || opts.backtrace) { \ + struct instruction *_insn = (insn); \ + char *_str = offstr(_insn->sec, _insn->offset); \ + WARN(" %s: " format, _str, ##__VA_ARGS__); \ + free(_str); \ + } \ })#define WARN_ELF (format, ...) \
Signature · 1 line
2.39.2
Josh Poimboeuf
maintainerPATCH v2 7/9 objtool: Remove superfluous dead_end_function() check
In reply to Josh Poimboeufannotate_call_site() already sets 'insn->dead_end' for calls to dead end functions.
Signed-off-by Josh Poimboeuf <[email protected]>
--- tools/objtool/check.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
tools/objtool/check.c +1 -2
@@ -4081,8 +4081,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio* It may also insert a UD2 after calling a __noreturn function. */ prev_insn =prev_insn_same_sec (file, insn); -if ((prev_insn->dead_end || -dead_end_function (file,insn_call_dest (prev_insn))) && +if (prev_insn->dead_end && (insn->type ==INSN_BUG || (insn->type ==INSN_JUMP_UNCONDITIONAL && insn->jump_dest && insn->jump_dest->type ==INSN_BUG )))
Signature · 1 line
2.39.2
[tip: objtool/core] objtool: Remove superfluous dead_end_function() check
The following commit has been merged into the objtool/core branch of tip:
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: 6126ed5dfbc656374e851bfdfb128f3aa9e1263a Gitweb: https://git.kernel.org/tip/6126ed5dfbc656374e851bfdfb128f3aa9e1263a Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 12 Apr 2023 12:03:22 -07:00 Committer: Peter Zijlstra <[email protected]> CommitterDate: Fri, 14 Apr 2023 16:08:29 +02:00
objtool: Remove superfluous dead_end_function() check
annotate_call_site() already sets 'insn->dead_end' for calls to dead end functions.
Signed-off-by Josh Poimboeuf <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
--- tools/objtool/check.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
tools/objtool/check.c +1 -2
@@ -4078,8 +4078,7 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio* It may also insert a UD2 after calling a __noreturn function. */ prev_insn =prev_insn_same_sec (file, insn); -if ((prev_insn->dead_end || -dead_end_function (file,insn_call_dest (prev_insn))) && +if (prev_insn->dead_end && (insn->type ==INSN_BUG || (insn->type ==INSN_JUMP_UNCONDITIONAL && insn->jump_dest && insn->jump_dest->type ==INSN_BUG )))
Josh Poimboeuf
maintainerPATCH v2 8/9 objtool: Detect missing __noreturn annotations
In reply to Josh PoimboeufMost "unreachable instruction" warnings these days seem to actually be the result of a missing __noreturn annotation. Add an explicit check for that.
Signed-off-by Josh Poimboeuf <[email protected]>
--- tools/objtool/Documentation/objtool.txt | 6 ++++++ tools/objtool/check.c | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-)
tools/objtool/Documentation/objtool.txt +6 -0
@@ -303,6 +303,12 @@ the objtool maintainers. If it's not actually in a callable function (e.g. kernel entry code), change ENDPROC to END. +3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation + + The call from foo() to bar() doesn't return, but bar() is missing the + __noreturn annotation. NOTE: In addition to adding the __noreturn + annotation, the function name also needs to be added to + 'global_noreturns' in tools/objtool/check.c. 4. file.o: warning: objtool: func(): can't find starting instruction or
tools/objtool/check.c +13 -1
@@ -4485,7 +4485,8 @@ static int validate_sls(struct objtool_file *file)static int validate_reachable_instructions (struct objtool_file *file) { -struct instruction *insn; +struct instruction *insn, *prev_insn; +struct symbol *call_dest;int warnings =0 ;if (file->ignore_unreachables)
@@ -4495,6 +4496,17 @@ static int validate_reachable_instructions(struct objtool_file *file)if (insn->visited ||ignore_unreachable_insn (file, insn))continue ; + prev_insn =prev_insn_same_sec (file, insn); +if (prev_insn && prev_insn->dead_end) { + call_dest =insn_call_dest (prev_insn); +if (call_dest) { +WARN_INSN (insn,"%s() is missing a __noreturn annotation" , + call_dest->name); + warnings++; +continue ; + } + } +WARN_INSN (insn,"unreachable instruction" ); warnings++; }
Signature · 1 line
2.39.2
Peter Zijlstra
maintainerRe: [PATCH v2 8/9] objtool: Detect missing __noreturn annotations
Quoted 15 lines · Josh Poimboeuf
Most "unreachable instruction" warnings these days seem to actually be the result of a missing
__noreturnannotation. Add an explicit check for that.Signed-off-by Josh Poimboeuf <[email protected]>--- tools/objtool/Documentation/objtool.txt | 6 ++++++ tools/objtool/check.c | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-)tools/objtool/Documentation/objtool.txt +0
-0
tools/objtool/Documentation/objtool.txt the objtool maintainers. · line 311
Quoted 9 lines
tools/objtool/Documentation/objtool.txt +6
-0@@ -303,6 +303,12 @@ the objtool maintainers. If it's not actually in a callable function (e.g. kernel entry code), change ENDPROC to END. +3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation + + The call from foo() to bar() doesn't return, but bar() is missing the + __noreturn annotation. NOTE: In addition to adding the __noreturn + annotation, the function name also needs to be added to + 'global_noreturns' in tools/objtool/check.c.
Do we want something like the below (except perhaps less horrible) ?
--- tools/objtool/Makefile | 1 + tools/objtool/check.c | 27 +-------------------------- tools/objtool/noreturns | 26 ++++++++++++++++++++++++++ tools/objtool/noreturns.sh | 7 +++++++ 4 files changed, 35 insertions(+), 26 deletions(-)
tools/objtool/Makefile +1 -0
@@ -63,6 +63,7 @@ include $(srctree)/tools/build/Makefile.include $(OBJTOOL_IN): fixdep $(LIBSUBCMD) FORCE $(Q)$(CONFIG_SHELL) ./sync-check.sh + $(Q)$(CONFIG_SHELL) ./noreturns.sh $(Q)$(MAKE) $(build)=objtool $(HOST_OVERRIDES) CFLAGS="$(OBJTOOL_CFLAGS)" \ LDFLAGS="$(OBJTOOL_LDFLAGS)"
tools/objtool/check.c +1 -26
@@ -197,32 +197,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,* attribute isn't provided in ELF data. Keep 'em sorted. */ static const char *const global_noreturns[] = { -"__invalid_creds" , -"__module_put_and_kthread_exit" , -"__reiserfs_panic" , -"__stack_chk_fail" , -"__ubsan_handle_builtin_unreachable" , -"cpu_bringup_and_idle" , -"cpu_startup_entry" , -"do_exit" , -"do_group_exit" , -"do_task_dead" , -"ex_handler_msr_mce" , -"fortify_panic" , -"kthread_complete_and_exit" , -"kthread_exit" , -"kunit_try_catch_throw" , -"lbug_with_loc" , -"machine_real_restart" , -"make_task_dead" , -"panic" , -"rewind_stack_and_make_dead" , -"sev_es_terminate" , -"snp_abort" , -"stop_this_cpu" , -"usercopy_abort" , -"xen_cpu_bringup_again" , -"xen_start_kernel" , +#include "noreturns.h" };if (!func)
tools/objtool/noreturns new file +26 -0
@@ -0,0 +1,26 @@ +__invalid_creds +__module_put_and_kthread_exit +__reiserfs_panic +__stack_chk_fail +__ubsan_handle_builtin_unreachable +cpu_bringup_and_idle +cpu_startup_entry +do_exit +do_group_exit +do_task_dead +ex_handler_msr_mce +fortify_panic +kthread_complete_and_exit +kthread_exit +kunit_try_catch_throw +lbug_with_loc +machine_real_restart +make_task_dead +panic +rewind_stack_and_make_dead +sev_es_terminate +snp_abort +stop_this_cpu +usercopy_abort +xen_cpu_bringup_again +xen_start_kernel
tools/objtool/noreturns.sh new file +7 -0
@@ -0,0 +1,7 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 + +cat noreturns | while read func +do + echo \"${func}\", +done > noreturns.h
Josh Poimboeuf
maintainerRe: [PATCH v2 8/9] objtool: Detect missing __noreturn annotations
Quoted 19 lines · Peter Zijlstra
Quoted 18 lines · Josh Poimboeuf
Most "unreachable instruction" warnings these days seem to actually be the result of a missing
__noreturnannotation. Add an explicit check for that.Signed-off-by Josh Poimboeuf <[email protected]>--- tools/objtool/Documentation/objtool.txt | 6 ++++++ tools/objtool/check.c | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-)tools/objtool/Documentation/objtool.txt +0
-0@@ -303,6 +303,12 @@ the objtool maintainers. If it's not actually in a callable function (e.g. kernel entry code), change ENDPROC to END.
Quoted 8 lines
tools/objtool/Documentation/objtool.txt the objtool maintainers. · line 311
Quoted 6 lines
tools/objtool/Documentation/objtool.txt +6
-0+3. file.o: warning: objtool: foo+0x48c: bar() is missing a __noreturn annotation + + The call from foo() to bar() doesn't return, but bar() is missing the + __noreturn annotation. NOTE: In addition to adding the __noreturn + annotation, the function name also needs to be added to + 'global_noreturns' in tools/objtool/check.c.Do we want something like the below (except perhaps less horrible) ?
Yeah, maybe. Another possible way to do it:
tools/objtool/check.c +4 -27
@@ -192,39 +192,16 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,struct instruction *insn;bool empty = true; +#define NORETURN (func) "\"" __stringify(func) "\"", +/* * Unfortunately these have to be hard coded because the noreturn * attribute isn't provided in ELF data. Keep 'em sorted. */ static const char *const global_noreturns[] = { -"__invalid_creds" , -"__module_put_and_kthread_exit" , -"__reiserfs_panic" , -"__stack_chk_fail" , -"__ubsan_handle_builtin_unreachable" , -"arch_cpu_idle_dead" , -"cpu_bringup_and_idle" , -"cpu_startup_entry" , -"do_exit" , -"do_group_exit" , -"do_task_dead" , -"ex_handler_msr_mce" , -"fortify_panic" , -"kthread_complete_and_exit" , -"kthread_exit" , -"kunit_try_catch_throw" , -"lbug_with_loc" , -"machine_real_restart" , -"make_task_dead" , -"panic" , -"rewind_stack_and_make_dead" , -"sev_es_terminate" , -"snp_abort" , -"stop_this_cpu" , -"usercopy_abort" , -"xen_cpu_bringup_again" , -"xen_start_kernel" , +#include "noreturns.h" }; +#undef NORETURNif (!func)return false;
tools/objtool/noreturns.h new file +27 -0
@@ -0,0 +1,27 @@ +NORETURN(__invalid_creds) +NORETURN(__module_put_and_kthread_exit) +NORETURN(__reiserfs_panic) +NORETURN(__stack_chk_fail) +NORETURN(__ubsan_handle_builtin_unreachable) +NORETURN(arch_cpu_idle_dead) +NORETURN(cpu_bringup_and_idle) +NORETURN(cpu_startup_entry) +NORETURN(do_exit) +NORETURN(do_group_exit) +NORETURN(do_task_dead) +NORETURN(ex_handler_msr_mce) +NORETURN(fortify_panic) +NORETURN(kthread_complete_and_exit) +NORETURN(kthread_exit) +NORETURN(kunit_try_catch_throw) +NORETURN(lbug_with_loc) +NORETURN(machine_real_restart) +NORETURN(make_task_dead) +NORETURN(panic) +NORETURN(rewind_stack_and_make_dead) +NORETURN(sev_es_terminate) +NORETURN(snp_abort) +NORETURN(stop_this_cpu) +NORETURN(usercopy_abort) +NORETURN(xen_cpu_bringup_again) +NORETURN(xen_start_kernel)
Peter Zijlstra
maintainerRe: [PATCH v2 8/9] objtool: Detect missing __noreturn annotations
Quoted 23 lines
tools/objtool/noreturns.h new file +17
-0@@ -0,0 +1,27 @@ +NORETURN(__invalid_creds) +NORETURN(__module_put_and_kthread_exit) +NORETURN(__reiserfs_panic) +NORETURN(__stack_chk_fail) +NORETURN(__ubsan_handle_builtin_unreachable) +NORETURN(arch_cpu_idle_dead) +NORETURN(cpu_bringup_and_idle) +NORETURN(cpu_startup_entry) +NORETURN(do_exit) +NORETURN(do_group_exit) +NORETURN(do_task_dead) +NORETURN(ex_handler_msr_mce) +NORETURN(fortify_panic) +NORETURN(kthread_complete_and_exit) +NORETURN(kthread_exit) +NORETURN(kunit_try_catch_throw) +NORETURN(lbug_with_loc)
tools/objtool/noreturns.h line 27
Quoted 10 lines
tools/objtool/noreturns.h new file +10
-0+NORETURN(machine_real_restart) +NORETURN(make_task_dead) +NORETURN(panic) +NORETURN(rewind_stack_and_make_dead) +NORETURN(sev_es_terminate) +NORETURN(snp_abort) +NORETURN(stop_this_cpu) +NORETURN(usercopy_abort) +NORETURN(xen_cpu_bringup_again) +NORETURN(xen_start_kernel)
Not as convenient to edit, but much easier to use. A bit of a toss up I suppose.
Re: [PATCH v2 8/9] objtool: Detect missing __noreturn annotations
In reply to Josh PoimboeufQuoted 14 lines
tools/objtool/check.c +2
-1@@ -4485,7 +4485,8 @@ static int validate_sls(struct objtool_file *file)static int validate_reachable_instructions (struct objtool_file *file) { -struct instruction *insn; +struct instruction *insn, *prev_insn; +struct symbol *call_dest;int warnings =0 ;if (file->ignore_unreachables)@@ -4495,6 +4496,17 @@ static int validate_reachable_instructions(struct objtool_file *file)if (insn->visited ||ignore_unreachable_insn (file, insn))
tools/objtool/check.c static int validate_reachable_instructions(struct objtool_file *file) · line 4506
Quoted 10 lines
tools/objtool/check.c +8
-0continue ; + prev_insn =prev_insn_same_sec (file, insn); +if (prev_insn && prev_insn->dead_end) { + call_dest =insn_call_dest (prev_insn); +if (call_dest) { +WARN_INSN (insn,"%s() is missing a __noreturn annotation" , + call_dest->name); + warnings++; +continue ;
A nit but this and
tools/objtool/check.c static int validate_reachable_instructions(struct objtool_file *file) · line 4511
Quoted 5 lines
tools/objtool/check.c +3
-0+ } + } +WARN_INSN (insn,"unreachable instruction" ); warnings++;
this makes me thinking. Wouldn't it be confusing to anyone that there is no correspondence between warnings and a number of actual reported warnings through WARN_INSN()? In the future when there would be a usage for warnings. It does not really matter now.
Miroslav
Josh Poimboeuf
maintainerRe: [PATCH v2 8/9] objtool: Detect missing __noreturn annotations
Quoted 27 lines · Miroslav Benes
Quoted 14 lines
tools/objtool/check.c +2
-1@@ -4485,7 +4485,8 @@ static int validate_sls(struct objtool_file *file)static int validate_reachable_instructions (struct objtool_file *file) { -struct instruction *insn; +struct instruction *insn, *prev_insn; +struct symbol *call_dest;int warnings =0 ;if (file->ignore_unreachables)@@ -4495,6 +4496,17 @@ static int validate_reachable_instructions(struct objtool_file *file)if (insn->visited ||ignore_unreachable_insn (file, insn))tools/objtool/check.c static int validate_reachable_instructions(struct objtool_file *file) · line 4506
Quoted 10 lines
tools/objtool/check.c +8
-0continue ; + prev_insn =prev_insn_same_sec (file, insn); +if (prev_insn && prev_insn->dead_end) { + call_dest =insn_call_dest (prev_insn); +if (call_dest) { +WARN_INSN (insn,"%s() is missing a __noreturn annotation" , + call_dest->name); + warnings++; +continue ;A nit but this and
Quoted 10 lines
tools/objtool/check.c static int validate_reachable_instructions(struct objtool_file *file) · line 4511
Quoted 5 lines
tools/objtool/check.c +3
-0+ } + } +WARN_INSN (insn,"unreachable instruction" ); warnings++;this makes me thinking. Wouldn't it be confusing to anyone that there is no correspondence between warnings and a number of actual reported warnings through
WARN_INSN()? In the future when there would be a usage for warnings. It does not really matter now.
True, maybe we need WARN_INSN_ONCE_PER_FUNC() or so ;-)
Signature · 1 line
Josh
Josh Poimboeuf
maintainerPATCH v2 9/9 objtool: Ignore exc_double_fault() __noreturn warnings
In reply to Josh PoimboeufThis is a hack, but it works for now.
Problem is, exc_double_fault() may or may not return, depending on whether CONFIG_X86_ESPFIX64 is set. But objtool has no visibility to the kernel config.
"Fix" it by silencing the exc_double_fault() __noreturn warning.
This removes the following warning:
vmlinux.o: warning: objtool: xenpv_exc_double_fault+0xd: exc_double_fault() is missing a __noreturn annotation
Signed-off-by Josh Poimboeuf <[email protected]>
--- tools/objtool/check.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
tools/objtool/check.c +30 -1
@@ -4483,6 +4483,35 @@ static int validate_sls(struct objtool_file *file)return warnings; } +static bool ignore_noreturn_call (struct instruction *insn) +{ +struct symbol *call_dest =insn_call_dest (insn); + +/* +* This is a hack, but it works for now. +* +* Problem is, exc_double_fault() may or may not return, depending on +* whether CONFIG_X86_ESPFIX64 is set. But objtool has no visibility +* to the kernel config. +* +* Other potential ways to fix it: +* +* - remove CONFIG_X86_ESPFIX64 +* - read the .config file +* - add a cmdline option +* - create a generic objtool annotation format (vs a bunch of custom +* formats) and annotate it +* - have compiler communicate __noreturn functions somehow +*/ +if (!strcmp (call_dest->name,"exc_double_fault" )) { +/* prevent further unreachable warnings for the caller */ + insn->sym->warned =1 ; +return true; + } + +return false; +} +static int validate_reachable_instructions (struct objtool_file *file) {struct instruction *insn, *prev_insn;
@@ -4499,7 +4528,7 @@ static int validate_reachable_instructions(struct objtool_file *file) prev_insn =prev_insn_same_sec (file, insn);if (prev_insn && prev_insn->dead_end) { call_dest =insn_call_dest (prev_insn); -if (call_dest) { +if (call_dest && !ignore_noreturn_call (prev_insn)) {WARN_INSN (insn,"%s() is missing a __noreturn annotation" , call_dest->name); warnings++;
Signature · 1 line
2.39.2
Peter Zijlstra
maintainerRe: [PATCH v2 9/9] objtool: Ignore exc_double_fault() __noreturn warnings
tools/objtool/check.c static int validate_sls(struct objtool_file *file) · line 4504
Quoted 1 line · Josh Poimboeuf
tools/objtool/check.c +1
-0+* - have compiler communicate __noreturn functions somehow
This, we're going to have to do that -- because keeping that (ever growing) list in sync is going to be a pain in the backside.
Re: [PATCH v2 9/9] objtool: Ignore exc_double_fault() __noreturn warnings
Quoted 3 lines · Peter Zijlstra
Quoted 1 line · Josh Poimboeuf
+ * - have compiler communicate
__noreturnfunctions somehow
Could probably stuff these in some kind of "noreturn function symbol table"/custom elf section; DWARF tags are nice but we don't always want debug info. (I should ask colleagues if there's anything like this in DWARF already, perhaps for DWARFv6).
Quoted 4 lines
This, we're going to have to do that -- because keeping that (ever growing) list in sync is going to be a pain in the backside.
Signature · 2 lines
Thanks, ~Nick Desaulniers