Ping... are there any comments on this?
On 03/08/18 19:40, 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.
>
> The fix is comprised of three parts:
>
> 1. Flagging all .rodata sections when importing elf information for
> easier checking later.
>
> 2. Keeping a reference to the section each relocation is from in order
> to get the list_head for the other relocations in that section.
>
> 3. Finding jump tables by following relocations to .rodata sections,
> rather than always referencing a single global .rodata section.
>
> The patch has been tested without data sections enabled and no
> differences in the resulting orc unwind information were seen.
>
> Note that as objtool adds terminators to end of each .text section the
> unwind information generated between a function+data sections build and
> a normal build aren't directly comparable. Manual inspection suggests
> that objtool is now generating the correct information, or at least
> making more of an effort to do so than it did previously.
>
> Signed-off-by: Allan Xavier <allan.x.xav...@oracle.com>
> ---
> Changes since v1:
> - Cleaned up section symbol check.
> - Fixed brackets.
> - Moved mark_rodata to decode_sections().
> - Excluded *.str1.[18] sections from rodata sections.
>
> tools/objtool/check.c | 39 +++++++++++++++++++++++++++++++++------
> tools/objtool/check.h | 4 ++--
> tools/objtool/elf.c | 1 +
> tools/objtool/elf.h | 3 ++-
> 4 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index f4a25bd1871fb..e3a5d53c4b83b 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -836,7 +836,7 @@ static int add_switch_table(struct objtool_file *file,
> struct instruction *insn,
> struct symbol *pfunc = insn->func->pfunc;
> unsigned int prev_offset = 0;
>
> - list_for_each_entry_from(rela, &file->rodata->rela->rela_list, list) {
> + list_for_each_entry_from(rela, &table->rela_sec->rela_list, list) {
> if (rela == next_table)
> break;
>
> @@ -926,6 +926,7 @@ static struct rela *find_switch_table(struct objtool_file
> *file,
> {
> struct rela *text_rela, *rodata_rela;
> struct instruction *orig_insn = insn;
> + struct section *rodata_sec;
> unsigned long table_offset;
>
> /*
> @@ -953,10 +954,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->type != STT_SECTION ||
> + !text_rela->sym->sec->rodata)
> continue;
>
> table_offset = text_rela->addend;
> + rodata_sec = text_rela->sym->sec;
> +
> if (text_rela->type == R_X86_64_PC32)
> table_offset += 4;
>
> @@ -964,10 +968,10 @@ static struct rela *find_switch_table(struct
> objtool_file *file,
> * Make sure the .rodata address isn't associated with a
> * symbol. gcc jump tables are anonymous data.
> */
> - if (find_symbol_containing(file->rodata, table_offset))
> + if (find_symbol_containing(rodata_sec, table_offset))
> continue;
>
> - rodata_rela = find_rela_by_dest(file->rodata, table_offset);
> + rodata_rela = find_rela_by_dest(rodata_sec, table_offset);
> if (rodata_rela) {
> /*
> * Use of RIP-relative switch jumps is quite rare, and
> @@ -1052,7 +1056,7 @@ static int add_switch_table_alts(struct objtool_file
> *file)
> struct symbol *func;
> int ret;
>
> - if (!file->rodata || !file->rodata->rela)
> + if (!file->rodata)
> return 0;
>
> for_each_sec(file, sec) {
> @@ -1197,10 +1201,34 @@ static int read_retpoline_hints(struct objtool_file
> *file)
> return 0;
> }
>
> +static void mark_rodata(struct objtool_file *file)
> +{
> + struct section *sec;
> + bool found = false;
> + static const char *str1 = ".str1.";
> + const int str1len = strlen(str1) + 1;
> +
> + for_each_sec(file, sec) {
> + if (strstr(sec->name, ".rodata") == sec->name) {
> + char *str1pos = sec->name + strlen(sec->name) - str1len;
> +
> + /* Skips over .rodata.str1.1 and .rodata.str.1.8 */
> + if (strstr(sec->name, str1) != str1pos) {
> + sec->rodata = true;
> + found = true;
> + }
> + }
> + }
> +
> + file->rodata = found;
> +}
> +
> static int decode_sections(struct objtool_file *file)
> {
> int ret;
>
> + mark_rodata(file);
> +
> ret = decode_instructions(file);
> if (ret)
> return ret;
> @@ -2170,7 +2198,6 @@ 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;
> diff --git a/tools/objtool/check.h b/tools/objtool/check.h
> index c6b68fcb926ff..a039521b67530 100644
> --- a/tools/objtool/check.h
> +++ b/tools/objtool/check.h
> @@ -60,8 +60,8 @@ struct objtool_file {
> struct elf *elf;
> struct list_head insn_list;
> DECLARE_HASHTABLE(insn_hash, 16);
> - struct section *rodata, *whitelist;
> - bool ignore_unreachables, c_file, hints;
> + struct section *whitelist;
> + bool ignore_unreachables, c_file, hints, rodata;
> };
>
> int check(const char *objname, bool orc);
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index 7ec85d567598c..f7082de1ee829 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -379,6 +379,7 @@ static int read_relas(struct elf *elf)
> rela->offset = rela->rela.r_offset;
> symndx = GELF_R_SYM(rela->rela.r_info);
> rela->sym = find_symbol_by_index(elf, symndx);
> + rela->rela_sec = sec;
> if (!rela->sym) {
> WARN("can't find rela entry symbol %d for %s",
> symndx, sec->name);
> diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
> index de5cd2ddded98..bc97ed86b9cd8 100644
> --- a/tools/objtool/elf.h
> +++ b/tools/objtool/elf.h
> @@ -48,7 +48,7 @@ struct section {
> char *name;
> int idx;
> unsigned int len;
> - bool changed, text;
> + bool changed, text, rodata;
> };
>
> struct symbol {
> @@ -68,6 +68,7 @@ struct rela {
> struct list_head list;
> struct hlist_node hash;
> GElf_Rela rela;
> + struct section *rela_sec;
> struct symbol *sym;
> unsigned int type;
> unsigned long offset;
>
