From: Ingo Molnar <mingo@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Thomas Gleixner <tglx@linutronix.de>,
Andrew Morton <akpm@linux-foundation.org>,
Arnaldo Carvalho de Melo <acme@infradead.org>,
Jiri Olsa <jolsa@redhat.com>,
Josh Poimboeuf <jpoimboe@redhat.com>
Subject: [GIT PULL] objtool fixes
Date: Sat, 23 Apr 2016 13:10:52 +0200 [thread overview]
Message-ID: <20160423111052.GA12566@gmail.com> (raw)
Linus,
Please pull the latest core-urgent-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-urgent-for-linus
# HEAD: c2bb9e32e2315971a8535fee77335c04a739d71d objtool: Fix Makefile to properly see if libelf is supported
A handful of objtool fixes: two improvements to how warnings are printed plus a
false positive warning fix, and build environment fix.
Thanks,
Ingo
------------------>
Josh Poimboeuf (2):
objtool: Add workaround for GCC switch jump table bug
objtool: Detect falling through to the next function
Steven Rostedt (1):
objtool: Fix Makefile to properly see if libelf is supported
Makefile | 3 +-
tools/objtool/Documentation/stack-validation.txt | 38 +++++++---
tools/objtool/builtin-check.c | 97 ++++++++++++++++++------
3 files changed, 103 insertions(+), 35 deletions(-)
diff --git a/Makefile b/Makefile
index 1d0aef03eae7..70ca38ef9f4b 100644
--- a/Makefile
+++ b/Makefile
@@ -1008,7 +1008,8 @@ prepare0: archprepare FORCE
prepare: prepare0 prepare-objtool
ifdef CONFIG_STACK_VALIDATION
- has_libelf := $(shell echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf - &> /dev/null && echo 1 || echo 0)
+ has_libelf := $(call try-run,\
+ echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -,1,0)
ifeq ($(has_libelf),1)
objtool_target := tools/objtool FORCE
else
diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt
index 5a95896105bc..55a60d331f47 100644
--- a/tools/objtool/Documentation/stack-validation.txt
+++ b/tools/objtool/Documentation/stack-validation.txt
@@ -299,18 +299,38 @@ they mean, and suggestions for how to fix them.
Errors in .c files
------------------
-If you're getting an objtool error in a compiled .c file, chances are
-the file uses an asm() statement which has a "call" instruction. An
-asm() statement with a call instruction must declare the use of the
-stack pointer in its output operand. For example, on x86_64:
+1. c_file.o: warning: objtool: funcA() falls through to next function funcB()
- register void *__sp asm("rsp");
- asm volatile("call func" : "+r" (__sp));
+ This means that funcA() doesn't end with a return instruction or an
+ unconditional jump, and that objtool has determined that the function
+ can fall through into the next function. There could be different
+ reasons for this:
-Otherwise the stack frame may not get created before the call.
+ 1) funcA()'s last instruction is a call to a "noreturn" function like
+ panic(). In this case the noreturn function needs to be added to
+ objtool's hard-coded global_noreturns array. Feel free to bug the
+ objtool maintainer, or you can submit a patch.
-Another possible cause for errors in C code is if the Makefile removes
--fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options.
+ 2) funcA() uses the unreachable() annotation in a section of code
+ that is actually reachable.
+
+ 3) If funcA() calls an inline function, the object code for funcA()
+ might be corrupt due to a gcc bug. For more details, see:
+ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
+
+2. If you're getting any other objtool error in a compiled .c file, it
+ may be because the file uses an asm() statement which has a "call"
+ instruction. An asm() statement with a call instruction must declare
+ the use of the stack pointer in its output operand. For example, on
+ x86_64:
+
+ register void *__sp asm("rsp");
+ asm volatile("call func" : "+r" (__sp));
+
+ Otherwise the stack frame may not get created before the call.
+
+3. Another possible cause for errors in C code is if the Makefile removes
+ -fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options.
Also see the above section for .S file errors for more information what
the individual error messages mean.
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 7515cb2e879a..e8a1e69eb92c 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -54,6 +54,7 @@ struct instruction {
struct symbol *call_dest;
struct instruction *jump_dest;
struct list_head alts;
+ struct symbol *func;
};
struct alternative {
@@ -66,6 +67,7 @@ struct objtool_file {
struct list_head insn_list;
DECLARE_HASHTABLE(insn_hash, 16);
struct section *rodata, *whitelist;
+ bool ignore_unreachables, c_file;
};
const char *objname;
@@ -228,7 +230,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
}
}
- if (insn->type == INSN_JUMP_DYNAMIC)
+ if (insn->type == INSN_JUMP_DYNAMIC && list_empty(&insn->alts))
/* sibling call */
return 0;
}
@@ -248,6 +250,7 @@ static int dead_end_function(struct objtool_file *file, struct symbol *func)
static int decode_instructions(struct objtool_file *file)
{
struct section *sec;
+ struct symbol *func;
unsigned long offset;
struct instruction *insn;
int ret;
@@ -281,6 +284,21 @@ static int decode_instructions(struct objtool_file *file)
hash_add(file->insn_hash, &insn->hash, insn->offset);
list_add_tail(&insn->list, &file->insn_list);
}
+
+ list_for_each_entry(func, &sec->symbol_list, list) {
+ if (func->type != STT_FUNC)
+ continue;
+
+ if (!find_insn(file, sec, func->offset)) {
+ WARN("%s(): can't find starting instruction",
+ func->name);
+ return -1;
+ }
+
+ func_for_each_insn(file, func, insn)
+ if (!insn->func)
+ insn->func = func;
+ }
}
return 0;
@@ -664,13 +682,40 @@ static int add_func_switch_tables(struct objtool_file *file,
text_rela->addend);
/*
- * TODO: Document where this is needed, or get rid of it.
- *
* rare case: jmpq *[addr](%rip)
+ *
+ * This check is for a rare gcc quirk, currently only seen in
+ * three driver functions in the kernel, only with certain
+ * obscure non-distro configs.
+ *
+ * As part of an optimization, gcc makes a copy of an existing
+ * switch jump table, modifies it, and then hard-codes the jump
+ * (albeit with an indirect jump) to use a single entry in the
+ * table. The rest of the jump table and some of its jump
+ * targets remain as dead code.
+ *
+ * In such a case we can just crudely ignore all unreachable
+ * instruction warnings for the entire object file. Ideally we
+ * would just ignore them for the function, but that would
+ * require redesigning the code quite a bit. And honestly
+ * that's just not worth doing: unreachable instruction
+ * warnings are of questionable value anyway, and this is such
+ * a rare issue.
+ *
+ * kbuild reports:
+ * - https://lkml.kernel.org/r/201603231906.LWcVUpxm%25fengguang.wu@intel.com
+ * - https://lkml.kernel.org/r/201603271114.K9i45biy%25fengguang.wu@intel.com
+ * - https://lkml.kernel.org/r/201603291058.zuJ6ben1%25fengguang.wu@intel.com
+ *
+ * gcc bug:
+ * - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70604
*/
- if (!rodata_rela)
+ if (!rodata_rela) {
rodata_rela = find_rela_by_dest(file->rodata,
text_rela->addend + 4);
+ if (rodata_rela)
+ file->ignore_unreachables = true;
+ }
if (!rodata_rela)
continue;
@@ -732,9 +777,6 @@ static int decode_sections(struct objtool_file *file)
{
int ret;
- file->whitelist = find_section_by_name(file->elf, "__func_stack_frame_non_standard");
- file->rodata = find_section_by_name(file->elf, ".rodata");
-
ret = decode_instructions(file);
if (ret)
return ret;
@@ -799,6 +841,7 @@ static int validate_branch(struct objtool_file *file,
struct alternative *alt;
struct instruction *insn;
struct section *sec;
+ struct symbol *func = NULL;
unsigned char state;
int ret;
@@ -813,6 +856,16 @@ static int validate_branch(struct objtool_file *file,
}
while (1) {
+ if (file->c_file && insn->func) {
+ if (func && func != insn->func) {
+ WARN("%s() falls through to next function %s()",
+ func->name, insn->func->name);
+ return 1;
+ }
+
+ func = insn->func;
+ }
+
if (insn->visited) {
if (frame_state(insn->state) != frame_state(state)) {
WARN_FUNC("frame pointer state mismatch",
@@ -823,13 +876,6 @@ static int validate_branch(struct objtool_file *file,
return 0;
}
- /*
- * Catch a rare case where a noreturn function falls through to
- * the next function.
- */
- if (is_fentry_call(insn) && (state & STATE_FENTRY))
- return 0;
-
insn->visited = true;
insn->state = state;
@@ -1035,12 +1081,8 @@ static int validate_functions(struct objtool_file *file)
continue;
insn = find_insn(file, sec, func->offset);
- if (!insn) {
- WARN("%s(): can't find starting instruction",
- func->name);
- warnings++;
+ if (!insn)
continue;
- }
ret = validate_branch(file, insn, 0);
warnings += ret;
@@ -1056,13 +1098,14 @@ static int validate_functions(struct objtool_file *file)
if (insn->visited)
continue;
- if (!ignore_unreachable_insn(func, insn) &&
- !warnings) {
- WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
- warnings++;
- }
-
insn->visited = true;
+
+ if (file->ignore_unreachables || warnings ||
+ ignore_unreachable_insn(func, insn))
+ continue;
+
+ WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
+ warnings++;
}
}
}
@@ -1133,6 +1176,10 @@ int cmd_check(int argc, const char **argv)
INIT_LIST_HEAD(&file.insn_list);
hash_init(file.insn_hash);
+ file.whitelist = find_section_by_name(file.elf, "__func_stack_frame_non_standard");
+ file.rodata = find_section_by_name(file.elf, ".rodata");
+ file.ignore_unreachables = false;
+ file.c_file = find_section_by_name(file.elf, ".comment");
ret = decode_sections(&file);
if (ret < 0)
next reply other threads:[~2016-04-23 11:10 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-23 11:10 Ingo Molnar [this message]
-- strict thread matches above, loose matches on Subject: below --
2017-02-28 7:53 [GIT PULL] objtool fixes Ingo Molnar
2017-03-01 1:55 ` Linus Torvalds
2017-03-01 6:05 ` Josh Poimboeuf
2017-03-01 22:01 Ingo Molnar
2017-11-26 12:34 Ingo Molnar
2018-11-30 6:18 Ingo Molnar
2018-11-30 21:00 ` pr-tracker-bot
2020-04-25 9:04 Ingo Molnar
2020-04-25 19:30 ` pr-tracker-bot
2021-05-15 7:46 Ingo Molnar
2021-05-15 17:55 ` pr-tracker-bot
2021-06-12 12:37 Ingo Molnar
2021-06-12 19:09 ` pr-tracker-bot
2021-06-24 6:54 Ingo Molnar
2021-06-24 16:34 ` pr-tracker-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160423111052.GA12566@gmail.com \
--to=mingo@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@infradead.org \
--cc=akpm@linux-foundation.org \
--cc=jolsa@redhat.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.