From 8133fbb4240ae2918d993defa0f6824864412f56 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 9 Mar 2016 00:06:58 -0600 Subject: objtool: Fix false positive warnings for functions with multiple switch statements Ingo reported [1] some false positive objtool warnings: drivers/net/wireless/realtek/rtlwifi/base.o: warning: objtool: rtlwifi_rate_mapping()+0x2e7: frame pointer state mismatch drivers/net/wireless/realtek/rtlwifi/base.o: warning: objtool: rtlwifi_rate_mapping()+0x2f3: frame pointer state mismatch ... And so did the 0-day bot [2]: drivers/gpu/drm/radeon/cik.o: warning: objtool: cik_tiling_mode_table_init()+0x6ce: call without frame pointer save/setup drivers/gpu/drm/radeon/cik.o: warning: objtool: cik_tiling_mode_table_init()+0x72b: call without frame pointer save/setup ... Both sets of warnings involve functions which have multiple switch statements. When there's more than one switch statement in a function, objtool interprets all the switch jump tables as a single table. If the targets of one jump table assume a stack frame and the targets of another one don't, it prints false positive warnings. Fix the bug by detecting the size of each switch jump table. For multiple tables, each one ends where the next one begins. [1] https://lkml.kernel.org/r/20160308103716.GA9618@gmail.com [2] https://lists.01.org/pipermail/kbuild-all/2016-March/018124.html Reported-by: Ingo Molnar Reported-by: kbuild test robot Signed-off-by: Josh Poimboeuf Cc: Andrew Morton Cc: Andy Lutomirski Cc: Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo Cc: Bernd Petrovitsch Cc: Borislav Petkov Cc: Chris J Arges Cc: Jiri Slaby Cc: Linus Torvalds Cc: Michal Marek Cc: Namhyung Kim Cc: Pedro Alves Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: live-patching@vger.kernel.org Link: http://lkml.kernel.org/r/2d7eecc6bc52d301f494b80f5fd62c2b6c895658.1457502970.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- tools/objtool/builtin-check.c | 145 +++++++++++++++++++++++++++++------------- 1 file changed, 100 insertions(+), 45 deletions(-) (limited to 'tools') diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c index cdbdd7d9333a..cf1e48dbfa97 100644 --- a/tools/objtool/builtin-check.c +++ b/tools/objtool/builtin-check.c @@ -61,6 +61,7 @@ struct alternative { struct objtool_file { struct elf *elf; struct list_head insn_list; + struct section *rodata; }; const char *objname; @@ -599,73 +600,125 @@ out: return ret; } -/* - * For some switch statements, gcc generates a jump table in the .rodata - * section which contains a list of addresses within the function to jump to. - * This finds these jump tables and adds them to the insn->alts lists. - */ -static int add_switch_table_alts(struct objtool_file *file) +static int add_switch_table(struct objtool_file *file, struct symbol *func, + struct instruction *insn, struct rela *table, + struct rela *next_table) { - struct instruction *insn, *alt_insn; - struct rela *rodata_rela, *text_rela; - struct section *rodata; - struct symbol *func; + struct rela *rela = table; + struct instruction *alt_insn; struct alternative *alt; - for_each_insn(file, insn) { + list_for_each_entry_from(rela, &file->rodata->rela->rela_list, list) { + if (rela == next_table) + break; + + if (rela->sym->sec != insn->sec || + rela->addend <= func->offset || + rela->addend >= func->offset + func->len) + break; + + alt_insn = find_insn(file, insn->sec, rela->addend); + if (!alt_insn) { + WARN("%s: can't find instruction at %s+0x%x", + file->rodata->rela->name, insn->sec->name, + rela->addend); + return -1; + } + + alt = malloc(sizeof(*alt)); + if (!alt) { + WARN("malloc failed"); + return -1; + } + + alt->insn = alt_insn; + list_add_tail(&alt->list, &insn->alts); + } + + return 0; +} + +static int add_func_switch_tables(struct objtool_file *file, + struct symbol *func) +{ + struct instruction *insn, *prev_jump; + struct rela *text_rela, *rodata_rela, *prev_rela; + 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 || strcmp(text_rela->sym->name, ".rodata")) - continue; - - rodata = find_section_by_name(file->elf, ".rodata"); - if (!rodata || !rodata->rela) + if (!text_rela || text_rela->sym != file->rodata->sym) continue; /* common case: jmpq *[addr](,%rax,8) */ - rodata_rela = find_rela_by_dest(rodata, text_rela->addend); + rodata_rela = find_rela_by_dest(file->rodata, + text_rela->addend); - /* rare case: jmpq *[addr](%rip) */ + /* + * TODO: Document where this is needed, or get rid of it. + * + * rare case: jmpq *[addr](%rip) + */ if (!rodata_rela) - rodata_rela = find_rela_by_dest(rodata, + rodata_rela = find_rela_by_dest(file->rodata, text_rela->addend + 4); + if (!rodata_rela) continue; - func = find_containing_func(insn->sec, insn->offset); - if (!func) { - WARN_FUNC("can't find containing func", - insn->sec, insn->offset); - return -1; + /* + * We found a switch table, but we don't know yet how big it + * is. Don't add it until we reach the end of the function or + * the beginning of another switch table in the same function. + */ + if (prev_jump) { + ret = add_switch_table(file, func, prev_jump, prev_rela, + rodata_rela); + if (ret) + return ret; } - list_for_each_entry_from(rodata_rela, &rodata->rela->rela_list, - list) { - if (rodata_rela->sym->sec != insn->sec || - rodata_rela->addend <= func->offset || - rodata_rela->addend >= func->offset + func->len) - break; + prev_jump = insn; + prev_rela = rodata_rela; + } - alt_insn = find_insn(file, insn->sec, - rodata_rela->addend); - if (!alt_insn) { - WARN("%s: can't find instruction at %s+0x%x", - rodata->rela->name, insn->sec->name, - rodata_rela->addend); - return -1; - } + if (prev_jump) { + ret = add_switch_table(file, func, prev_jump, prev_rela, NULL); + if (ret) + return ret; + } - alt = malloc(sizeof(*alt)); - if (!alt) { - WARN("malloc failed"); - return -1; - } + return 0; +} - alt->insn = alt_insn; - list_add_tail(&alt->list, &insn->alts); +/* + * For some switch statements, gcc generates a jump table in the .rodata + * section which contains a list of addresses within the function to jump to. + * This finds these jump tables and adds them to the insn->alts lists. + */ +static int add_switch_table_alts(struct objtool_file *file) +{ + struct section *sec; + struct symbol *func; + int ret; + + if (!file->rodata || !file->rodata->rela) + return 0; + + list_for_each_entry(sec, &file->elf->sections, list) { + list_for_each_entry(func, &sec->symbol_list, list) { + if (func->type != STT_FUNC) + continue; + + ret = add_func_switch_tables(file, func); + if (ret) + return ret; } } @@ -676,6 +729,8 @@ static int decode_sections(struct objtool_file *file) { int ret; + file->rodata = find_section_by_name(file->elf, ".rodata"); + ret = decode_instructions(file); if (ret) return ret; -- cgit v1.2.3