diff options
author | Linus Torvalds <torvalds@linux-foundation.org> | 2016-04-23 14:25:01 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2016-04-23 14:25:01 -0400 |
commit | 6527efba38a06410f19670adba8a1951f8fe3d3c (patch) | |
tree | 760861ec87730eed4eaf4f6c20a2509edbf8af87 | |
parent | 68dc08b5802f606b5b50dde558533b643b6d9526 (diff) | |
parent | c2bb9e32e2315971a8535fee77335c04a739d71d (diff) |
Merge branch 'core-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull objtool fixes from Ingo Molnar:
"A handful of objtool fixes: two improvements to how warnings are
printed plus a false positive warning fix, and build environment fix"
* 'core-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip:
objtool: Fix Makefile to properly see if libelf is supported
objtool: Detect falling through to the next function
objtool: Add workaround for GCC switch jump table bug
-rw-r--r-- | Makefile | 3 | ||||
-rw-r--r-- | tools/objtool/Documentation/stack-validation.txt | 38 | ||||
-rw-r--r-- | tools/objtool/builtin-check.c | 97 |
3 files changed, 103 insertions, 35 deletions
@@ -1008,7 +1008,8 @@ prepare0: archprepare FORCE | |||
1008 | prepare: prepare0 prepare-objtool | 1008 | prepare: prepare0 prepare-objtool |
1009 | 1009 | ||
1010 | ifdef CONFIG_STACK_VALIDATION | 1010 | ifdef CONFIG_STACK_VALIDATION |
1011 | has_libelf := $(shell echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf - &> /dev/null && echo 1 || echo 0) | 1011 | has_libelf := $(call try-run,\ |
1012 | echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -,1,0) | ||
1012 | ifeq ($(has_libelf),1) | 1013 | ifeq ($(has_libelf),1) |
1013 | objtool_target := tools/objtool FORCE | 1014 | objtool_target := tools/objtool FORCE |
1014 | else | 1015 | 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. | |||
299 | Errors in .c files | 299 | Errors in .c files |
300 | ------------------ | 300 | ------------------ |
301 | 301 | ||
302 | If you're getting an objtool error in a compiled .c file, chances are | 302 | 1. c_file.o: warning: objtool: funcA() falls through to next function funcB() |
303 | the file uses an asm() statement which has a "call" instruction. An | ||
304 | asm() statement with a call instruction must declare the use of the | ||
305 | stack pointer in its output operand. For example, on x86_64: | ||
306 | 303 | ||
307 | register void *__sp asm("rsp"); | 304 | This means that funcA() doesn't end with a return instruction or an |
308 | asm volatile("call func" : "+r" (__sp)); | 305 | unconditional jump, and that objtool has determined that the function |
306 | can fall through into the next function. There could be different | ||
307 | reasons for this: | ||
309 | 308 | ||
310 | Otherwise the stack frame may not get created before the call. | 309 | 1) funcA()'s last instruction is a call to a "noreturn" function like |
310 | panic(). In this case the noreturn function needs to be added to | ||
311 | objtool's hard-coded global_noreturns array. Feel free to bug the | ||
312 | objtool maintainer, or you can submit a patch. | ||
311 | 313 | ||
312 | Another possible cause for errors in C code is if the Makefile removes | 314 | 2) funcA() uses the unreachable() annotation in a section of code |
313 | -fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options. | 315 | that is actually reachable. |
316 | |||
317 | 3) If funcA() calls an inline function, the object code for funcA() | ||
318 | might be corrupt due to a gcc bug. For more details, see: | ||
319 | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 | ||
320 | |||
321 | 2. If you're getting any other objtool error in a compiled .c file, it | ||
322 | may be because the file uses an asm() statement which has a "call" | ||
323 | instruction. An asm() statement with a call instruction must declare | ||
324 | the use of the stack pointer in its output operand. For example, on | ||
325 | x86_64: | ||
326 | |||
327 | register void *__sp asm("rsp"); | ||
328 | asm volatile("call func" : "+r" (__sp)); | ||
329 | |||
330 | Otherwise the stack frame may not get created before the call. | ||
331 | |||
332 | 3. Another possible cause for errors in C code is if the Makefile removes | ||
333 | -fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options. | ||
314 | 334 | ||
315 | Also see the above section for .S file errors for more information what | 335 | Also see the above section for .S file errors for more information what |
316 | the individual error messages mean. | 336 | 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 { | |||
54 | struct symbol *call_dest; | 54 | struct symbol *call_dest; |
55 | struct instruction *jump_dest; | 55 | struct instruction *jump_dest; |
56 | struct list_head alts; | 56 | struct list_head alts; |
57 | struct symbol *func; | ||
57 | }; | 58 | }; |
58 | 59 | ||
59 | struct alternative { | 60 | struct alternative { |
@@ -66,6 +67,7 @@ struct objtool_file { | |||
66 | struct list_head insn_list; | 67 | struct list_head insn_list; |
67 | DECLARE_HASHTABLE(insn_hash, 16); | 68 | DECLARE_HASHTABLE(insn_hash, 16); |
68 | struct section *rodata, *whitelist; | 69 | struct section *rodata, *whitelist; |
70 | bool ignore_unreachables, c_file; | ||
69 | }; | 71 | }; |
70 | 72 | ||
71 | const char *objname; | 73 | const char *objname; |
@@ -228,7 +230,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func, | |||
228 | } | 230 | } |
229 | } | 231 | } |
230 | 232 | ||
231 | if (insn->type == INSN_JUMP_DYNAMIC) | 233 | if (insn->type == INSN_JUMP_DYNAMIC && list_empty(&insn->alts)) |
232 | /* sibling call */ | 234 | /* sibling call */ |
233 | return 0; | 235 | return 0; |
234 | } | 236 | } |
@@ -248,6 +250,7 @@ static int dead_end_function(struct objtool_file *file, struct symbol *func) | |||
248 | static int decode_instructions(struct objtool_file *file) | 250 | static int decode_instructions(struct objtool_file *file) |
249 | { | 251 | { |
250 | struct section *sec; | 252 | struct section *sec; |
253 | struct symbol *func; | ||
251 | unsigned long offset; | 254 | unsigned long offset; |
252 | struct instruction *insn; | 255 | struct instruction *insn; |
253 | int ret; | 256 | int ret; |
@@ -281,6 +284,21 @@ static int decode_instructions(struct objtool_file *file) | |||
281 | hash_add(file->insn_hash, &insn->hash, insn->offset); | 284 | hash_add(file->insn_hash, &insn->hash, insn->offset); |
282 | list_add_tail(&insn->list, &file->insn_list); | 285 | list_add_tail(&insn->list, &file->insn_list); |
283 | } | 286 | } |
287 | |||
288 | list_for_each_entry(func, &sec->symbol_list, list) { | ||
289 | if (func->type != STT_FUNC) | ||
290 | continue; | ||
291 | |||
292 | if (!find_insn(file, sec, func->offset)) { | ||
293 | WARN("%s(): can't find starting instruction", | ||
294 | func->name); | ||
295 | return -1; | ||
296 | } | ||
297 | |||
298 | func_for_each_insn(file, func, insn) | ||
299 | if (!insn->func) | ||
300 | insn->func = func; | ||
301 | } | ||
284 | } | 302 | } |
285 | 303 | ||
286 | return 0; | 304 | return 0; |
@@ -664,13 +682,40 @@ static int add_func_switch_tables(struct objtool_file *file, | |||
664 | text_rela->addend); | 682 | text_rela->addend); |
665 | 683 | ||
666 | /* | 684 | /* |
667 | * TODO: Document where this is needed, or get rid of it. | ||
668 | * | ||
669 | * rare case: jmpq *[addr](%rip) | 685 | * rare case: jmpq *[addr](%rip) |
686 | * | ||
687 | * This check is for a rare gcc quirk, currently only seen in | ||
688 | * three driver functions in the kernel, only with certain | ||
689 | * obscure non-distro configs. | ||
690 | * | ||
691 | * As part of an optimization, gcc makes a copy of an existing | ||
692 | * switch jump table, modifies it, and then hard-codes the jump | ||
693 | * (albeit with an indirect jump) to use a single entry in the | ||
694 | * table. The rest of the jump table and some of its jump | ||
695 | * targets remain as dead code. | ||
696 | * | ||
697 | * In such a case we can just crudely ignore all unreachable | ||
698 | * instruction warnings for the entire object file. Ideally we | ||
699 | * would just ignore them for the function, but that would | ||
700 | * require redesigning the code quite a bit. And honestly | ||
701 | * that's just not worth doing: unreachable instruction | ||
702 | * warnings are of questionable value anyway, and this is such | ||
703 | * a rare issue. | ||
704 | * | ||
705 | * kbuild reports: | ||
706 | * - https://lkml.kernel.org/r/201603231906.LWcVUpxm%25fengguang.wu@intel.com | ||
707 | * - https://lkml.kernel.org/r/201603271114.K9i45biy%25fengguang.wu@intel.com | ||
708 | * - https://lkml.kernel.org/r/201603291058.zuJ6ben1%25fengguang.wu@intel.com | ||
709 | * | ||
710 | * gcc bug: | ||
711 | * - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70604 | ||
670 | */ | 712 | */ |
671 | if (!rodata_rela) | 713 | if (!rodata_rela) { |
672 | rodata_rela = find_rela_by_dest(file->rodata, | 714 | rodata_rela = find_rela_by_dest(file->rodata, |
673 | text_rela->addend + 4); | 715 | text_rela->addend + 4); |
716 | if (rodata_rela) | ||
717 | file->ignore_unreachables = true; | ||
718 | } | ||
674 | 719 | ||
675 | if (!rodata_rela) | 720 | if (!rodata_rela) |
676 | continue; | 721 | continue; |
@@ -732,9 +777,6 @@ static int decode_sections(struct objtool_file *file) | |||
732 | { | 777 | { |
733 | int ret; | 778 | int ret; |
734 | 779 | ||
735 | file->whitelist = find_section_by_name(file->elf, "__func_stack_frame_non_standard"); | ||
736 | file->rodata = find_section_by_name(file->elf, ".rodata"); | ||
737 | |||
738 | ret = decode_instructions(file); | 780 | ret = decode_instructions(file); |
739 | if (ret) | 781 | if (ret) |
740 | return ret; | 782 | return ret; |
@@ -799,6 +841,7 @@ static int validate_branch(struct objtool_file *file, | |||
799 | struct alternative *alt; | 841 | struct alternative *alt; |
800 | struct instruction *insn; | 842 | struct instruction *insn; |
801 | struct section *sec; | 843 | struct section *sec; |
844 | struct symbol *func = NULL; | ||
802 | unsigned char state; | 845 | unsigned char state; |
803 | int ret; | 846 | int ret; |
804 | 847 | ||
@@ -813,6 +856,16 @@ static int validate_branch(struct objtool_file *file, | |||
813 | } | 856 | } |
814 | 857 | ||
815 | while (1) { | 858 | while (1) { |
859 | if (file->c_file && insn->func) { | ||
860 | if (func && func != insn->func) { | ||
861 | WARN("%s() falls through to next function %s()", | ||
862 | func->name, insn->func->name); | ||
863 | return 1; | ||
864 | } | ||
865 | |||
866 | func = insn->func; | ||
867 | } | ||
868 | |||
816 | if (insn->visited) { | 869 | if (insn->visited) { |
817 | if (frame_state(insn->state) != frame_state(state)) { | 870 | if (frame_state(insn->state) != frame_state(state)) { |
818 | WARN_FUNC("frame pointer state mismatch", | 871 | WARN_FUNC("frame pointer state mismatch", |
@@ -823,13 +876,6 @@ static int validate_branch(struct objtool_file *file, | |||
823 | return 0; | 876 | return 0; |
824 | } | 877 | } |
825 | 878 | ||
826 | /* | ||
827 | * Catch a rare case where a noreturn function falls through to | ||
828 | * the next function. | ||
829 | */ | ||
830 | if (is_fentry_call(insn) && (state & STATE_FENTRY)) | ||
831 | return 0; | ||
832 | |||
833 | insn->visited = true; | 879 | insn->visited = true; |
834 | insn->state = state; | 880 | insn->state = state; |
835 | 881 | ||
@@ -1035,12 +1081,8 @@ static int validate_functions(struct objtool_file *file) | |||
1035 | continue; | 1081 | continue; |
1036 | 1082 | ||
1037 | insn = find_insn(file, sec, func->offset); | 1083 | insn = find_insn(file, sec, func->offset); |
1038 | if (!insn) { | 1084 | if (!insn) |
1039 | WARN("%s(): can't find starting instruction", | ||
1040 | func->name); | ||
1041 | warnings++; | ||
1042 | continue; | 1085 | continue; |
1043 | } | ||
1044 | 1086 | ||
1045 | ret = validate_branch(file, insn, 0); | 1087 | ret = validate_branch(file, insn, 0); |
1046 | warnings += ret; | 1088 | warnings += ret; |
@@ -1056,13 +1098,14 @@ static int validate_functions(struct objtool_file *file) | |||
1056 | if (insn->visited) | 1098 | if (insn->visited) |
1057 | continue; | 1099 | continue; |
1058 | 1100 | ||
1059 | if (!ignore_unreachable_insn(func, insn) && | ||
1060 | !warnings) { | ||
1061 | WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset); | ||
1062 | warnings++; | ||
1063 | } | ||
1064 | |||
1065 | insn->visited = true; | 1101 | insn->visited = true; |
1102 | |||
1103 | if (file->ignore_unreachables || warnings || | ||
1104 | ignore_unreachable_insn(func, insn)) | ||
1105 | continue; | ||
1106 | |||
1107 | WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset); | ||
1108 | warnings++; | ||
1066 | } | 1109 | } |
1067 | } | 1110 | } |
1068 | } | 1111 | } |
@@ -1133,6 +1176,10 @@ int cmd_check(int argc, const char **argv) | |||
1133 | 1176 | ||
1134 | INIT_LIST_HEAD(&file.insn_list); | 1177 | INIT_LIST_HEAD(&file.insn_list); |
1135 | hash_init(file.insn_hash); | 1178 | hash_init(file.insn_hash); |
1179 | file.whitelist = find_section_by_name(file.elf, "__func_stack_frame_non_standard"); | ||
1180 | file.rodata = find_section_by_name(file.elf, ".rodata"); | ||
1181 | file.ignore_unreachables = false; | ||
1182 | file.c_file = find_section_by_name(file.elf, ".comment"); | ||
1136 | 1183 | ||
1137 | ret = decode_sections(&file); | 1184 | ret = decode_sections(&file); |
1138 | if (ret < 0) | 1185 | if (ret < 0) |