lkml.org 
[lkml]   [2018]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2018-07-26 18:37    [W:0.217 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site