[PATCH v3 18/26] objtool: Fix !CFI insn_state propagation

From: Peter Zijlstra
Date: Tue Mar 24 2020 - 12:13:01 EST


Objtool keeps per instruction CFI state in struct insn_state and will
save/restore this where required. However, insn_state has grown some
!CFI state, and this must not be saved/restored (and thus lost).

Fix this by explicitly preserving the !CFI state and clarify by
restucturing the code and adding a comment.

XXX, the insn==first condition is not handled right.

Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
---
tools/objtool/check.c | 95 +++++++++++++++++++++++++++++---------------------
tools/objtool/check.h | 8 ++++
2 files changed, 64 insertions(+), 39 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2033,6 +2033,59 @@ 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 *first,
+ struct instruction *insn, struct insn_state *state)
+{
+ struct insn_state old = *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) {
+ /*
+ * Oops, no state to copy yet.
+ * Hopefully we can reach this
+ * instruction from another branch
+ * after the save insn has been
+ * visited.
+ */
+ if (insn == first)
+ return 0; // XXX
+
+ 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;
+
+ /* restore !CFI state */
+ state->df = old.df;
+ state->uaccess = old.uaccess;
+ state->uaccess_stack = old.uaccess_stack;
+
+ return 0;
+}
+
/*
* Follow the branch starting at the given instruction, and recursively follow
* any other branches (jumps). Meanwhile, track the frame pointer state at
@@ -2082,45 +2135,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) {
- /*
- * Oops, no state to copy yet.
- * Hopefully we can reach this
- * instruction from another branch
- * after the save insn has been
- * visited.
- */
- if (insn == first)
- return 0;
-
- 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, first, insn, &state);
+ if (ret)
+ return ret;
} else
insn->state = state;

--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -13,6 +13,14 @@
#include "orc.h"
#include <linux/hashtable.h>

+/*
+ * This structure (mostly) contains the instruction level CFI (Call Frame
+ * Information) and is saved/restored where appropriate; see validate_branch().
+ *
+ * However it does also contain a limited amount of !CFI state; this state must
+ * not be saved/restored along with the CFI information and is manually
+ * preserved. See apply_insn_hint().
+ */
struct insn_state {
struct cfi_reg cfa;
struct cfi_reg regs[CFI_NUM_REGS];