objtool: a couple of minor fixes
cover objtool: a couple of minor fixes
Fix a couple of issues:
- fix a false positive warning related to switch statement jump tables
- get rid of useless "unreachable instruction" warnings for gcov kernels
Josh Poimboeuf (2):
objtool: improve rare switch jump table pattern detection
objtool: skip all "unreachable instruction" warnings for gcov kernels
tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 33 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/2 objtool: improve rare switch jump table pattern detection
gcc 6 added a new switch statement jump table optimization which makes objtool's life harder. It looks like:
mov [rodata addr],%reg1
... some instructions ...
jmpq *(%reg1,%reg2,8)
The optimization is quite rare, but objtool still needs to be able to identify the pattern so that it can follow all possible control flow paths related to the switch statement.
In order to detect the pattern, objtool starts from the indirect jump and scans backwards through the function until it finds the first instruction in the pattern. If it encounters an unconditional jump along the way, it stops and considers the pattern to be not found.
As it turns out, unconditional jumps can happen, as long as they are small forward jumps within the range being scanned.
This fixes the following warnings:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x2f4: sibling call from callable instruction with changed frame pointer drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0x10f: sibling call from callable instruction with changed frame pointer
--- tools/objtool/builtin-check.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
tools/objtool/builtin-check.c +9 -2
@@ -713,6 +713,7 @@ static struct rela *find_switch_table(struct objtool_file *file, struct instruction *insn) {struct rela *text_rela, *rodata_rela; +struct instruction *orig_insn = insn; text_rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len);if (text_rela && text_rela->sym == file->rodata->sym) {
@@ -733,10 +734,16 @@ static struct rela *find_switch_table(struct objtool_file *file,/* case 3 */ func_for_each_insn_continue_reverse (file, func, insn) { -if (insn->type ==INSN_JUMP_UNCONDITIONAL || - insn->type ==INSN_JUMP_DYNAMIC ) +if (insn->type ==INSN_JUMP_DYNAMIC )break ; +/* allow small jumps within the range */ +if (insn->type ==INSN_JUMP_UNCONDITIONAL && + insn->jump_dest && + (insn->jump_dest->offset <= insn->offset || + insn->jump_dest->offset >= orig_insn->offset)) +break ; + text_rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len);if (text_rela && text_rela->sym == file->rodata->sym)
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:core/urgent] objtool: Improve rare switch jump table pattern detection
objtool: Improve rare switch jump table pattern detection
Commit-ID: 3732710ff6f2ce2b1b7f044937a422b717d4f953 Gitweb: http://git.kernel.org/tip/3732710ff6f2ce2b1b7f044937a422b717d4f953 Author: Josh Poimboeuf <[email protected]> AuthorDate: Thu, 13 Oct 2016 16:22:52 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Sun, 16 Oct 2016 09:12:35 +0200
objtool: Improve rare switch jump table pattern detection
GCC 6 added a new switch statement jump table optimization which makes objtool's life harder. It looks like:
mov [rodata addr],%reg1
... some instructions ...
jmpq *(%reg1,%reg2,8)
The optimization is quite rare, but objtool still needs to be able to identify the pattern so that it can follow all possible control flow paths related to the switch statement.
In order to detect the pattern, objtool starts from the indirect jump and scans backwards through the function until it finds the first instruction in the pattern. If it encounters an unconditional jump along the way, it stops and considers the pattern to be not found.
As it turns out, unconditional jumps can happen, as long as they are small forward jumps within the range being scanned.
This fixes the following warnings:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x2f4: sibling call from callable instruction with changed frame pointer drivers/infiniband/sw/rxe/rxe_resp.o: warning: objtool: rxe_responder()+0x10f: sibling call from callable instruction with changed frame pointer
--- tools/objtool/builtin-check.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
tools/objtool/builtin-check.c +9 -2
@@ -713,6 +713,7 @@ static struct rela *find_switch_table(struct objtool_file *file, struct instruction *insn) {struct rela *text_rela, *rodata_rela; +struct instruction *orig_insn = insn; text_rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len);if (text_rela && text_rela->sym == file->rodata->sym) {
@@ -733,10 +734,16 @@ static struct rela *find_switch_table(struct objtool_file *file,/* case 3 */ func_for_each_insn_continue_reverse (file, func, insn) { -if (insn->type ==INSN_JUMP_UNCONDITIONAL || - insn->type ==INSN_JUMP_DYNAMIC ) +if (insn->type ==INSN_JUMP_DYNAMIC )break ; +/* allow small jumps within the range */ +if (insn->type ==INSN_JUMP_UNCONDITIONAL && + insn->jump_dest && + (insn->jump_dest->offset <= insn->offset || + insn->jump_dest->offset >= orig_insn->offset)) +break ; + text_rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len);if (text_rela && text_rela->sym == file->rodata->sym)
PATCH 2/2 objtool: skip all "unreachable instruction" warnings for gcov kernels
In reply to Josh PoimboeufRecently objtool has started reporting a few "unreachable instruction" warnings when CONFIG_GCOV is enabled for newer versions of gcc. Usually this warning means there's some new control flow that objtool doesn't understand. But in this case, objtool is correct and the instructions really are inaccessible. It's an annoying quirk of gcov, but it's harmless, so it's ok to just silence the warnings.
With older versions of gcc, it was relatively easy to detect gcov-specific instructions and to skip any unreachable warnings produced by them. But gcc 6 has gotten craftier.
Instead of continuing to play whack-a-mole with gcov, just use a bigger, more permanent hammer and disable unreachable warnings for the whole file when gcov is enabled. This is fine to do because a) unreachable warnings are usually of questionable value; and b) gcov isn't used for production kernels and we can relax the checks a bit there.
--- tools/objtool/builtin-check.c | 57 ++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 31 deletions(-)
tools/objtool/builtin-check.c +26 -31
@@ -97,6 +97,19 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file,return next; } +static bool gcov_enabled (struct objtool_file *file) +{ +struct section *sec; +struct symbol *sym; + +list_for_each_entry (sec, &file->elf->sections, list) +list_for_each_entry (sym, &sec->symbol_list, list) +if (!strncmp (sym->name,"__gcov_." ,8 )) +return true; + +return false; +} +#define for_each_insn (file, insn) \ list_for_each_entry(insn, &file->insn_list, list)
@@ -1041,34 +1054,6 @@ static int validate_branch(struct objtool_file *file,return 0 ; } -static bool is_gcov_insn (struct instruction *insn) -{ -struct rela *rela; -struct section *sec; -struct symbol *sym; -unsigned long offset; - - rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len); -if (!rela) -return false; - -if (rela->sym->type !=STT_SECTION ) -return false; - - sec = rela->sym->sec; - offset = rela->addend + insn->offset + insn->len - rela->offset; - -list_for_each_entry (sym, &sec->symbol_list, list) { -if (sym->type !=STT_OBJECT ) -continue ; - -if (offset >= sym->offset && offset < sym->offset + sym->len) -return (!memcmp (sym->name,"__gcov0." ,8 )); - } - -return false; -} -static bool is_kasan_insn (struct instruction *insn) { return (insn->type == INSN_CALL &&
@@ -1090,9 +1075,6 @@ static bool ignore_unreachable_insn(struct symbol *func,if (insn->type ==INSN_NOP )return true; -if (is_gcov_insn (insn)) -return true; -/* * Check if this (or a subsequent) instruction is related to * CONFIG_UBSAN or CONFIG_KASAN.
@@ -1153,6 +1135,19 @@ static int validate_functions(struct objtool_file *file) ignore_unreachable_insn(func, insn))continue ; +/* +* gcov produces a lot of unreachable +* instructions. If we get an unreachable +* warning and the file has gcov enabled, just +* ignore it, and all other such warnings for +* the file. +*/ +if (!file->ignore_unreachables && +gcov_enabled (file)) { + file->ignore_unreachables = true; +continue ; + } +WARN_FUNC ("function has unreachable instruction" , insn->sec, insn->offset); warnings++; }
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:core/urgent] objtool: Skip all "unreachable instruction" warnings for gcov kernels
objtool: Skip all "unreachable instruction" warnings for gcov kernels
Commit-ID: 9cfffb116887b1b7c51cd4e3fa5790dc52a0758f Gitweb: http://git.kernel.org/tip/9cfffb116887b1b7c51cd4e3fa5790dc52a0758f Author: Josh Poimboeuf <[email protected]> AuthorDate: Thu, 13 Oct 2016 16:22:53 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Sun, 16 Oct 2016 09:12:36 +0200
objtool: Skip all "unreachable instruction" warnings for gcov kernels
Recently objtool has started reporting a few "unreachable instruction" warnings when CONFIG_GCOV is enabled for newer versions of GCC. Usually this warning means there's some new control flow that objtool doesn't understand. But in this case, objtool is correct and the instructions really are inaccessible. It's an annoying quirk of gcov, but it's harmless, so it's ok to just silence the warnings.
With older versions of GCC, it was relatively easy to detect gcov-specific instructions and to skip any unreachable warnings produced by them. But GCC 6 has gotten craftier.
Instead of continuing to play whack-a-mole with gcov, just use a bigger, more permanent hammer and disable unreachable warnings for the whole file when gcov is enabled. This is fine to do because a) unreachable warnings are usually of questionable value; and b) gcov isn't used for production kernels and we can relax the checks a bit there.
--- tools/objtool/builtin-check.c | 57 ++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 31 deletions(-)
tools/objtool/builtin-check.c +26 -31
@@ -97,6 +97,19 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file,return next; } +static bool gcov_enabled (struct objtool_file *file) +{ +struct section *sec; +struct symbol *sym; + +list_for_each_entry (sec, &file->elf->sections, list) +list_for_each_entry (sym, &sec->symbol_list, list) +if (!strncmp (sym->name,"__gcov_." ,8 )) +return true; + +return false; +} +#define for_each_insn (file, insn) \ list_for_each_entry(insn, &file->insn_list, list)
@@ -1041,34 +1054,6 @@ static int validate_branch(struct objtool_file *file,return 0 ; } -static bool is_gcov_insn (struct instruction *insn) -{ -struct rela *rela; -struct section *sec; -struct symbol *sym; -unsigned long offset; - - rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len); -if (!rela) -return false; - -if (rela->sym->type !=STT_SECTION ) -return false; - - sec = rela->sym->sec; - offset = rela->addend + insn->offset + insn->len - rela->offset; - -list_for_each_entry (sym, &sec->symbol_list, list) { -if (sym->type !=STT_OBJECT ) -continue ; - -if (offset >= sym->offset && offset < sym->offset + sym->len) -return (!memcmp (sym->name,"__gcov0." ,8 )); - } - -return false; -} -static bool is_kasan_insn (struct instruction *insn) { return (insn->type == INSN_CALL &&
@@ -1090,9 +1075,6 @@ static bool ignore_unreachable_insn(struct symbol *func,if (insn->type ==INSN_NOP )return true; -if (is_gcov_insn (insn)) -return true; -/* * Check if this (or a subsequent) instruction is related to * CONFIG_UBSAN or CONFIG_KASAN.
@@ -1153,6 +1135,19 @@ static int validate_functions(struct objtool_file *file) ignore_unreachable_insn(func, insn))continue ; +/* +* gcov produces a lot of unreachable +* instructions. If we get an unreachable +* warning and the file has gcov enabled, just +* ignore it, and all other such warnings for +* the file. +*/ +if (!file->ignore_unreachables && +gcov_enabled (file)) { + file->ignore_unreachables = true; +continue ; + } +WARN_FUNC ("function has unreachable instruction" , insn->sec, insn->offset); warnings++; }
Quoted 11 lines · CEST Josh Poimboeuf
Fix a couple of issues:
- fix a false positive warning related to switch statement jump tables
- get rid of useless "unreachable instruction" warnings for gcov kernelsJosh Poimboeuf (2):
objtool: improve rare switch jump table pattern detection
objtool: skip all "unreachable instruction" warnings for gcov kernelstools/objtool/builtin-check.c | 68 ++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 33 deletions(-)
I got another warning today with linux-next, but have not looked into it:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
.config and object file attached.
Arnd
Quoted 10 lines · CEST Arnd Bergmann
Quoted 9 lines · CEST Josh Poimboeuf
Fix a couple of issues:
- fix a false positive warning related to switch statement jump tables
- get rid of useless "unreachable instruction" warnings for gcov kernelsJosh Poimboeuf (2):
objtool: improve rare switch jump table pattern detection
objtool: skip all "unreachable instruction" warnings for gcov kernels
Quoted 8 lines
Quoted 2 lines
tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 33 deletions(-)I got another warning today with linux-next, but have not looked into it:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer.config and object file attached.
And another one, let me know if you want detailed information about this one too, I suspect they are related.
drivers/media/usb/ttusb-dec/ttusb_dec.o: warning: objtool: ttusb_dec_process_urb_frame_list()+0x112: sibling call from callable instruction with changed frame pointer
Arnd
Quoted 10 lines · Arnd Bergmann
Quoted 9 lines · CEST Josh Poimboeuf
Fix a couple of issues:
- fix a false positive warning related to switch statement jump tables
- get rid of useless "unreachable instruction" warnings for gcov kernelsJosh Poimboeuf (2):
objtool: improve rare switch jump table pattern detection
objtool: skip all "unreachable instruction" warnings for gcov kernels
Quoted 8 lines
Quoted 2 lines
tools/objtool/builtin-check.c | 68 ++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 33 deletions(-)I got another warning today with linux-next, but have not looked into it:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer.config and object file attached.
I think this is the same as one of the warnings you reported before (for which I opened the gcc bug):
https://lkml.kernel.org/r/1855683.1QnG4Fe4Hq@wuerfel
Signature · 1 line
Josh
Quoted 16 lines · Josh Poimboeuf
Quoted 15 lines · Arnd Bergmann
Quoted 11 lines · CEST Josh Poimboeuf
Fix a couple of issues:
- fix a false positive warning related to switch statement jump tables
- get rid of useless "unreachable instruction" warnings for gcov kernelsJosh Poimboeuf (2):
objtool: improve rare switch jump table pattern detection
objtool: skip all "unreachable instruction" warnings for gcov kernelstools/objtool/builtin-check.c | 68 ++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 33 deletions(-)I got another warning today with linux-next, but have not looked into it:
Quoted 8 lines
Quoted 3 lines
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer.config and object file attached.
I think this is the same as one of the warnings you reported before (for which I opened the gcc bug):
Er, scratch that. Actually this is similar to one I supposedly fixed with commit 3732710ff6f2ce2b1b7f044937a422b717d4f953. And the other warning looks similar. Will take a look.
Signature · 1 line
Josh
Quoted 21 lines · Josh Poimboeuf
Quoted 20 lines · Josh Poimboeuf
Quoted 18 lines · Arnd Bergmann
Quoted 11 lines · CEST Josh Poimboeuf
Fix a couple of issues:
- fix a false positive warning related to switch statement jump tables
- get rid of useless "unreachable instruction" warnings for gcov kernelsJosh Poimboeuf (2):
objtool: improve rare switch jump table pattern detection
objtool: skip all "unreachable instruction" warnings for gcov kernelstools/objtool/builtin-check.c | 68 ++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 33 deletions(-)I got another warning today with linux-next, but have not looked into it:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer.config and object file attached.
Quoted 8 lines
Quoted 4 lines
I think this is the same as one of the warnings you reported before (for which I opened the gcc bug):
Er, scratch that. Actually this is similar to one I supposedly fixed with commit
3732710ff6f2ce2b1b7f044937a422b717d4f953. And the other warning looks similar. Will take a look.
As it turns out, this one is a simple fix to the above mentioned commit.
But as for the other warning... the code generation is just crazy. It's another issue with switch statement jump tables, which as it turns out are really hard to detect.
I may ask the gcc folks for help. If they could mark the jump tables somehow, like maybe associating them with ELF symbols, it would make objtool's life (and mine!) so much easier.
Signature · 1 line
Josh
PATCH 1/1 objtool: fix rare switch jump table pattern detection
In reply to Arnd BergmannThe following commit:
3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")
... improved objtool's ability to detect gcc switch statement jump tables for gcc 6. However the check to allow short jumps with the scanned range of instructions wasn't quite right. The pattern detection should allow jumps to the indirect jump instruction itself.
This fixes the following warning:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
--- tools/objtool/builtin-check.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/objtool/builtin-check.c +1 -1
@@ -754,7 +754,7 @@ static struct rela *find_switch_table(struct objtool_file *file,if (insn->type ==INSN_JUMP_UNCONDITIONAL && insn->jump_dest && (insn->jump_dest->offset <= insn->offset || - insn->jump_dest->offset >= orig_insn->offset)) + insn->jump_dest->offset > orig_insn->offset))break ; text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
Signature · 1 line
2.7.4
Re: [PATCH] objtool: fix rare switch jump table pattern detection
Quoted 9 lines · CEST Josh Poimboeuf
The following commit:
3732710ff6f2("objtool: Improve rare switch jump table pattern detection")... improved objtool's ability to detect gcc switch statement jump tables for gcc 6. However the check to allow short jumps with the scanned range of instructions wasn't quite right. The pattern detection should allow jumps to the indirect jump instruction itself.
Quoted 7 lines
This fixes the following warning:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointerReported-by Arnd Bergmann <[email protected]>Fixes 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")Signed-off-by Josh Poimboeuf <[email protected]>
It fixes the mlx4/resource_tracker.o problem, but not the other one for ttusb-dec/ttusb_dec.o, as you mentioned.
Do you need any more help creating a testcase for that one?
Arnd
Re: [PATCH] objtool: fix rare switch jump table pattern detection
Quoted 18 lines · Arnd Bergmann
Quoted 9 lines · CEST Josh Poimboeuf
The following commit:
3732710ff6f2("objtool: Improve rare switch jump table pattern detection")... improved objtool's ability to detect gcc switch statement jump tables for gcc 6. However the check to allow short jumps with the scanned range of instructions wasn't quite right. The pattern detection should allow jumps to the indirect jump instruction itself.
Quoted 7 lines
This fixes the following warning:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointerReported-by Arnd Bergmann <[email protected]>Fixes 3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")Signed-off-by Josh Poimboeuf <[email protected]>
Quoted 6 lines
Tested-by Arnd Bergmann <[email protected]>It fixes the
mlx4/resource_tracker.oproblem, but not the other one forttusb-dec/ttusb_dec.o, as you mentioned.Do you need any more help creating a testcase for that one?
No, I can recreate the ttusb_dec.o warning with the config you gave me for this one. Thanks!
Signature · 1 line
Josh
tip-bot for Josh Poimboeuf
merge bot[tip:core/urgent] objtool: Fix rare switch jump table pattern detection
In reply to Josh Poimboeufobjtool: Fix rare switch jump table pattern detection
Commit-ID: 56fb2d6eb63acd48b50437b415b6f7d2fcffe75d Gitweb: http://git.kernel.org/tip/56fb2d6eb63acd48b50437b415b6f7d2fcffe75d Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 26 Oct 2016 10:34:08 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 27 Oct 2016 08:20:27 +0200
objtool: Fix rare switch jump table pattern detection
The following commit:
3732710ff6f2 ("objtool: Improve rare switch jump table pattern detection")
... improved objtool's ability to detect GCC switch statement jump tables for GCC 6. However the check to allow short jumps with the scanned range of instructions wasn't quite right. The pattern detection should allow jumps to the indirect jump instruction itself.
This fixes the following warning:
drivers/infiniband/sw/rxe/rxe_comp.o: warning: objtool: rxe_completer()+0x315: sibling call from callable instruction with changed frame pointer
--- tools/objtool/builtin-check.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/objtool/builtin-check.c +1 -1
@@ -754,7 +754,7 @@ static struct rela *find_switch_table(struct objtool_file *file,if (insn->type ==INSN_JUMP_UNCONDITIONAL && insn->jump_dest && (insn->jump_dest->offset <= insn->offset || - insn->jump_dest->offset >= orig_insn->offset)) + insn->jump_dest->offset > orig_insn->offset))break ; text_rela = find_rela_by_dest_range(insn->sec, insn->offset,