From db2be8ee01613d18ed73f8a96c506277b8766d95 Mon Sep 17 00:00:00 2001
From: Dan Gohman <sunfish@mozilla.com>
Date: Wed, 28 Mar 2018 20:09:49 -0700
Subject: [PATCH] Verifier: Diagnose an instruction using its own result
 values.

---
 .../verifier/defs_dominates_uses.cton         | 16 ++++++
 lib/cretonne/src/verifier/mod.rs              | 56 ++++++++++++++++---
 2 files changed, 65 insertions(+), 7 deletions(-)
 create mode 100644 cranelift/filetests/verifier/defs_dominates_uses.cton

diff --git a/cranelift/filetests/verifier/defs_dominates_uses.cton b/cranelift/filetests/verifier/defs_dominates_uses.cton
new file mode 100644
index 0000000000..1bc3819726
--- /dev/null
+++ b/cranelift/filetests/verifier/defs_dominates_uses.cton
@@ -0,0 +1,16 @@
+test verifier
+
+; Test verification that uses properly dominate defs.
+
+function %non_dominating(i32) -> i32 native {
+ebb0(v0: i32):
+    v1 = iadd.i32 v2, v0   ; error: uses value from non-dominating
+    v2 = iadd.i32 v1, v0
+    return v2
+}
+
+function %inst_uses_its_own_values(i32) -> i32 native {
+ebb0(v0: i32):
+    v1 = iadd.i32 v1, v0   ; error: uses value from itself
+    return v1
+}
diff --git a/lib/cretonne/src/verifier/mod.rs b/lib/cretonne/src/verifier/mod.rs
index 8b6eb5e972..e916094bbb 100644
--- a/lib/cretonne/src/verifier/mod.rs
+++ b/lib/cretonne/src/verifier/mod.rs
@@ -268,7 +268,7 @@ impl<'a> Verifier<'a> {
         use ir::instructions::InstructionData::*;
 
         for &arg in self.func.dfg.inst_args(inst) {
-            self.verify_value(inst, arg)?;
+            self.verify_inst_arg(inst, arg)?;
 
             // All used values must be attached to something.
             let original = self.func.dfg.resolve_aliases(arg);
@@ -278,7 +278,7 @@ impl<'a> Verifier<'a> {
         }
 
         for &res in self.func.dfg.inst_results(inst) {
-            self.verify_value(inst, res)?;
+            self.verify_inst_result(inst, res)?;
         }
 
         match self.func.dfg[inst] {
@@ -446,8 +446,16 @@ impl<'a> Verifier<'a> {
     fn verify_value(&self, loc_inst: Inst, v: Value) -> Result {
         let dfg = &self.func.dfg;
         if !dfg.value_is_valid(v) {
-            return err!(loc_inst, "invalid value reference {}", v);
+            err!(loc_inst, "invalid value reference {}", v)
+        } else {
+            Ok(())
         }
+    }
+
+    fn verify_inst_arg(&self, loc_inst: Inst, v: Value) -> Result {
+        self.verify_value(loc_inst, v)?;
+
+        let dfg = &self.func.dfg;
         let loc_ebb = self.func.layout.pp_ebb(loc_inst);
         let is_reachable = self.expected_domtree.is_reachable(loc_ebb);
 
@@ -473,14 +481,23 @@ impl<'a> Verifier<'a> {
                     );
                 }
                 // Defining instruction dominates the instruction that uses the value.
-                if is_reachable &&
-                    !self.expected_domtree.dominates(
+                if is_reachable {
+                    if !self.expected_domtree.dominates(
                         def_inst,
                         loc_inst,
                         &self.func.layout,
                     )
-                {
-                    return err!(loc_inst, "uses value from non-dominating {}", def_inst);
+                    {
+                        return err!(loc_inst, "uses value from non-dominating {}", def_inst);
+                    }
+                    if def_inst == loc_inst {
+                        return err!(
+                            loc_inst,
+                            "uses value from itself {},  {}",
+                            def_inst,
+                            loc_inst
+                        );
+                    }
                 }
             }
             ValueDef::Param(ebb, _) => {
@@ -512,6 +529,31 @@ impl<'a> Verifier<'a> {
         Ok(())
     }
 
+    fn verify_inst_result(&self, loc_inst: Inst, v: Value) -> Result {
+        self.verify_value(loc_inst, v)?;
+
+        match self.func.dfg.value_def(v) {
+            ValueDef::Result(def_inst, _) => {
+                if def_inst != loc_inst {
+                    err!(
+                        loc_inst,
+                        "instruction result {} is not defined by the instruction",
+                        v
+                    )
+                } else {
+                    Ok(())
+                }
+            }
+            ValueDef::Param(_, _) => {
+                err!(
+                    loc_inst,
+                    "instruction result {} is not defined by the instruction",
+                    v
+                )
+            }
+        }
+    }
+
     fn domtree_integrity(&self, domtree: &DominatorTree) -> Result {
         // We consider two `DominatorTree`s to be equal if they return the same immediate
         // dominator for each EBB. Therefore the current domtree is valid if it matches the freshly