On Sun, Mar 10, 2019 at 02:10:46PM +0100, Peter Zijlstra wrote:
> On Fri, Mar 08, 2019 at 03:02:09PM -0600, Josh Poimboeuf wrote:
>
> > These gotos make my head spin. Again I would much prefer a small amount
> > of code duplication over this.
>
> something like so then?
Yeah, that looks a lot nicer to me. Thanks.
>
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1888,6 +1888,23 @@ static inline const char *insn_dest_name
> return "{dynamic}";
> }
>
> +static int validate_call(struct instruction *insn, struct insn_state *state)
> +{
> + if (state->uaccess && !func_uaccess_safe(insn->call_dest)) {
> + WARN_FUNC("call to %s() with UACCESS enabled",
> + insn->sec, insn->offset, insn_dest_name(insn));
> + return 1;
> + }
> +
> + if (state->df) {
> + WARN_FUNC("call to %s() with DF set",
> + insn->sec, insn->offset, insn_dest_name(insn));
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> /*
> * Follow the branch starting at the given instruction, and recursively
> follow
> * any other branches (jumps). Meanwhile, track the frame pointer state at
> @@ -2036,25 +2053,9 @@ static int validate_branch(struct objtoo
>
> case INSN_CALL:
> case INSN_CALL_DYNAMIC:
> -do_call:
> - if (state.uaccess &&
> !func_uaccess_safe(insn->call_dest)) {
> - WARN_FUNC("call to %s() with UACCESS enabled",
> - sec, insn->offset,
> insn_dest_name(insn));
> - return 1;
> - }
> -
> - if (state.df) {
> - WARN_FUNC("call to %s() with DF set",
> - sec, insn->offset,
> insn_dest_name(insn));
> - return 1;
> - }
> -
> - if (insn->type == INSN_JUMP_UNCONDITIONAL ||
> - insn->type == INSN_JUMP_DYNAMIC)
> - return 0;
> -
> - if (insn->type == INSN_JUMP_CONDITIONAL)
> - break;
> + ret = validate_call(insn, &state);
> + if (ret)
> + return ret;
>
> if (insn->type == INSN_CALL) {
> if (is_fentry_call(insn))
> @@ -2077,13 +2078,15 @@ static int validate_branch(struct objtoo
> case INSN_JUMP_CONDITIONAL:
> case INSN_JUMP_UNCONDITIONAL:
> if (func && !insn->jump_dest) {
> -do_sibling_call:
> + /* sibling call */
> if (has_modified_stack_frame(&state)) {
> WARN_FUNC("sibling call from callable
> instruction with modified stack frame",
> sec, insn->offset);
> return 1;
> }
> - goto do_call;
> + ret = validate_call(insn, &state);
> + if (ret)
> + return ret;
>
> } else if (insn->jump_dest &&
> (!func || !insn->jump_dest->func ||
> @@ -2104,8 +2107,17 @@ static int validate_branch(struct objtoo
> break;
>
> case INSN_JUMP_DYNAMIC:
> - if (func && list_empty(&insn->alts))
> - goto do_sibling_call;
> + if (func && list_empty(&insn->alts)) {
> + /* sibling call */
> + if (has_modified_stack_frame(&state)) {
> + WARN_FUNC("sibling call from callable
> instruction with modified stack frame",
> + sec, insn->offset);
> + return 1;
> + }
> + ret = validate_call(insn, &state);
> + if (ret)
> + return ret;
> + }
>
> return 0;
>
>
--
Josh
