Messages in this thread Patch in this message | | | Date | Thu, 26 Jul 2018 11:36:47 -0500 | From | Josh Poimboeuf <> | Subject | Re: [PATCH] objtool: Support multiple rodata sections. |
| |
On Wed, Jul 25, 2018 at 03:17:53PM +0100, Allan Xavier wrote: > This commit adds support for processing switch jump tables in objects > with multiple .rodata sections, such as those created when using > -ffunction-sections and -fdata-sections. Currently, objtool always > looks in .rodata for jump table information which results in many > "sibling call from callable instruction with modified stack frame" > warnings with objects compiled using those flags.
Thanks for the patch! kpatch-build also uses those flags and has a similar issue, so it's good this will be useful for multiple users.
> @@ -930,10 +931,13 @@ static struct rela *find_switch_table(struct objtool_file *file, > /* look for a relocation which references .rodata */ > text_rela = find_rela_by_dest_range(insn->sec, insn->offset, > insn->len); > - if (!text_rela || text_rela->sym != file->rodata->sym) > + if (!text_rela || !text_rela->sym->sec->rodata > + || text_rela->sym != text_rela->sym->sec->sym) > continue;
This last condition is a bit hard to follow. I think we can just check that the sym type is STT_SECTION. Also the formatting deviates a bit from the kernel style. How about something like this?
if (!text_rela || text_rela->sym->type != STT_SECTION || !text_rela->sym->sec->rodata) continue;
> @@ -2133,6 +2137,20 @@ static void cleanup(struct objtool_file *file) > elf_close(file->elf); > } > > +static bool mark_rodata(struct objtool_file *file) > +{ > + struct section *sec; > + bool found = false; > + > + for_each_sec(file, sec)
Please use brackets for the outer loop.
> + if (strstr(sec->name, ".rodata") == sec->name) { > + sec->rodata = true; > + found = true;
This check is too broad. It will also match the following sections:
.rodata.str1.8 .rodata.str1.1 .rela.rodata .init.rodata
Also, the loop should break once it finds an rodata section.
> + } > + > + return found; > +} > + > int check(const char *_objname, bool orc) > { > struct objtool_file file; > @@ -2147,10 +2165,10 @@ int check(const char *_objname, bool orc) > INIT_LIST_HEAD(&file.insn_list); > hash_init(file.insn_hash); > file.whitelist = find_section_by_name(file.elf, ".discard.func_stack_frame_non_standard"); > - file.rodata = find_section_by_name(file.elf, ".rodata"); > file.c_file = find_section_by_name(file.elf, ".comment"); > file.ignore_unreachables = no_unreachable; > file.hints = false; > + file.rodata = mark_rodata(&file);
I believe it would be cleaner to call mark_rodata() from decode_sections(). And file.rodata bool should be set from within mark_rodata().
-- Josh
| |