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
