Skip to content

Commit 8133fbb

Browse files
jpoimboeIngo Molnar
jpoimboe
authored and
Ingo Molnar
committed
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 <mingo@kernel.org> Reported-by: kbuild test robot <fengguang.wu@intel.com> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Arnaldo Carvalho de Melo <acme@infradead.org> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Bernd Petrovitsch <bernd@petrovitsch.priv.at> Cc: Borislav Petkov <bp@alien8.de> Cc: Chris J Arges <chris.j.arges@canonical.com> Cc: Jiri Slaby <jslaby@suse.cz> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Michal Marek <mmarek@suse.cz> Cc: Namhyung Kim <namhyung@gmail.com> Cc: Pedro Alves <palves@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: live-patching@vger.kernel.org Link: http://lkml.kernel.org/r/2d7eecc6bc52d301f494b80f5fd62c2b6c895658.1457502970.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org>
1 parent a196e17 commit 8133fbb

File tree

1 file changed

+100
-45
lines changed

1 file changed

+100
-45
lines changed

‎tools/objtool/builtin-check.c

Lines changed: 100 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ struct alternative {
6161
struct objtool_file {
6262
struct elf *elf;
6363
struct list_head insn_list;
64+
struct section *rodata;
6465
};
6566

6667
const char *objname;
@@ -599,73 +600,125 @@ static int add_special_section_alts(struct objtool_file *file)
599600
return ret;
600601
}
601602

602-
/*
603-
* For some switch statements, gcc generates a jump table in the .rodata
604-
* section which contains a list of addresses within the function to jump to.
605-
* This finds these jump tables and adds them to the insn->alts lists.
606-
*/
607-
static int add_switch_table_alts(struct objtool_file *file)
603+
static int add_switch_table(struct objtool_file *file, struct symbol *func,
604+
struct instruction *insn, struct rela *table,
605+
struct rela *next_table)
608606
{
609-
struct instruction *insn, *alt_insn;
610-
struct rela *rodata_rela, *text_rela;
611-
struct section *rodata;
612-
struct symbol *func;
607+
struct rela *rela = table;
608+
struct instruction *alt_insn;
613609
struct alternative *alt;
614610

615-
for_each_insn(file, insn) {
611+
list_for_each_entry_from(rela, &file->rodata->rela->rela_list, list) {
612+
if (rela == next_table)
613+
break;
614+
615+
if (rela->sym->sec != insn->sec ||
616+
rela->addend <= func->offset ||
617+
rela->addend >= func->offset + func->len)
618+
break;
619+
620+
alt_insn = find_insn(file, insn->sec, rela->addend);
621+
if (!alt_insn) {
622+
WARN("%s: can't find instruction at %s+0x%x",
623+
file->rodata->rela->name, insn->sec->name,
624+
rela->addend);
625+
return -1;
626+
}
627+
628+
alt = malloc(sizeof(*alt));
629+
if (!alt) {
630+
WARN("malloc failed");
631+
return -1;
632+
}
633+
634+
alt->insn = alt_insn;
635+
list_add_tail(&alt->list, &insn->alts);
636+
}
637+
638+
return 0;
639+
}
640+
641+
static int add_func_switch_tables(struct objtool_file *file,
642+
struct symbol *func)
643+
{
644+
struct instruction *insn, *prev_jump;
645+
struct rela *text_rela, *rodata_rela, *prev_rela;
646+
int ret;
647+
648+
prev_jump = NULL;
649+
650+
func_for_each_insn(file, func, insn) {
616651
if (insn->type != INSN_JUMP_DYNAMIC)
617652
continue;
618653

619654
text_rela = find_rela_by_dest_range(insn->sec, insn->offset,
620655
insn->len);
621-
if (!text_rela || strcmp(text_rela->sym->name, ".rodata"))
622-
continue;
623-
624-
rodata = find_section_by_name(file->elf, ".rodata");
625-
if (!rodata || !rodata->rela)
656+
if (!text_rela || text_rela->sym != file->rodata->sym)
626657
continue;
627658

628659
/* common case: jmpq *[addr](,%rax,8) */
629-
rodata_rela = find_rela_by_dest(rodata, text_rela->addend);
660+
rodata_rela = find_rela_by_dest(file->rodata,
661+
text_rela->addend);
630662

631-
/* rare case: jmpq *[addr](%rip) */
663+
/*
664+
* TODO: Document where this is needed, or get rid of it.
665+
*
666+
* rare case: jmpq *[addr](%rip)
667+
*/
632668
if (!rodata_rela)
633-
rodata_rela = find_rela_by_dest(rodata,
669+
rodata_rela = find_rela_by_dest(file->rodata,
634670
text_rela->addend + 4);
671+
635672
if (!rodata_rela)
636673
continue;
637674

638-
func = find_containing_func(insn->sec, insn->offset);
639-
if (!func) {
640-
WARN_FUNC("can't find containing func",
641-
insn->sec, insn->offset);
642-
return -1;
675+
/*
676+
* We found a switch table, but we don't know yet how big it
677+
* is. Don't add it until we reach the end of the function or
678+
* the beginning of another switch table in the same function.
679+
*/
680+
if (prev_jump) {
681+
ret = add_switch_table(file, func, prev_jump, prev_rela,
682+
rodata_rela);
683+
if (ret)
684+
return ret;
643685
}
644686

645-
list_for_each_entry_from(rodata_rela, &rodata->rela->rela_list,
646-
list) {
647-
if (rodata_rela->sym->sec != insn->sec ||
648-
rodata_rela->addend <= func->offset ||
649-
rodata_rela->addend >= func->offset + func->len)
650-
break;
687+
prev_jump = insn;
688+
prev_rela = rodata_rela;
689+
}
651690

652-
alt_insn = find_insn(file, insn->sec,
653-
rodata_rela->addend);
654-
if (!alt_insn) {
655-
WARN("%s: can't find instruction at %s+0x%x",
656-
rodata->rela->name, insn->sec->name,
657-
rodata_rela->addend);
658-
return -1;
659-
}
691+
if (prev_jump) {
692+
ret = add_switch_table(file, func, prev_jump, prev_rela, NULL);
693+
if (ret)
694+
return ret;
695+
}
660696

661-
alt = malloc(sizeof(*alt));
662-
if (!alt) {
663-
WARN("malloc failed");
664-
return -1;
665-
}
697+
return 0;
698+
}
666699

667-
alt->insn = alt_insn;
668-
list_add_tail(&alt->list, &insn->alts);
700+
/*
701+
* For some switch statements, gcc generates a jump table in the .rodata
702+
* section which contains a list of addresses within the function to jump to.
703+
* This finds these jump tables and adds them to the insn->alts lists.
704+
*/
705+
static int add_switch_table_alts(struct objtool_file *file)
706+
{
707+
struct section *sec;
708+
struct symbol *func;
709+
int ret;
710+
711+
if (!file->rodata || !file->rodata->rela)
712+
return 0;
713+
714+
list_for_each_entry(sec, &file->elf->sections, list) {
715+
list_for_each_entry(func, &sec->symbol_list, list) {
716+
if (func->type != STT_FUNC)
717+
continue;
718+
719+
ret = add_func_switch_tables(file, func);
720+
if (ret)
721+
return ret;
669722
}
670723
}
671724

@@ -676,6 +729,8 @@ static int decode_sections(struct objtool_file *file)
676729
{
677730
int ret;
678731

732+
file->rodata = find_section_by_name(file->elf, ".rodata");
733+
679734
ret = decode_instructions(file);
680735
if (ret)
681736
return ret;

0 commit comments

Comments
 (0)