[RFC 07/13] objtool: Extract code to validate instruction from the validate branch loop

[Alexandre Chartre <alexandre.chartre@oracle.com>]

The code to validate a branch loops through all instructions of the
branch and validate each instruction. Move the code to validate an
instruction to a separated function.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c | 375 +++++++++++++++++++++++-------------------
 1 file changed, 208 insertions(+), 167 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2c73c8d3515d..36ec08b8d654 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3527,6 +3527,11 @@ static bool skip_alt_group(struct instruction *insn)
 	return alt_insn->type == INSN_CLAC || alt_insn->type == INSN_STAC;
 }
 
+static int validate_insn(struct objtool_file *file, struct symbol *func,
+			 struct instruction *insn, struct insn_state *statep,
+			 struct instruction *prev_insn, struct instruction *next_insn,
+			 bool *validate_nextp);
+
 /*
  * Follow the branch starting at the given instruction, and recursively follow
  * any other branches (jumps).  Meanwhile, track the frame pointer state at
@@ -3536,10 +3541,9 @@ static bool skip_alt_group(struct instruction *insn)
 static int validate_branch(struct objtool_file *file, struct symbol *func,
 			   struct instruction *insn, struct insn_state state)
 {
-	struct alternative *alt;
 	struct instruction *next_insn, *prev_insn = NULL;
 	struct section *sec;
-	u8 visited;
+	bool validate_next;
 	int ret;
 
 	if (func && func->ignore)
@@ -3566,232 +3570,269 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			return 1;
 		}
 
-		visited = VISITED_BRANCH << state.uaccess;
-		if (insn->visited & VISITED_BRANCH_MASK) {
-			if (!insn->hint && !insn_cfi_match(insn, &state.cfi))
-				return 1;
+		ret = validate_insn(file, func, insn, &state,
+				    prev_insn, next_insn,
+				    &validate_next);
+		if (!validate_next)
+			break;
 
-			if (insn->visited & visited)
+		if (!next_insn) {
+			if (state.cfi.cfa.base == CFI_UNDEFINED)
 				return 0;
-		} else {
-			nr_insns_visited++;
+			if (file->ignore_unreachables)
+				return 0;
+
+			WARN("%s%sunexpected end of section %s",
+			     func ? func->name : "", func ? "(): " : "",
+			     sec->name);
+			return 1;
 		}
 
-		if (state.noinstr)
-			state.instr += insn->instr;
+		prev_insn = insn;
+		insn = next_insn;
+	}
 
-		if (insn->hint) {
-			if (insn->restore) {
-				struct instruction *save_insn, *i;
+	return ret;
+}
 
-				i = insn;
-				save_insn = NULL;
+static int validate_insn(struct objtool_file *file, struct symbol *func,
+			 struct instruction *insn, struct insn_state *statep,
+			 struct instruction *prev_insn, struct instruction *next_insn,
+			 bool *validate_nextp)
+{
+	struct alternative *alt;
+	u8 visited;
+	int ret;
 
-				sym_for_each_insn_continue_reverse(file, func, i) {
-					if (i->save) {
-						save_insn = i;
-						break;
-					}
-				}
+	/*
+	 * Indicate that, by default, the calling function should not
+	 * validate the next instruction and validation should be
+	 * stopped. That way this function can stop validation by just
+	 * returning at any point before reaching the end of the function.
+	 *
+	 * If the end of this function is reached then that means that the
+	 * validation should continue and the caller should validate the
+	 * next instruction, so *validate_nextp will be set to true at
+	 * that point.
+	 */
+	*validate_nextp = false;
 
-				if (!save_insn) {
-					WARN_INSN(insn, "no corresponding CFI save for CFI restore");
-					return 1;
-				}
+	visited = VISITED_BRANCH << statep->uaccess;
+	if (insn->visited & VISITED_BRANCH_MASK) {
+		if (!insn->hint && !insn_cfi_match(insn, &statep->cfi))
+			return 1;
 
-				if (!save_insn->visited) {
-					/*
-					 * If the restore hint insn is at the
-					 * beginning of a basic block and was
-					 * branched to from elsewhere, and the
-					 * save insn hasn't been visited yet,
-					 * defer following this branch for now.
-					 * It will be seen later via the
-					 * straight-line path.
-					 */
-					if (!prev_insn)
-						return 0;
+		if (insn->visited & visited)
+			return 0;
+	} else {
+		nr_insns_visited++;
+	}
 
-					WARN_INSN(insn, "objtool isn't smart enough to handle this CFI save/restore combo");
-					return 1;
-				}
+	if (statep->noinstr)
+		statep->instr += insn->instr;
 
-				insn->cfi = save_insn->cfi;
-				nr_cfi_reused++;
-			}
+	if (insn->hint) {
+		if (insn->restore) {
+			struct instruction *save_insn, *i;
 
-			state.cfi = *insn->cfi;
-		} else {
-			/* XXX track if we actually changed state.cfi */
+			i = insn;
+			save_insn = NULL;
 
-			if (prev_insn && !cficmp(prev_insn->cfi, &state.cfi)) {
-				insn->cfi = prev_insn->cfi;
-				nr_cfi_reused++;
-			} else {
-				insn->cfi = cfi_hash_find_or_add(&state.cfi);
+			sym_for_each_insn_continue_reverse(file, func, i) {
+				if (i->save) {
+					save_insn = i;
+					break;
+				}
 			}
-		}
 
-		insn->visited |= visited;
+			if (!save_insn) {
+				WARN_INSN(insn, "no corresponding CFI save for CFI restore");
+				return 1;
+			}
 
-		if (propagate_alt_cfi(file, insn))
-			return 1;
+			if (!save_insn->visited) {
+				/*
+				 * If the restore hint insn is at the
+				 * beginning of a basic block and was
+				 * branched to from elsewhere, and the
+				 * save insn hasn't been visited yet,
+				 * defer following this branch for now.
+				 * It will be seen later via the
+				 * straight-line path.
+				 */
+				if (!prev_insn)
+					return 0;
 
-		if (insn->alts) {
-			for (alt = insn->alts; alt; alt = alt->next) {
-				ret = validate_branch(file, func, alt->insn, state);
-				if (ret) {
-					BT_INSN(insn, "(alt)");
-					return ret;
-				}
+				WARN_INSN(insn, "objtool isn't smart enough to handle this CFI save/restore combo");
+				return 1;
 			}
+
+			insn->cfi = save_insn->cfi;
+			nr_cfi_reused++;
 		}
 
-		if (skip_alt_group(insn))
-			return 0;
+		statep->cfi = *insn->cfi;
+	} else {
+		/* XXX track if we actually changed statep->cfi */
 
-		if (handle_insn_ops(insn, next_insn, &state))
-			return 1;
+		if (prev_insn && !cficmp(prev_insn->cfi, &statep->cfi)) {
+			insn->cfi = prev_insn->cfi;
+			nr_cfi_reused++;
+		} else {
+			insn->cfi = cfi_hash_find_or_add(&statep->cfi);
+		}
+	}
 
-		switch (insn->type) {
+	insn->visited |= visited;
 
-		case INSN_RETURN:
-			return validate_return(func, insn, &state);
+	if (propagate_alt_cfi(file, insn))
+		return 1;
 
-		case INSN_CALL:
-		case INSN_CALL_DYNAMIC:
-			ret = validate_call(file, insn, &state);
-			if (ret)
+	if (insn->alts) {
+		for (alt = insn->alts; alt; alt = alt->next) {
+			ret = validate_branch(file, func, alt->insn, *statep);
+			if (ret) {
+				BT_INSN(insn, "(alt)");
 				return ret;
-
-			if (opts.stackval && func && !is_special_call(insn) &&
-			    !has_valid_stack_frame(&state)) {
-				WARN_INSN(insn, "call without frame pointer save/setup");
-				return 1;
 			}
+		}
+	}
 
-			break;
-
-		case INSN_JUMP_CONDITIONAL:
-		case INSN_JUMP_UNCONDITIONAL:
-			if (is_sibling_call(insn)) {
-				ret = validate_sibling_call(file, insn, &state);
-				if (ret)
-					return ret;
-
-			} else if (insn->jump_dest) {
-				ret = validate_branch(file, func,
-						      insn->jump_dest, state);
-				if (ret) {
-					BT_INSN(insn, "(branch)");
-					return ret;
-				}
-			}
+	if (skip_alt_group(insn))
+		return 0;
 
-			if (insn->type == INSN_JUMP_UNCONDITIONAL)
-				return 0;
+	if (handle_insn_ops(insn, next_insn, statep))
+		return 1;
 
-			break;
+	switch (insn->type) {
 
-		case INSN_JUMP_DYNAMIC:
-		case INSN_JUMP_DYNAMIC_CONDITIONAL:
-			if (is_sibling_call(insn)) {
-				ret = validate_sibling_call(file, insn, &state);
-				if (ret)
-					return ret;
-			}
+	case INSN_RETURN:
+		return validate_return(func, insn, statep);
 
-			if (insn->type == INSN_JUMP_DYNAMIC)
-				return 0;
+	case INSN_CALL:
+	case INSN_CALL_DYNAMIC:
+		ret = validate_call(file, insn, statep);
+		if (ret)
+			return ret;
 
-			break;
+		if (opts.stackval && func && !is_special_call(insn) &&
+		    !has_valid_stack_frame(statep)) {
+			WARN_INSN(insn, "call without frame pointer save/setup");
+			return 1;
+		}
 
-		case INSN_SYSCALL:
-			if (func && (!next_insn || !next_insn->hint)) {
-				WARN_INSN(insn, "unsupported instruction in callable function");
-				return 1;
-			}
+		break;
 
-			break;
+	case INSN_JUMP_CONDITIONAL:
+	case INSN_JUMP_UNCONDITIONAL:
+		if (is_sibling_call(insn)) {
+			ret = validate_sibling_call(file, insn, statep);
+			if (ret)
+				return ret;
 
-		case INSN_SYSRET:
-			if (func && (!next_insn || !next_insn->hint)) {
-				WARN_INSN(insn, "unsupported instruction in callable function");
-				return 1;
+		} else if (insn->jump_dest) {
+			ret = validate_branch(file, func,
+					      insn->jump_dest, *statep);
+			if (ret) {
+				BT_INSN(insn, "(branch)");
+				return ret;
 			}
+		}
 
+		if (insn->type == INSN_JUMP_UNCONDITIONAL)
 			return 0;
 
-		case INSN_STAC:
-			if (!opts.uaccess)
-				break;
+		break;
 
-			if (state.uaccess) {
-				WARN_INSN(insn, "recursive UACCESS enable");
-				return 1;
-			}
+	case INSN_JUMP_DYNAMIC:
+	case INSN_JUMP_DYNAMIC_CONDITIONAL:
+		if (is_sibling_call(insn)) {
+			ret = validate_sibling_call(file, insn, statep);
+			if (ret)
+				return ret;
+		}
 
-			state.uaccess = true;
-			break;
+		if (insn->type == INSN_JUMP_DYNAMIC)
+			return 0;
 
-		case INSN_CLAC:
-			if (!opts.uaccess)
-				break;
+		break;
 
-			if (!state.uaccess && func) {
-				WARN_INSN(insn, "redundant UACCESS disable");
-				return 1;
-			}
+	case INSN_SYSCALL:
+		if (func && (!next_insn || !next_insn->hint)) {
+			WARN_INSN(insn, "unsupported instruction in callable function");
+			return 1;
+		}
 
-			if (func_uaccess_safe(func) && !state.uaccess_stack) {
-				WARN_INSN(insn, "UACCESS-safe disables UACCESS");
-				return 1;
-			}
+		break;
 
-			state.uaccess = false;
-			break;
+	case INSN_SYSRET:
+		if (func && (!next_insn || !next_insn->hint)) {
+			WARN_INSN(insn, "unsupported instruction in callable function");
+			return 1;
+		}
 
-		case INSN_STD:
-			if (state.df) {
-				WARN_INSN(insn, "recursive STD");
-				return 1;
-			}
+		return 0;
 
-			state.df = true;
+	case INSN_STAC:
+		if (!opts.uaccess)
 			break;
 
-		case INSN_CLD:
-			if (!state.df && func) {
-				WARN_INSN(insn, "redundant CLD");
-				return 1;
-			}
+		if (statep->uaccess) {
+			WARN_INSN(insn, "recursive UACCESS enable");
+			return 1;
+		}
 
-			state.df = false;
-			break;
+		statep->uaccess = true;
+		break;
 
-		default:
+	case INSN_CLAC:
+		if (!opts.uaccess)
 			break;
+
+		if (!statep->uaccess && func) {
+			WARN_INSN(insn, "redundant UACCESS disable");
+			return 1;
 		}
 
-		if (insn->dead_end)
-			return 0;
+		if (func_uaccess_safe(func) && !statep->uaccess_stack) {
+			WARN_INSN(insn, "UACCESS-safe disables UACCESS");
+			return 1;
+		}
 
-		if (!next_insn) {
-			if (state.cfi.cfa.base == CFI_UNDEFINED)
-				return 0;
-			if (file->ignore_unreachables)
-				return 0;
+		statep->uaccess = false;
+		break;
 
-			WARN("%s%sunexpected end of section %s",
-			     func ? func->name : "", func ? "(): " : "",
-			     sec->name);
+	case INSN_STD:
+		if (statep->df) {
+			WARN_INSN(insn, "recursive STD");
 			return 1;
 		}
 
-		prev_insn = insn;
-		insn = next_insn;
+		statep->df = true;
+		break;
+
+	case INSN_CLD:
+		if (!statep->df && func) {
+			WARN_INSN(insn, "redundant CLD");
+			return 1;
+		}
+
+		statep->df = false;
+		break;
+
+	default:
+		break;
 	}
 
+	if (insn->dead_end)
+		return 0;
+
+	/*
+	 * Indicate that the caller should validate the next
+	 * instruction and continue the validation.
+	 */
+	*validate_nextp = true;
+
 	return 0;
 }
 
-- 
2.43.5