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

[Miroslav Benes]

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;
>  
> 
>