* [PATCH 0/2] objtool: Fix function fallthrough detection @ 2019-05-13 17:01 Josh Poimboeuf 2019-05-13 17:01 ` [PATCH 1/2] objtool: Don't use ignore flag for fake jumps Josh Poimboeuf 2019-05-13 17:01 ` [PATCH 2/2] objtool: Fix function fallthrough detection Josh Poimboeuf 0 siblings, 2 replies; 5+ messages in thread From: Josh Poimboeuf @ 2019-05-13 17:01 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Peter Zijlstra Patch 1 is a minor objtool cleanup which is a prereq for patch 2. Patch 2 fixes objtool function fallthrough detection. Josh Poimboeuf (2): objtool: Don't use 'ignore' flag for fake jumps objtool: Fix function fallthrough detection tools/objtool/check.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) -- 2.17.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] objtool: Don't use ignore flag for fake jumps 2019-05-13 17:01 [PATCH 0/2] objtool: Fix function fallthrough detection Josh Poimboeuf @ 2019-05-13 17:01 ` Josh Poimboeuf 2019-05-13 18:34 ` [tip:core/urgent] " tip-bot for Josh Poimboeuf 2019-05-13 17:01 ` [PATCH 2/2] objtool: Fix function fallthrough detection Josh Poimboeuf 1 sibling, 1 reply; 5+ messages in thread From: Josh Poimboeuf @ 2019-05-13 17:01 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Peter Zijlstra The ignore flag is set on fake jumps in order to keep add_jump_destinations() from setting their jump_dest, since it already got set when the fake jump was created. But using the ignore flag is a bit of a hack. It's normally used to skip validation of an instruction, which doesn't really make sense for fake jumps. Also, after the next patch, using the ignore flag for fake jumps can trigger a false "why am I validating an ignored function?" warning. Instead just add an explicit check in add_jump_destinations() to skip fake jumps. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- tools/objtool/check.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index ac743a1d53ab..90226791df6b 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -28,6 +28,8 @@ #include <linux/hashtable.h> #include <linux/kernel.h> +#define FAKE_JUMP_OFFSET -1 + struct alternative { struct list_head list; struct instruction *insn; @@ -568,7 +570,7 @@ static int add_jump_destinations(struct objtool_file *file) insn->type != INSN_JUMP_UNCONDITIONAL) continue; - if (insn->ignore) + if (insn->ignore || insn->offset == FAKE_JUMP_OFFSET) continue; rela = find_rela_by_dest_range(insn->sec, insn->offset, @@ -745,10 +747,10 @@ static int handle_group_alt(struct objtool_file *file, clear_insn_state(&fake_jump->state); fake_jump->sec = special_alt->new_sec; - fake_jump->offset = -1; + fake_jump->offset = FAKE_JUMP_OFFSET; fake_jump->type = INSN_JUMP_UNCONDITIONAL; fake_jump->jump_dest = list_next_entry(last_orig_insn, list); - fake_jump->ignore = true; + fake_jump->func = orig_insn->func; } if (!special_alt->new_len) { -- 2.17.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:core/urgent] objtool: Don't use ignore flag for fake jumps 2019-05-13 17:01 ` [PATCH 1/2] objtool: Don't use ignore flag for fake jumps Josh Poimboeuf @ 2019-05-13 18:34 ` tip-bot for Josh Poimboeuf 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Josh Poimboeuf @ 2019-05-13 18:34 UTC (permalink / raw) To: linux-tip-commits Cc: jpoimboe, mingo, peterz, tglx, linux-kernel, torvalds, hpa Commit-ID: e6da9567959e164f82bc81967e0d5b10dee870b4 Gitweb: https://git.kernel.org/tip/e6da9567959e164f82bc81967e0d5b10dee870b4 Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Mon, 13 May 2019 12:01:31 -0500 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Mon, 13 May 2019 20:31:17 +0200 objtool: Don't use ignore flag for fake jumps The ignore flag is set on fake jumps in order to keep add_jump_destinations() from setting their jump_dest, since it already got set when the fake jump was created. But using the ignore flag is a bit of a hack. It's normally used to skip validation of an instruction, which doesn't really make sense for fake jumps. Also, after the next patch, using the ignore flag for fake jumps can trigger a false "why am I validating an ignored function?" warning. Instead just add an explicit check in add_jump_destinations() to skip fake jumps. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/71abc072ff48b2feccc197723a9c52859476c068.1557766718.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- tools/objtool/check.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index ac743a1d53ab..90226791df6b 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -28,6 +28,8 @@ #include <linux/hashtable.h> #include <linux/kernel.h> +#define FAKE_JUMP_OFFSET -1 + struct alternative { struct list_head list; struct instruction *insn; @@ -568,7 +570,7 @@ static int add_jump_destinations(struct objtool_file *file) insn->type != INSN_JUMP_UNCONDITIONAL) continue; - if (insn->ignore) + if (insn->ignore || insn->offset == FAKE_JUMP_OFFSET) continue; rela = find_rela_by_dest_range(insn->sec, insn->offset, @@ -745,10 +747,10 @@ static int handle_group_alt(struct objtool_file *file, clear_insn_state(&fake_jump->state); fake_jump->sec = special_alt->new_sec; - fake_jump->offset = -1; + fake_jump->offset = FAKE_JUMP_OFFSET; fake_jump->type = INSN_JUMP_UNCONDITIONAL; fake_jump->jump_dest = list_next_entry(last_orig_insn, list); - fake_jump->ignore = true; + fake_jump->func = orig_insn->func; } if (!special_alt->new_len) { ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] objtool: Fix function fallthrough detection 2019-05-13 17:01 [PATCH 0/2] objtool: Fix function fallthrough detection Josh Poimboeuf 2019-05-13 17:01 ` [PATCH 1/2] objtool: Don't use ignore flag for fake jumps Josh Poimboeuf @ 2019-05-13 17:01 ` Josh Poimboeuf 2019-05-13 18:34 ` [tip:core/urgent] " tip-bot for Josh Poimboeuf 1 sibling, 1 reply; 5+ messages in thread From: Josh Poimboeuf @ 2019-05-13 17:01 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Peter Zijlstra When a function falls through to the next function due to a compiler bug, objtool prints some obscure warnings. For example: drivers/regulator/core.o: warning: objtool: regulator_count_voltages()+0x95: return with modified stack frame drivers/regulator/core.o: warning: objtool: regulator_count_voltages()+0x0: stack state mismatch: cfa1=7+32 cfa2=7+8 Instead it should be printing: drivers/regulator/core.o: warning: objtool: regulator_supply_is_couple() falls through to next function regulator_count_voltages() This used to work, but was broken by the following commit: 13810435b9a7 ("objtool: Support GCC 8's cold subfunctions"). The padding nops at the end of a function aren't actually part of the function, as defined by the symbol table. So the 'func' variable in validate_branch() is getting cleared to NULL when a padding nop is encountered, breaking the fallthrough detection. If the current instruction doesn't have a function associated with it, just consider it to be part of the previously detected function by not overwriting the previous value of 'func'. Fixes: 13810435b9a7 ("objtool: Support GCC 8's cold subfunctions") Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- tools/objtool/check.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 90226791df6b..7325d89ccad9 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1959,7 +1959,8 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, return 1; } - func = insn->func ? insn->func->pfunc : NULL; + if (insn->func) + func = insn->func->pfunc; if (func && insn->ignore) { WARN_FUNC("BUG: why am I validating an ignored function?", -- 2.17.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:core/urgent] objtool: Fix function fallthrough detection 2019-05-13 17:01 ` [PATCH 2/2] objtool: Fix function fallthrough detection Josh Poimboeuf @ 2019-05-13 18:34 ` tip-bot for Josh Poimboeuf 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Josh Poimboeuf @ 2019-05-13 18:34 UTC (permalink / raw) To: linux-tip-commits Cc: jpoimboe, linux-kernel, tglx, mingo, stable, lkp, torvalds, hpa, peterz Commit-ID: e6f393bc939d566ce3def71232d8013de9aaadde Gitweb: https://git.kernel.org/tip/e6f393bc939d566ce3def71232d8013de9aaadde Author: Josh Poimboeuf <jpoimboe@redhat.com> AuthorDate: Mon, 13 May 2019 12:01:32 -0500 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Mon, 13 May 2019 20:31:17 +0200 objtool: Fix function fallthrough detection When a function falls through to the next function due to a compiler bug, objtool prints some obscure warnings. For example: drivers/regulator/core.o: warning: objtool: regulator_count_voltages()+0x95: return with modified stack frame drivers/regulator/core.o: warning: objtool: regulator_count_voltages()+0x0: stack state mismatch: cfa1=7+32 cfa2=7+8 Instead it should be printing: drivers/regulator/core.o: warning: objtool: regulator_supply_is_couple() falls through to next function regulator_count_voltages() This used to work, but was broken by the following commit: 13810435b9a7 ("objtool: Support GCC 8's cold subfunctions") The padding nops at the end of a function aren't actually part of the function, as defined by the symbol table. So the 'func' variable in validate_branch() is getting cleared to NULL when a padding nop is encountered, breaking the fallthrough detection. If the current instruction doesn't have a function associated with it, just consider it to be part of the previously detected function by not overwriting the previous value of 'func'. Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: <stable@vger.kernel.org> Fixes: 13810435b9a7 ("objtool: Support GCC 8's cold subfunctions") Link: http://lkml.kernel.org/r/546d143820cd08a46624ae8440d093dd6c902cae.1557766718.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org> --- tools/objtool/check.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 90226791df6b..7325d89ccad9 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1959,7 +1959,8 @@ static int validate_branch(struct objtool_file *file, struct instruction *first, return 1; } - func = insn->func ? insn->func->pfunc : NULL; + if (insn->func) + func = insn->func->pfunc; if (func && insn->ignore) { WARN_FUNC("BUG: why am I validating an ignored function?", ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-13 18:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-05-13 17:01 [PATCH 0/2] objtool: Fix function fallthrough detection Josh Poimboeuf 2019-05-13 17:01 ` [PATCH 1/2] objtool: Don't use ignore flag for fake jumps Josh Poimboeuf 2019-05-13 18:34 ` [tip:core/urgent] " tip-bot for Josh Poimboeuf 2019-05-13 17:01 ` [PATCH 2/2] objtool: Fix function fallthrough detection Josh Poimboeuf 2019-05-13 18:34 ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox