Re: [RFC PATCH v1 3/3] powerpc: WIP draft support to objtool check

From: Peter Zijlstra
Date: Fri Jun 16 2023 - 10:43:57 EST



Few comments..

On Fri, Jun 16, 2023 at 03:47:52PM +0200, Christophe Leroy wrote:

> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 0fcf99c91400..f945fe271706 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -236,6 +236,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
> "x86_64_start_reservations",
> "xen_cpu_bringup_again",
> "xen_start_kernel",
> + "longjmp",
> };
>
> if (!func)
> @@ -2060,13 +2061,12 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
> * instruction.
> */
> list_for_each_entry_from(reloc, &table->sec->reloc_list, list) {
> -
> /* Check for the end of the table: */
> if (reloc != table && reloc->jump_table_start)
> break;
>
> /* Make sure the table entries are consecutive: */
> - if (prev_offset && reloc->offset != prev_offset + 8)
> + if (prev_offset && reloc->offset != prev_offset + 4)

Do we want a global variable (from elf.c) called elf_sizeof_long or so?

> break;
>
> /* Detect function pointers from contiguous objects: */
> @@ -2074,7 +2074,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
> reloc->addend == pfunc->offset)
> break;
>
> - dest_insn = find_insn(file, reloc->sym->sec, reloc->addend);
> + if (table->jump_table_is_rel)
> + dest_insn = find_insn(file, reloc->sym->sec, reloc->addend + table->offset - reloc->offset);
> + else
> + dest_insn = find_insn(file, reloc->sym->sec, reloc->addend);

offset = reloc->addend;
if (table->jump_table_is_rel)
offset += table->offset - reloc->offset;
dest_insn = find_insn(file, reloc->sym->sec, offset);

perhaps?

> if (!dest_insn)
> break;
>
> @@ -4024,6 +4022,11 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
> if (insn->ignore || insn->type == INSN_NOP || insn->type == INSN_TRAP)
> return true;
>
> + /* powerpc relocatable files have a word in front of each relocatable function */
> + if ((file->elf->ehdr.e_machine == EM_PPC || file->elf->ehdr.e_machine == EM_PPC64) &&
> + (file->elf->ehdr.e_flags & EF_PPC_RELOCATABLE_LIB) &&
> + insn_func(next_insn_same_sec(file, insn)))
> + return true;

Can't you simply decode that word to INSN_NOP or so?