Commit b1547d31 authored by Josh Poimboeuf's avatar Josh Poimboeuf Committed by Ingo Molnar

objtool: Detect falling through to the next function

There are several cases in compiled C code where a function may not
return at the end, and may instead fall through to the next function.

That may indicate a bug in the code, or a gcc bug, or even an objtool
bug.  But in each case, objtool reports an unhelpful warning, something
like:

  drivers/scsi/qla2xxx/qla_attr.o: warning: objtool: qla2x00_get_fc_host_stats()+0x0: duplicate frame pointer save
  drivers/scsi/qla2xxx/qla_attr.o: warning: objtool: qla2x00_get_fc_host_stats()+0x0: frame pointer state mismatch

Detect this situation and print a more useful error message:

  drivers/scsi/qla2xxx/qla_attr.o: warning: objtool: qla2x00_get_host_fabric_name() falls through to next function qla2x00_get_starget_node_name()

Also add some information about this warning and its potential causes to
the documentation.
Reported-by: default avatarkbuild test robot <fengguang.wu@intel.com>
Signed-off-by: default avatarJosh Poimboeuf <jpoimboe@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jiri Olsa <jolsa@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/caa4ec6c687931db805e692d4e4bf06cd87d33e6.1460729697.git.jpoimboe@redhat.comSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 7e578441
...@@ -299,18 +299,38 @@ they mean, and suggestions for how to fix them. ...@@ -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 1. c_file.o: warning: objtool: funcA() falls through to next function funcB()
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"); This means that funcA() doesn't end with a return instruction or an
asm volatile("call func" : "+r" (__sp)); 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 2) funcA() uses the unreachable() annotation in a section of code
-fno-omit-frame-pointer or adds -fomit-frame-pointer to the gcc options. 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.
......
...@@ -54,6 +54,7 @@ struct instruction { ...@@ -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,7 +67,7 @@ struct objtool_file { ...@@ -66,7 +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; bool ignore_unreachables, c_file;
}; };
const char *objname; const char *objname;
...@@ -229,7 +230,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func, ...@@ -229,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;
} }
...@@ -249,6 +250,7 @@ static int dead_end_function(struct objtool_file *file, struct symbol *func) ...@@ -249,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;
...@@ -282,6 +284,21 @@ static int decode_instructions(struct objtool_file *file) ...@@ -282,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;
...@@ -824,6 +841,7 @@ static int validate_branch(struct objtool_file *file, ...@@ -824,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;
...@@ -838,6 +856,16 @@ static int validate_branch(struct objtool_file *file, ...@@ -838,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",
...@@ -848,13 +876,6 @@ static int validate_branch(struct objtool_file *file, ...@@ -848,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;
...@@ -1060,12 +1081,8 @@ static int validate_functions(struct objtool_file *file) ...@@ -1060,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) { if (!insn)
WARN("%s(): can't find starting instruction",
func->name);
warnings++;
continue; continue;
}
ret = validate_branch(file, insn, 0); ret = validate_branch(file, insn, 0);
warnings += ret; warnings += ret;
...@@ -1162,6 +1179,7 @@ int cmd_check(int argc, const char **argv) ...@@ -1162,6 +1179,7 @@ int cmd_check(int argc, const char **argv)
file.whitelist = find_section_by_name(file.elf, "__func_stack_frame_non_standard"); file.whitelist = find_section_by_name(file.elf, "__func_stack_frame_non_standard");
file.rodata = find_section_by_name(file.elf, ".rodata"); file.rodata = find_section_by_name(file.elf, ".rodata");
file.ignore_unreachables = false; 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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment