Commit 6d01f28b authored by Josh Poimboeuf's avatar Josh Poimboeuf Committed by Ingo Molnar

objtool: Support new GCC 6 switch jump table pattern

This fixes some false positive objtool warnings seen with gcc 6.1.1:

  kernel/trace/ring_buffer.o: warning: objtool: ring_buffer_read_page()+0x36c: sibling call from callable instruction with changed frame pointer
  arch/x86/kernel/reboot.o: warning: objtool: native_machine_emergency_restart()+0x139: sibling call from callable instruction with changed frame pointer
  lib/xz/xz_dec_stream.o: warning: objtool: xz_dec_run()+0xc2: sibling call from callable instruction with changed frame pointer

With GCC 6, a new code pattern is sometimes used to access a switch
statement jump table in .rodata, which objtool doesn't yet recognize:

  mov [rodata addr],%reg1
  ... some instructions ...
  jmpq *(%reg1,%reg2,8)

Add support for detecting that pattern.  The detection code is rather
crude, but it's still effective at weeding out false positives and
catching real warnings.  It can be refined later once objtool starts
reading DWARF CFI.
Reported-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: default avatarJosh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
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: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/b8c9503b4ad8c8a827cc5400db4c1b40a3ea07bc.1469751119.git.jpoimboe@redhat.comSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent ce4f06dc
...@@ -107,6 +107,12 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file, ...@@ -107,6 +107,12 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file,
insn->offset < func->offset + func->len; \ insn->offset < func->offset + func->len; \
insn = list_next_entry(insn, list)) insn = list_next_entry(insn, list))
#define func_for_each_insn_continue_reverse(file, func, insn) \
for (insn = list_prev_entry(insn, list); \
&insn->list != &file->insn_list && \
insn->sec == func->sec && insn->offset >= func->offset; \
insn = list_prev_entry(insn, list))
#define sec_for_each_insn_from(file, insn) \ #define sec_for_each_insn_from(file, insn) \
for (; insn; insn = next_insn_same_sec(file, insn)) for (; insn; insn = next_insn_same_sec(file, insn))
...@@ -664,65 +670,95 @@ static int add_switch_table(struct objtool_file *file, struct symbol *func, ...@@ -664,65 +670,95 @@ static int add_switch_table(struct objtool_file *file, struct symbol *func,
return 0; return 0;
} }
static int add_func_switch_tables(struct objtool_file *file, /*
struct symbol *func) * find_switch_table() - Given a dynamic jump, find the switch jump table in
{ * .rodata associated with it.
struct instruction *insn, *prev_jump; *
struct rela *text_rela, *rodata_rela, *prev_rela = NULL; * There are 3 basic patterns:
int ret;
prev_jump = NULL;
func_for_each_insn(file, func, insn) {
if (insn->type != INSN_JUMP_DYNAMIC)
continue;
text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
insn->len);
if (!text_rela || text_rela->sym != file->rodata->sym)
continue;
/* common case: jmpq *[addr](,%rax,8) */
rodata_rela = find_rela_by_dest(file->rodata,
text_rela->addend);
/*
* rare case: jmpq *[addr](%rip)
* *
* This check is for a rare gcc quirk, currently only seen in * 1. jmpq *[rodata addr](,%reg,8)
* 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 * This is the most common case by far. It jumps to an address in a simple
* switch jump table, modifies it, and then hard-codes the jump * jump table which is stored in .rodata.
* (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 * 2. jmpq *[rodata addr](%rip)
* 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: * This is caused by a rare GCC quirk, currently only seen in three driver
* - https://lkml.kernel.org/r/201603231906.LWcVUpxm%25fengguang.wu@intel.com * functions in the kernel, only with certain obscure non-distro configs.
* - https://lkml.kernel.org/r/201603271114.K9i45biy%25fengguang.wu@intel.com
* - https://lkml.kernel.org/r/201603291058.zuJ6ben1%25fengguang.wu@intel.com
* *
* gcc bug: * As part of an optimization, GCC makes a copy of an existing switch jump
* - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70604 * 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.
*
* 3. mov [rodata addr],%reg1
* ... some instructions ...
* jmpq *(%reg1,%reg2,8)
*
* This is a fairly uncommon pattern which is new for GCC 6. As of this
* writing, there are 11 occurrences of it in the allmodconfig kernel.
*
* TODO: Once we have DWARF CFI and smarter instruction decoding logic,
* ensure the same register is used in the mov and jump instructions.
*/ */
if (!rodata_rela) { static struct rela *find_switch_table(struct objtool_file *file,
struct symbol *func,
struct instruction *insn)
{
struct rela *text_rela, *rodata_rela;
text_rela = find_rela_by_dest_range(insn->sec, insn->offset, insn->len);
if (text_rela && text_rela->sym == file->rodata->sym) {
/* case 1 */
rodata_rela = find_rela_by_dest(file->rodata, rodata_rela = find_rela_by_dest(file->rodata,
text_rela->addend + 4); text_rela->addend);
if (rodata_rela) if (rodata_rela)
return rodata_rela;
/* case 2 */
rodata_rela = find_rela_by_dest(file->rodata,
text_rela->addend + 4);
if (!rodata_rela)
return NULL;
file->ignore_unreachables = true; file->ignore_unreachables = true;
return rodata_rela;
} }
if (!rodata_rela) /* case 3 */
func_for_each_insn_continue_reverse(file, func, insn) {
if (insn->type == INSN_JUMP_UNCONDITIONAL ||
insn->type == INSN_JUMP_DYNAMIC)
break;
text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
insn->len);
if (text_rela && text_rela->sym == file->rodata->sym)
return find_rela_by_dest(file->rodata,
text_rela->addend);
}
return NULL;
}
static int add_func_switch_tables(struct objtool_file *file,
struct symbol *func)
{
struct instruction *insn, *prev_jump = NULL;
struct rela *rela, *prev_rela = NULL;
int ret;
func_for_each_insn(file, func, insn) {
if (insn->type != INSN_JUMP_DYNAMIC)
continue;
rela = find_switch_table(file, func, insn);
if (!rela)
continue; continue;
/* /*
...@@ -732,13 +768,13 @@ static int add_func_switch_tables(struct objtool_file *file, ...@@ -732,13 +768,13 @@ static int add_func_switch_tables(struct objtool_file *file,
*/ */
if (prev_jump) { if (prev_jump) {
ret = add_switch_table(file, func, prev_jump, prev_rela, ret = add_switch_table(file, func, prev_jump, prev_rela,
rodata_rela); rela);
if (ret) if (ret)
return ret; return ret;
} }
prev_jump = insn; prev_jump = insn;
prev_rela = rodata_rela; prev_rela = rela;
} }
if (prev_jump) { if (prev_jump) {
......
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