Эх сурвалжийг харах

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
Linus Torvalds 9 жил өмнө
parent
commit
6527efba38

+ 2 - 1
Makefile

@@ -1008,7 +1008,8 @@ prepare0: archprepare FORCE
 prepare: prepare0 prepare-objtool
 prepare: prepare0 prepare-objtool
 
 
 ifdef CONFIG_STACK_VALIDATION
 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)
   ifeq ($(has_libelf),1)
     objtool_target := tools/objtool FORCE
     objtool_target := tools/objtool FORCE
   else
   else

+ 29 - 9
tools/objtool/Documentation/stack-validation.txt

@@ -299,18 +299,38 @@ they mean, and suggestions for how to fix them.
 Errors in .c files
 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
 Also see the above section for .S file errors for more information what
 the individual error messages mean.
 the individual error messages mean.

+ 72 - 25
tools/objtool/builtin-check.c

@@ -54,6 +54,7 @@ struct instruction {
 	struct symbol *call_dest;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;
 	struct list_head alts;
+	struct symbol *func;
 };
 };
 
 
 struct alternative {
 struct alternative {
@@ -66,6 +67,7 @@ struct objtool_file {
 	struct list_head insn_list;
 	struct list_head insn_list;
 	DECLARE_HASHTABLE(insn_hash, 16);
 	DECLARE_HASHTABLE(insn_hash, 16);
 	struct section *rodata, *whitelist;
 	struct section *rodata, *whitelist;
+	bool ignore_unreachables, c_file;
 };
 };
 
 
 const char *objname;
 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 */
 			/* sibling call */
 			return 0;
 			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)
 static int decode_instructions(struct objtool_file *file)
 {
 {
 	struct section *sec;
 	struct section *sec;
+	struct symbol *func;
 	unsigned long offset;
 	unsigned long offset;
 	struct instruction *insn;
 	struct instruction *insn;
 	int ret;
 	int ret;
@@ -281,6 +284,21 @@ static int decode_instructions(struct objtool_file *file)
 			hash_add(file->insn_hash, &insn->hash, insn->offset);
 			hash_add(file->insn_hash, &insn->hash, insn->offset);
 			list_add_tail(&insn->list, &file->insn_list);
 			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;
 	return 0;
@@ -664,13 +682,40 @@ static int add_func_switch_tables(struct objtool_file *file,
 						text_rela->addend);
 						text_rela->addend);
 
 
 		/*
 		/*
-		 * TODO: Document where this is needed, or get rid of it.
-		 *
 		 * rare case:   jmpq *[addr](%rip)
 		 * 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,
 			rodata_rela = find_rela_by_dest(file->rodata,
 							text_rela->addend + 4);
 							text_rela->addend + 4);
+			if (rodata_rela)
+				file->ignore_unreachables = true;
+		}
 
 
 		if (!rodata_rela)
 		if (!rodata_rela)
 			continue;
 			continue;
@@ -732,9 +777,6 @@ static int decode_sections(struct objtool_file *file)
 {
 {
 	int ret;
 	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);
 	ret = decode_instructions(file);
 	if (ret)
 	if (ret)
 		return ret;
 		return ret;
@@ -799,6 +841,7 @@ static int validate_branch(struct objtool_file *file,
 	struct alternative *alt;
 	struct alternative *alt;
 	struct instruction *insn;
 	struct instruction *insn;
 	struct section *sec;
 	struct section *sec;
+	struct symbol *func = NULL;
 	unsigned char state;
 	unsigned char state;
 	int ret;
 	int ret;
 
 
@@ -813,6 +856,16 @@ static int validate_branch(struct objtool_file *file,
 	}
 	}
 
 
 	while (1) {
 	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 (insn->visited) {
 			if (frame_state(insn->state) != frame_state(state)) {
 			if (frame_state(insn->state) != frame_state(state)) {
 				WARN_FUNC("frame pointer state mismatch",
 				WARN_FUNC("frame pointer state mismatch",
@@ -823,13 +876,6 @@ static int validate_branch(struct objtool_file *file,
 			return 0;
 			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->visited = true;
 		insn->state = state;
 		insn->state = state;
 
 
@@ -1035,12 +1081,8 @@ static int validate_functions(struct objtool_file *file)
 				continue;
 				continue;
 
 
 			insn = find_insn(file, sec, func->offset);
 			insn = find_insn(file, sec, func->offset);
-			if (!insn) {
-				WARN("%s(): can't find starting instruction",
-				     func->name);
-				warnings++;
+			if (!insn)
 				continue;
 				continue;
-			}
 
 
 			ret = validate_branch(file, insn, 0);
 			ret = validate_branch(file, insn, 0);
 			warnings += ret;
 			warnings += ret;
@@ -1056,13 +1098,14 @@ static int validate_functions(struct objtool_file *file)
 				if (insn->visited)
 				if (insn->visited)
 					continue;
 					continue;
 
 
-				if (!ignore_unreachable_insn(func, insn) &&
-				    !warnings) {
-					WARN_FUNC("function has unreachable instruction", insn->sec, insn->offset);
-					warnings++;
-				}
-
 				insn->visited = true;
 				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);
 	INIT_LIST_HEAD(&file.insn_list);
 	hash_init(file.insn_hash);
 	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);
 	ret = decode_sections(&file);
 	if (ret < 0)
 	if (ret < 0)