Messages in this thread Patch in this message | | | Date | Thu, 23 Apr 2020 16:34:21 +0200 (CEST) | From | Miroslav Benes <> | Subject | Re: [PATCH 4/8] objtool: Add support for intra-function calls |
| |
> /* > * Find the destination instructions for all calls. > */ > @@ -715,10 +725,7 @@ static int add_call_destinations(struct > continue; > > if (!insn->call_dest) { > - WARN_FUNC("unsupported intra-function call", > - insn->sec, insn->offset); > - if (retpoline) > - WARN("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE."); > + WARN_FUNC("intra-function call", insn->sec, insn->offset);
"unsupported intra-function call"?
> return -1; > } > > @@ -741,6 +748,12 @@ static int add_call_destinations(struct > } > } else > insn->call_dest = rela->sym; > + > + /* > + * Whatever stack impact regular CALLs have, should be > + * undone by the RETURN of the called function.
* Annotated intra-function CALLs are treated as JMPs with a stack_op. * See read_intra_function_calls().
would make it a bit clearer.
> + */ > + remove_insn_ops(insn); > } > > return 0; > @@ -1416,6 +1429,57 @@ static int read_instr_hints(struct objto > return 0; > } > > +static int read_intra_function_calls(struct objtool_file *file) > +{ > + struct instruction *insn; > + struct section *sec; > + struct rela *rela; > + > + sec = find_section_by_name(file->elf, ".rela.discard.intra_function_calls"); > + if (!sec) > + return 0; > + > + list_for_each_entry(rela, &sec->rela_list, list) { > + unsigned long dest_off; > + > + if (rela->sym->type != STT_SECTION) { > + WARN("unexpected relocation symbol type in %s", > + sec->name); > + return -1; > + } > + > + insn = find_insn(file, rela->sym->sec, rela->addend); > + if (!insn) { > + WARN("bad .discard.intra_function_call entry"); > + return -1; > + } > + > + if (insn->type != INSN_CALL) { > + WARN_FUNC("intra_function_call not a direct call", > + insn->sec, insn->offset); > + return -1; > + } > + > + /* > + * Treat intra-function CALLs as JMPs, but with a stack_op. > + * Also see how setup_call_dest() strips stack_ops from normal > + * CALLs.
/* * Treat annotated intra-function CALLs as JMPs, but with a stack_op. * Also see how add_call_destinations() strips stack_ops from normal * CALLs. */
? (note added "annotated" and s/setup_call_dest/add_call_destinations/)
> + */ > + insn->type = INSN_JUMP_UNCONDITIONAL;
[...]
> @@ -2245,6 +2313,9 @@ static int validate_branch(struct objtoo > return 0; > } > > + if (handle_insn_ops(insn, &state)) > + return 1; > + > switch (insn->type) { > > case INSN_RETURN: > @@ -2304,9 +2375,6 @@ static int validate_branch(struct objtoo > break; > > case INSN_EXCEPTION_RETURN: > - if (handle_insn_ops(insn, &state)) > - return 1; > - > /* > * This handles x86's sync_core() case, where we use an > * IRET to self. All 'normal' IRET instructions are in > @@ -2326,8 +2394,6 @@ static int validate_branch(struct objtoo > return 0; > > case INSN_STACK: > - if (handle_insn_ops(insn, &state)) > - return 1; > break;
So we could get rid of INSN_STACK now as Julien proposed, couldn't we? If I am not missing something. handle_insn_ops() is called unconditionally here for all insn types and you remove stack_ops when unneeded.
We could also go ahead with Julien's proposal to remove INSN_EXCEPTION_RETURN hack and move it to tools/objtool/arch/x86/decode.c.
Miroslav
| |