Now diagnosing missing vmctx arguments (fixes #376) (#384)

* Now diagnosing missing vmctx arguments (fixes #376).

* Added filetest for fix of #376.

* Respect formatting rules in verifier/mod.rs.

* Added parameters for each use of vmctx in test files.

* Added comments on additions on vmctx verifications.

cranelift/docs/heapex-dyn.cton

@@ -1,11 +1,11 @@
 test verifier
 
-function %add_members(i32) -> f32 baldrdash {
+function %add_members(i32, i64 vmctx) -> f32 baldrdash {
     gv0 = vmctx+64
     gv1 = vmctx+72
     heap0 = dynamic gv0, min 0x1000, bound gv1, guard 0
 
-ebb0(v0: i32):
+ebb0(v0: i32, v6: i64):
     v1 = heap_addr.i64 heap0, v0, 20
     v2 = load.f32 v1+16
     v3 = heap_addr.i64 heap0, v0, 24

cranelift/docs/heapex-sm32.cton

@@ -1,10 +1,10 @@
 test verifier
 
-function %add_members(i32) -> f32 baldrdash {
+function %add_members(i32, i32 vmctx) -> f32 baldrdash {
     gv0 = vmctx+64
     heap0 = static gv0, min 0x1000, bound 0x10_0000, guard 0x1000
 
-ebb0(v0: i32):
+ebb0(v0: i32, v5: i32):
     v1 = heap_addr.i32 heap0, v0, 1
     v2 = load.f32 v1+16
     v3 = load.f32 v1+20

cranelift/docs/heapex-sm64.cton

@@ -1,10 +1,10 @@
 test verifier
 
-function %add_members(i32) -> f32 baldrdash {
+function %add_members(i32, i64 vmctx) -> f32 baldrdash {
     gv0 = vmctx+64
     heap0 = static gv0, min 0x1000, bound 0x1_0000_0000, guard 0x8000_0000
 
-ebb0(v0: i32):
+ebb0(v0: i32, v5: i64):
     v1 = heap_addr.i64 heap0, v0, 1
     v2 = load.f32 v1+16
     v3 = load.f32 v1+20

cranelift/filetests/parser/memory.cton

@@ -1,7 +1,7 @@
 test cat
 test verifier
 
-function %vmglobal() -> i32 {
+function %vmglobal(i64 vmctx) -> i32 {
     gv3 = vmctx+16
     ; check: gv3 = vmctx+16
     gv4 = vmctx+0
@@ -9,29 +9,29 @@ function %vmglobal() -> i32 {
     ; not: +0
     gv5 = vmctx -256
     ; check: gv5 = vmctx-256
-ebb0:
+ebb0(v0: i64):
     v1 = global_value.i32 gv3
     ; check: v1 = global_value.i32 gv3
     return v1
 }
 
-function %deref() -> i32 {
+function %deref(i64 vmctx) -> i32 {
     gv3 = vmctx+16
     gv4 = deref(gv3)-32
     ; check: gv4 = deref(gv3)-32
-ebb0:
+ebb0(v0: i64):
     v1 = global_value.i32 gv4
     ; check: v1 = global_value.i32 gv4
     return v1
 }
 
 ; Refer to a global value before it's been declared.
-function %backref() -> i32 {
+function %backref(i64 vmctx) -> i32 {
     gv1 = deref(gv2)-32
     ; check: gv1 = deref(gv2)-32
     gv2 = vmctx+16
     ; check: gv2 = vmctx+16
-ebb0:
+ebb0(v0: i64):
     v1 = global_value.i32 gv1
     return v1
 }
@@ -51,21 +51,21 @@ ebb0:
 }
 
 ; Declare static heaps.
-function %sheap(i32) -> i64 {
+function %sheap(i32, i64 vmctx) -> i64 {
     heap1 = static reserved_reg, min 0x1_0000, bound 0x1_0000_0000, guard 0x8000_0000
     heap2 = static gv5, guard 0x1000, bound 0x1_0000
     gv5 = vmctx+64
 
     ; check: heap1 = static reserved_reg, min 0x0001_0000, bound 0x0001_0000_0000, guard 0x8000_0000
     ; check: heap2 = static gv5, min 0, bound 0x0001_0000, guard 4096
-ebb0(v1: i32):
+ebb0(v1: i32, v2: i64):
-    v2 = heap_addr.i64 heap1, v1, 0
+    v3 = heap_addr.i64 heap1, v1, 0
-    ; check: v2 = heap_addr.i64 heap1, v1, 0
+    ; check: v3 = heap_addr.i64 heap1, v1, 0
-    return v2
+    return v3
 }
 
 ; Declare dynamic heaps.
-function %dheap(i32) -> i64 {
+function %dheap(i32, i64 vmctx) -> i64 {
     heap1 = dynamic reserved_reg, min 0x1_0000, bound gv6, guard 0x8000_0000
     heap2 = dynamic gv5, bound gv6, guard 0x1000
     gv5 = vmctx+64
@@ -73,8 +73,8 @@ function %dheap(i32) -> i64 {
 
     ; check: heap1 = dynamic reserved_reg, min 0x0001_0000, bound gv6, guard 0x8000_0000
     ; check: heap2 = dynamic gv5, min 0, bound gv6, guard 4096
-ebb0(v1: i32):
+ebb0(v1: i32, v2: i64):
-    v2 = heap_addr.i64 heap2, v1, 0
+    v3 = heap_addr.i64 heap2, v1, 0
-    ; check: v2 = heap_addr.i64 heap2, v1, 0
+    ; check: v3 = heap_addr.i64 heap2, v1, 0
-    return v2
+    return v3
 }

cranelift/filetests/verifier/undeclared_vmctx.cton

@@ -0,0 +1,17 @@
+test verifier
+
+; Using a vmctx global value without declaring it first leads to an error.
+function %vmglobal_err(i64) -> i64 {
+    gv4 = vmctx+0 ; error: undeclared vmctx reference
+ebb0(v0: i64):
+    v1 = global_value.i64 gv4
+    return v1
+}
+
+; If it is declared, all is fine.
+function %vmglobal_ok(i64 vmctx) -> i64 {
+    gv4 = vmctx+0
+ebb0(v0: i64):
+    v1 = global_value.i64 gv4
+    return v1
+}

lib/codegen/src/verifier/mod.rs

@@ -44,6 +44,7 @@
 //! Global values
 //!
 //! - Detect cycles in deref(base) declarations.
+//! - Detect use of 'vmctx' global value when no corresponding parameter is defined.
 //!
 //! TODO:
 //! Ad hoc checking
@@ -170,7 +171,9 @@ impl<'a> Verifier<'a> {
         }
     }
 
-    // Check for cycles in the global value declarations.
+    // Check for:
+    //  - cycles in the global value declarations.
+    //  - use of 'vmctx' when no special parameter declares it.
     fn verify_global_values(&self) -> VerifierResult<()> {
         let mut seen = SparseSet::new();
 
@@ -186,6 +189,15 @@ impl<'a> Verifier<'a> {
 
                 cur = base;
             }
+
+            if let ir::GlobalValueData::VMContext { .. } = self.func.global_values[cur] {
+                if self.func
+                    .special_param(ir::ArgumentPurpose::VMContext)
+                    .is_none()
+                {
+                    return err!(cur, "undeclared vmctx reference {}", cur);
+                }
+            }
         }
 
         Ok(())