Re: [PATCH v4 02/13] objtool: Factor out CFI hints

From: Miroslav Benes
Date: Wed Mar 25 2020 - 14:26:10 EST


On Wed, 25 Mar 2020, Peter Zijlstra wrote:

> Move the application of CFI hints into it's own function.
> No functional changes intended.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Acked-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> ---
> tools/objtool/check.c | 67 ++++++++++++++++++++++++++++----------------------
> 1 file changed, 38 insertions(+), 29 deletions(-)
>
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2033,6 +2033,41 @@ static int validate_return(struct symbol
> return 0;
> }
>
> +static int apply_insn_hint(struct objtool_file *file, struct section *sec,
> + struct symbol *func, struct instruction *insn,
> + struct insn_state *state)
> +{
> + if (insn->restore) {
> + struct instruction *save_insn, *i;
> +
> + i = insn;
> + save_insn = NULL;
> + sym_for_each_insn_continue_reverse(file, func, i) {
> + if (i->save) {
> + save_insn = i;
> + break;
> + }
> + }
> +
> + if (!save_insn) {
> + WARN_FUNC("no corresponding CFI save for CFI restore",
> + sec, insn->offset);
> + return 1;
> + }
> +
> + if (!save_insn->visited) {
> + WARN_FUNC("objtool isn't smart enough to handle this CFI save/restore combo",
> + sec, insn->offset);
> + return 1;
> + }
> +
> + insn->state = save_insn->state;
> + }
> +
> + state = insn->state;

It does not matter, because it will change later again, but there should
be

*state = insn->state;

here, right?

> + return 0;
> +}
> +
> /*
> * Follow the branch starting at the given instruction, and recursively follow
> * any other branches (jumps). Meanwhile, track the frame pointer state at
> @@ -2081,35 +2116,9 @@ static int validate_branch(struct objtoo
> }
>
> if (insn->hint) {
> - if (insn->restore) {
> - struct instruction *save_insn, *i;
> -
> - i = insn;
> - save_insn = NULL;
> - sym_for_each_insn_continue_reverse(file, func, i) {
> - if (i->save) {
> - save_insn = i;
> - break;
> - }
> - }
> -
> - if (!save_insn) {
> - WARN_FUNC("no corresponding CFI save for CFI restore",
> - sec, insn->offset);
> - return 1;
> - }
> -
> - if (!save_insn->visited) {
> - WARN_FUNC("objtool isn't smart enough to handle this CFI save/restore combo",
> - sec, insn->offset);
> - return 1;
> - }
> -
> - insn->state = save_insn->state;
> - }
> -
> - state = insn->state;
> -
> + ret = apply_insn_hint(file, sec, func, insn, &state);
> + if (ret)
> + return ret;
> } else
> insn->state = state;
>
>
>