objtool: warning improvements
Josh Poimboeuf
maintainerv3 cover objtool: warning improvements
v3:
- dropped merged patches and rebased on tip/objtool/core
- moved list of noreturn functions to separate file
- improved WARN_INSN() to do single evaluation of insn arg
Josh Poimboeuf (8):
objtool: Limit unreachable warnings to once per function
objtool: Add verbose option for disassembling affected functions
objtool: Include backtrace in verbose mode
objtool: Detect missing __noreturn annotations
objtool: Ignore exc_double_fault() __noreturn warnings
objtool: Remove superfluous global_noreturns entries tools/lib/subcmd: Replace NORETURN usage with __noreturn
objtool: Move noreturn function list to separate file
tools/lib/subcmd/parse-options.h | 8 +- tools/lib/subcmd/subcmd-util.h | 5 +- tools/objtool/Documentation/objtool.txt | 10 ++ tools/objtool/builtin-check.c | 5 + tools/objtool/check.c | 195 ++++++++++++++++-------- tools/objtool/include/objtool/builtin.h | 1 + tools/objtool/include/objtool/elf.h | 1 + tools/objtool/include/objtool/warn.h | 21 ++- tools/objtool/noreturns.h | 45 ++++++ 9 files changed, 214 insertions(+), 77 deletions(-) create mode 100644 tools/objtool/noreturns.h
Signature · 1 line
2.39.2
Josh Poimboeuf
maintainerPATCH v3 1/8 objtool: Limit unreachable warnings to once per function
Unreachable 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.
--- tools/objtool/check.c | 5 +++-- tools/objtool/include/objtool/elf.h | 1 + tools/objtool/include/objtool/warn.h | 7 ++++++- 3 files changed, 10 insertions(+), 3 deletions(-)
tools/objtool/check.c +3 -2
@@ -4514,6 +4514,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 ;
@@ -4523,10 +4524,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 +6 -1
@@ -55,7 +55,12 @@ static inline char *offstr(struct section *sec, unsigned long offset)#define WARN_INSN (insn, format, ...) \ ({ \ - WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__); \ + 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; \ })#define BT_FUNC (format, insn, ...) \
Signature · 1 line
2.39.2
[tip: objtool/core] objtool: Limit unreachable warnings to once per function
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: 5e3992fe72748ed3892be876f09d4d990548b7af Gitweb: https://git.kernel.org/tip/5e3992fe72748ed3892be876f09d4d990548b7af Author: Josh Poimboeuf <[email protected]> AuthorDate: Tue, 18 Apr 2023 14:27:47 -07:00 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Tue, 16 May 2023 06:31:51 -07:00
objtool: Limit unreachable warnings to once per function
Unreachable 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.
--- tools/objtool/check.c | 5 +++-- tools/objtool/include/objtool/elf.h | 1 + tools/objtool/include/objtool/warn.h | 7 ++++++- 3 files changed, 10 insertions(+), 3 deletions(-)
tools/objtool/check.c +3 -2
@@ -4514,6 +4514,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 ;
@@ -4523,10 +4524,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 +6 -1
@@ -55,7 +55,12 @@ static inline char *offstr(struct section *sec, unsigned long offset)#define WARN_INSN (insn, format, ...) \ ({ \ - WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__); \ + 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; \ })#define BT_FUNC (format, insn, ...) \
Josh Poimboeuf
maintainerPATCH v3 2/8 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.
--- 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
[tip: objtool/core] objtool: Add verbose option for disassembling affected 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: ca653464dd097fe64e69f1735e9f348b2a0f8037 Gitweb: https://git.kernel.org/tip/ca653464dd097fe64e69f1735e9f348b2a0f8037 Author: Josh Poimboeuf <[email protected]> AuthorDate: Tue, 18 Apr 2023 14:27:48 -07:00 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Tue, 16 May 2023 06:31:51 -07:00
objtool: Add verbose option for disassembling affected functions
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.
--- 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;
Josh Poimboeuf
maintainerPATCH v3 3/8 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.
--- 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
@@ -3657,8 +3657,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; } }
@@ -3703,8 +3702,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; } }
@@ -3802,8 +3800,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; }
@@ -3861,8 +3859,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; } }
@@ -3888,10 +3885,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; }
@@ -3913,8 +3908,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; }/*
@@ -4216,8 +4210,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
@@ -63,12 +63,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
[tip: objtool/core] objtool: Include backtrace in verbose mode
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: ced23d2e3762ecfb859ae65d3a351218edff7205 Gitweb: https://git.kernel.org/tip/ced23d2e3762ecfb859ae65d3a351218edff7205 Author: Josh Poimboeuf <[email protected]> AuthorDate: Tue, 18 Apr 2023 14:27:49 -07:00 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Tue, 16 May 2023 06:31:52 -07:00
objtool: Include backtrace in verbose mode
Include backtrace in verbose mode. This makes it easy to gather all the information needed for diagnosing objtool warnings.
--- 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
@@ -3657,8 +3657,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; } }
@@ -3703,8 +3702,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; } }
@@ -3802,8 +3800,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; }
@@ -3861,8 +3859,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; } }
@@ -3888,10 +3885,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; }
@@ -3913,8 +3908,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; }/*
@@ -4216,8 +4210,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
@@ -63,12 +63,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, ...) \
Josh Poimboeuf
maintainerPATCH v3 4/8 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.
--- 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
@@ -4507,7 +4507,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)
@@ -4517,6 +4518,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
[tip: objtool/core] objtool: Detect missing __noreturn annotations
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: fedb724c3db5490234ddde0103811c28c2fedae0 Gitweb: https://git.kernel.org/tip/fedb724c3db5490234ddde0103811c28c2fedae0 Author: Josh Poimboeuf <[email protected]> AuthorDate: Tue, 18 Apr 2023 14:27:50 -07:00 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Tue, 16 May 2023 06:31:53 -07:00
objtool: Detect missing __noreturn annotations
Most "unreachable instruction" warnings these days seem to actually be the result of a missing __noreturn annotation. Add an explicit check for that.
--- 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
@@ -4507,7 +4507,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)
@@ -4517,6 +4518,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++; }
Josh Poimboeuf
maintainerPATCH v3 5/8 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
--- tools/objtool/check.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
tools/objtool/check.c +30 -1
@@ -4505,6 +4505,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); + +/* +* FIXME: hack, we need a real noreturn solution +* +* 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: +* +* - have compiler communicate __noreturn functions somehow +* - 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 +*/ +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;
@@ -4521,7 +4550,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
[tip: objtool/core] objtool: Ignore exc_double_fault() __noreturn warnings
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: 55eeab2a8a11b71586ef0ad3adf532ca5f97d4be Gitweb: https://git.kernel.org/tip/55eeab2a8a11b71586ef0ad3adf532ca5f97d4be Author: Josh Poimboeuf <[email protected]> AuthorDate: Tue, 18 Apr 2023 14:27:51 -07:00 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Tue, 16 May 2023 06:31:53 -07:00
objtool: Ignore exc_double_fault() __noreturn warnings
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.
"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
--- tools/objtool/check.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
tools/objtool/check.c +30 -1
@@ -4505,6 +4505,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); + +/* +* FIXME: hack, we need a real noreturn solution +* +* 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: +* +* - have compiler communicate __noreturn functions somehow +* - 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 +*/ +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;
@@ -4521,7 +4550,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++;
Josh Poimboeuf
maintainerPATCH v3 6/8 objtool: Remove superfluous global_noreturns entries
In reply to Josh Poimboeuflbug_with_loc() no longer exists, and resume_play_dead() is static (objtool only checks globals and weaks).
--- tools/objtool/check.c | 2 -- 1 file changed, 2 deletions(-)
tools/objtool/check.c +0 -2
@@ -217,7 +217,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,"kthread_complete_and_exit" ,"kthread_exit" ,"kunit_try_catch_throw" , -"lbug_with_loc" ,"machine_real_restart" ,"make_task_dead" , "mpt_halt_firmware",
@@ -225,7 +224,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,"panic" ,"panic_smp_self_stop" ,"rest_init" , -"resume_play_dead" ,"rewind_stack_and_make_dead" ,"sev_es_terminate" , "snp_abort",
Signature · 1 line
2.39.2
[tip: objtool/core] objtool: Remove superfluous global_noreturns entries
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: 34245659debd194cbd4148d2ee5176306bdf8899 Gitweb: https://git.kernel.org/tip/34245659debd194cbd4148d2ee5176306bdf8899 Author: Josh Poimboeuf <[email protected]> AuthorDate: Tue, 18 Apr 2023 14:27:52 -07:00 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Tue, 16 May 2023 06:31:54 -07:00
objtool: Remove superfluous global_noreturns entries
lbug_with_loc() no longer exists, and resume_play_dead() is static (objtool only checks globals and weaks).
--- tools/objtool/check.c | 2 -- 1 file changed, 2 deletions(-)
tools/objtool/check.c +0 -2
@@ -217,7 +217,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,"kthread_complete_and_exit" ,"kthread_exit" ,"kunit_try_catch_throw" , -"lbug_with_loc" ,"machine_real_restart" ,"make_task_dead" , "mpt_halt_firmware",
@@ -225,7 +224,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,"panic" ,"panic_smp_self_stop" ,"rest_init" , -"resume_play_dead" ,"rewind_stack_and_make_dead" ,"sev_es_terminate" , "snp_abort",
Josh Poimboeuf
maintainerPATCH v3 7/8 tools/lib/subcmd: Replace NORETURN usage with __noreturn
In reply to Josh PoimboeufNORETURN is redundant with __noreturn, just use the latter.
--- tools/lib/subcmd/parse-options.h | 8 ++------ tools/lib/subcmd/subcmd-util.h | 5 ++--- 2 files changed, 4 insertions(+), 9 deletions(-)
tools/lib/subcmd/parse-options.h +2 -6
@@ -6,10 +6,6 @@#include <stdbool.h> #include <stdint.h> -#ifndef NORETURN -#define NORETURN __attribute__((__noreturn__)) -#endif -enum parse_opt_type {/* special types */ OPTION_END ,
@@ -183,9 +179,9 @@ extern int parse_options_subcommand(int argc, const char **argv,const char *const subcommands[], const char *usagestr[], int flags); -extern NORETURN void usage_with_options(const char * const *usagestr, +extern __noreturn void usage_with_options (const char *const *usagestr,const struct option *options); -extern NORETURN __attribute__((format (printf,3 ,4 ))) +extern __noreturn __attribute__((format (printf,3 ,4 )))void usage_with_options_msg (const char *const *usagestr,const struct option *options,const char *fmt, ...);
tools/lib/subcmd/subcmd-util.h +2 -3
@@ -5,8 +5,7 @@#include <stdarg.h> #include <stdlib.h> #include <stdio.h> - -#define NORETURN __attribute__((__noreturn__)) +#include <linux/compiler.h> static inline void report (const char *prefix,const char *err,va_list params) {
@@ -15,7 +14,7 @@ static inline void report(const char *prefix, const char *err, va_list params)fprintf (stderr," %s%s\n" , prefix, msg); } -static NORETURN inline void die(const char *err, ...) +static __noreturn inline void die (const char *err, ...) {va_list params;
Signature · 1 line
2.39.2
[tip: objtool/core] tools/lib/subcmd: Replace NORETURN usage with __noreturn
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: d59fec29b131f30b27343d54bdf1071ee98eda8e Gitweb: https://git.kernel.org/tip/d59fec29b131f30b27343d54bdf1071ee98eda8e Author: Josh Poimboeuf <[email protected]> AuthorDate: Tue, 18 Apr 2023 14:27:53 -07:00 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Tue, 16 May 2023 06:31:54 -07:00
tools/lib/subcmd: Replace NORETURN usage with __noreturn
NORETURN is redundant with __noreturn, just use the latter.
--- tools/lib/subcmd/parse-options.h | 8 ++------ tools/lib/subcmd/subcmd-util.h | 5 ++--- 2 files changed, 4 insertions(+), 9 deletions(-)
tools/lib/subcmd/parse-options.h +2 -6
@@ -6,10 +6,6 @@#include <stdbool.h> #include <stdint.h> -#ifndef NORETURN -#define NORETURN __attribute__((__noreturn__)) -#endif -enum parse_opt_type {/* special types */ OPTION_END ,
@@ -183,9 +179,9 @@ extern int parse_options_subcommand(int argc, const char **argv,const char *const subcommands[], const char *usagestr[], int flags); -extern NORETURN void usage_with_options(const char * const *usagestr, +extern __noreturn void usage_with_options (const char *const *usagestr,const struct option *options); -extern NORETURN __attribute__((format (printf,3 ,4 ))) +extern __noreturn __attribute__((format (printf,3 ,4 )))void usage_with_options_msg (const char *const *usagestr,const struct option *options,const char *fmt, ...);
tools/lib/subcmd/subcmd-util.h +2 -3
@@ -5,8 +5,7 @@#include <stdarg.h> #include <stdlib.h> #include <stdio.h> - -#define NORETURN __attribute__((__noreturn__)) +#include <linux/compiler.h> static inline void report (const char *prefix,const char *err,va_list params) {
@@ -15,7 +14,7 @@ static inline void report(const char *prefix, const char *err, va_list params)fprintf (stderr," %s%s\n" , prefix, msg); } -static NORETURN inline void die(const char *err, ...) +static __noreturn inline void die (const char *err, ...) {va_list params;
Josh Poimboeuf
maintainerPATCH v3 8/8 objtool: Move noreturn function list to separate file
In reply to Josh PoimboeufThis makes it a little cleaner and easier to maintain.
--- tools/objtool/Documentation/objtool.txt | 5 ++- tools/objtool/check.c | 44 ++---------------------- tools/objtool/noreturns.h | 45 +++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 44 deletions(-) create mode 100644 tools/objtool/noreturns.h
tools/objtool/Documentation/objtool.txt +2 -3
@@ -306,9 +306,8 @@ the objtool maintainers. 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. + __noreturn annotation. NOTE: In addition to annotating the function + with __noreturn, please also add it to tools/objtool/noreturns.h. 4. file.o: warning: objtool: func(): can't find starting instruction or
tools/objtool/check.c +3 -41
52 lines not on this page · raw message
tools/objtool/noreturns.h new file +45 -0
45 lines not on this page · raw message
Signature · 1 line
2.39.2
[tip: objtool/core] objtool: Move noreturn function list to separate file
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: 6245ce4ab670166efcdae843c35c14e4c0811aa3 Gitweb: https://git.kernel.org/tip/6245ce4ab670166efcdae843c35c14e4c0811aa3 Author: Josh Poimboeuf <[email protected]> AuthorDate: Tue, 18 Apr 2023 14:27:54 -07:00 Committer: Josh Poimboeuf <[email protected]> CommitterDate: Tue, 16 May 2023 06:31:55 -07:00
objtool: Move noreturn function list to separate file
This makes it a little cleaner and easier to maintain.
--- tools/objtool/Documentation/objtool.txt | 5 +-- tools/objtool/check.c | 44 +---------------------- tools/objtool/noreturns.h | 45 ++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 44 deletions(-) create mode 100644 tools/objtool/noreturns.h
tools/objtool/Documentation/objtool.txt +2 -3
@@ -306,9 +306,8 @@ the objtool maintainers. 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. + __noreturn annotation. NOTE: In addition to annotating the function + with __noreturn, please also add it to tools/objtool/noreturns.h. 4. file.o: warning: objtool: func(): can't find starting instruction or
tools/objtool/check.c +3 -41
52 lines not on this page · raw message
tools/objtool/noreturns.h new file +45 -0
45 lines not on this page · raw message
Quoted 15 lines · Josh Poimboeuf
v3:
- dropped merged patches and rebased on tip/objtool/core
- moved list of noreturn functions to separate file
- improvedWARN_INSN()to do single evaluation of insn argJosh Poimboeuf (8):
objtool: Limit unreachable warnings to once per function
objtool: Add verbose option for disassembling affected functions
objtool: Include backtrace in verbose mode
objtool: Detect missing__noreturnannotations
objtool: Ignoreexc_double_fault()__noreturnwarnings
objtool: Remove superfluousglobal_noreturnsentriestools/lib/subcmd: Replace NORETURN usage with__noreturn
objtool: Move noreturn function list to separate file
Quoted 11 lines
tools/lib/subcmd/parse-options.h | 8 +- tools/lib/subcmd/subcmd-util.h | 5 +- tools/objtool/Documentation/objtool.txt | 10 ++ tools/objtool/builtin-check.c | 5 + tools/objtool/check.c | 195 ++++++++++++++++-------- tools/objtool/include/objtool/builtin.h | 1 + tools/objtool/include/objtool/elf.h | 1 + tools/objtool/include/objtool/warn.h | 21 ++- tools/objtool/noreturns.h | 45 ++++++ 9 files changed, 214 insertions(+), 77 deletions(-) create mode 100644 tools/objtool/noreturns.h
M