* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.