cranelift: Add heap support to the interpreter (#3302)

* cranelift: Add heaps to interpreter * cranelift: Add RunTest Environment mechanism to test interpret * cranelift: Remove unused `MemoryError` * cranelift: Add docs for `State::resolve_global_value` * cranelift: Rename heap tests * cranelift: Refactor heap address resolution * Fix typos and clarify docs (thanks @cfallin)
2022-07-05 17:05:26 +01:00
parent 76a2545a7f
commit e91f493ff5
7 changed files with 531 additions and 140 deletions
--- a/cranelift/filetests/filetests/runtests/heap.clif
+++ b/cranelift/filetests/filetests/runtests/heap.clif
@@ -1,10 +1,11 @@
+test interpret
 test run
 target x86_64
 target s390x
 target aarch64


-function %static_heap_i64_load_store(i64 vmctx, i64, i32) -> i32 {
+function %static_heap_i64(i64 vmctx, i64, i32) -> i32 {
    gv0 = vmctx
    gv1 = load.i64 notrap aligned gv0+0
    heap0 = static gv1, min 0x1000, bound 0x1_0000_0000, offset_guard 0, index_type i64
@@ -16,13 +17,13 @@ block0(v0: i64, v1: i64, v2: i32):
    return v4
 }
 ; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8
-; run: %static_heap_i64_load_store(0, 1) == 1
-; run: %static_heap_i64_load_store(0, -1) == -1
-; run: %static_heap_i64_load_store(16, 1) == 1
-; run: %static_heap_i64_load_store(16, -1) == -1
+; run: %static_heap_i64(0, 1) == 1
+; run: %static_heap_i64(0, -1) == -1
+; run: %static_heap_i64(16, 1) == 1
+; run: %static_heap_i64(16, -1) == -1


-function %static_heap_i32_load_store(i64 vmctx, i32, i32) -> i32 {
+function %static_heap_i32(i64 vmctx, i32, i32) -> i32 {
    gv0 = vmctx
    gv1 = load.i64 notrap aligned gv0+0
    heap0 = static gv1, min 0x1000, bound 0x1_0000_0000, offset_guard 0, index_type i32
@@ -34,13 +35,13 @@ block0(v0: i64, v1: i32, v2: i32):
    return v4
 }
 ; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8
-; run: %static_heap_i32_load_store(0, 1) == 1
-; run: %static_heap_i32_load_store(0, -1) == -1
-; run: %static_heap_i32_load_store(16, 1) == 1
-; run: %static_heap_i32_load_store(16, -1) == -1
+; run: %static_heap_i32(0, 1) == 1
+; run: %static_heap_i32(0, -1) == -1
+; run: %static_heap_i32(16, 1) == 1
+; run: %static_heap_i32(16, -1) == -1


-function %static_heap_i32_load_store_no_min(i64 vmctx, i32, i32) -> i32 {
+function %heap_no_min(i64 vmctx, i32, i32) -> i32 {
    gv0 = vmctx
    gv1 = load.i64 notrap aligned gv0+0
    heap0 = static gv1, bound 0x1_0000_0000, offset_guard 0, index_type i32
@@ -52,13 +53,13 @@ block0(v0: i64, v1: i32, v2: i32):
    return v4
 }
 ; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8
-; run: %static_heap_i32_load_store_no_min(0, 1) == 1
-; run: %static_heap_i32_load_store_no_min(0, -1) == -1
-; run: %static_heap_i32_load_store_no_min(16, 1) == 1
-; run: %static_heap_i32_load_store_no_min(16, -1) == -1
+; run: %heap_no_min(0, 1) == 1
+; run: %heap_no_min(0, -1) == -1
+; run: %heap_no_min(16, 1) == 1
+; run: %heap_no_min(16, -1) == -1


-function %dynamic_heap_i64_load_store(i64 vmctx, i64, i32) -> i32 {
+function %dynamic_i64(i64 vmctx, i64, i32) -> i32 {
    gv0 = vmctx
    gv1 = load.i64 notrap aligned gv0+0
    gv2 = load.i64 notrap aligned gv0+8
@@ -71,13 +72,13 @@ block0(v0: i64, v1: i64, v2: i32):
    return v4
 }
 ; heap: dynamic, size=0x1000, ptr=vmctx+0, bound=vmctx+8
-; run: %dynamic_heap_i64_load_store(0, 1) == 1
-; run: %dynamic_heap_i64_load_store(0, -1) == -1
-; run: %dynamic_heap_i64_load_store(16, 1) == 1
-; run: %dynamic_heap_i64_load_store(16, -1) == -1
+; run: %dynamic_i64(0, 1) == 1
+; run: %dynamic_i64(0, -1) == -1
+; run: %dynamic_i64(16, 1) == 1
+; run: %dynamic_i64(16, -1) == -1


-function %dynamic_heap_i32_load_store(i64 vmctx, i32, i32) -> i32 {
+function %dynamic_i32(i64 vmctx, i32, i32) -> i32 {
    gv0 = vmctx
    gv1 = load.i64 notrap aligned gv0+0
    gv2 = load.i64 notrap aligned gv0+8
@@ -90,13 +91,13 @@ block0(v0: i64, v1: i32, v2: i32):
    return v4
 }
 ; heap: dynamic, size=0x1000, ptr=vmctx+0, bound=vmctx+8
-; run: %dynamic_heap_i32_load_store(0, 1) == 1
-; run: %dynamic_heap_i32_load_store(0, -1) == -1
-; run: %dynamic_heap_i32_load_store(16, 1) == 1
-; run: %dynamic_heap_i32_load_store(16, -1) == -1
+; run: %dynamic_i32(0, 1) == 1
+; run: %dynamic_i32(0, -1) == -1
+; run: %dynamic_i32(16, 1) == 1
+; run: %dynamic_i32(16, -1) == -1


-function %multi_heap_load_store(i64 vmctx, i32, i32) -> i32 {
+function %multi_load_store(i64 vmctx, i32, i32) -> i32 {
    gv0 = vmctx
    gv1 = load.i64 notrap aligned gv0+0
    gv2 = load.i64 notrap aligned gv0+16
@@ -125,12 +126,47 @@ block0(v0: i64, v1: i32, v2: i32):
 }
 ; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8
 ; heap: dynamic, size=0x1000, ptr=vmctx+16, bound=vmctx+24
-; run: %multi_heap_load_store(1, 2) == 3
-; run: %multi_heap_load_store(4, 5) == 9
+; run: %multi_load_store(1, 2) == 3
+; run: %multi_load_store(4, 5) == 9



-function %static_heap_i64_load_store_unaligned(i64 vmctx, i64, i32) -> i32 {
+; Uses multiple heaps, but heap0 refers to the second heap, and heap1 refers to the first heap
+; This is a regression test for the interpreter
+function %out_of_order(i64 vmctx, i32, i32) -> i32 {
+    gv0 = vmctx
+    gv1 = load.i64 notrap aligned gv0+0
+    gv2 = load.i64 notrap aligned gv0+16
+    gv3 = load.i64 notrap aligned gv0+24
+    heap0 = dynamic gv2, bound gv3, offset_guard 0, index_type i32
+    heap1 = static gv1, min 0x1000, bound 0x1_0000_0000, offset_guard 0, index_type i64
+
+block0(v0: i64, v1: i32, v2: i32):
+    v3 = iconst.i32 0
+    v4 = iconst.i64 0
+
+    ; Store lhs in heap0
+    v5 = heap_addr.i64 heap0, v3, 4
+    store.i32 v1, v5
+
+    ; Store rhs in heap1
+    v6 = heap_addr.i64 heap1, v4, 4
+    store.i32 v2, v6
+
+
+    v7 = load.i32 v5
+    v8 = load.i32 v6
+
+    v9 = iadd.i32 v7, v8
+    return v9
+}
+; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8
+; heap: dynamic, size=0x1000, ptr=vmctx+16, bound=vmctx+24
+; run: %out_of_order(1, 2) == 3
+; run: %out_of_order(4, 5) == 9
+
+
+function %unaligned_access(i64 vmctx, i64, i32) -> i32 {
    gv0 = vmctx
    gv1 = load.i64 notrap aligned gv0+0
    heap0 = static gv1, min 0x1000, bound 0x1_0000_0000, offset_guard 0, index_type i64
@@ -142,18 +178,18 @@ block0(v0: i64, v1: i64, v2: i32):
    return v4
 }
 ; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8
-; run: %static_heap_i64_load_store_unaligned(0, 1) == 1
-; run: %static_heap_i64_load_store_unaligned(0, -1) == -1
-; run: %static_heap_i64_load_store_unaligned(1, 1) == 1
-; run: %static_heap_i64_load_store_unaligned(1, -1) == -1
-; run: %static_heap_i64_load_store_unaligned(2, 1) == 1
-; run: %static_heap_i64_load_store_unaligned(2, -1) == -1
-; run: %static_heap_i64_load_store_unaligned(3, 1) == 1
-; run: %static_heap_i64_load_store_unaligned(3, -1) == -1
+; run: %unaligned_access(0, 1) == 1
+; run: %unaligned_access(0, -1) == -1
+; run: %unaligned_access(1, 1) == 1
+; run: %unaligned_access(1, -1) == -1
+; run: %unaligned_access(2, 1) == 1
+; run: %unaligned_access(2, -1) == -1
+; run: %unaligned_access(3, 1) == 1
+; run: %unaligned_access(3, -1) == -1


 ; This stores data in the place of the pointer in the vmctx struct, not in the heap itself.
-function %static_heap_i64_iadd_imm(i64 vmctx, i32) -> i32 {
+function %iadd_imm(i64 vmctx, i32) -> i32 {
    gv0 = vmctx
    gv1 = iadd_imm.i64 gv0, 0
    heap0 = static gv1, min 0x1000, bound 0x1_0000_0000, offset_guard 0x8000_0000, index_type i64
@@ -166,5 +202,5 @@ block0(v0: i64, v1: i32):
    return v4
 }
 ; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8
-; run: %static_heap_i64_iadd_imm(1) == 1
-; run: %static_heap_i64_iadd_imm(-1) == -1
+; run: %iadd_imm(1) == 1
+; run: %iadd_imm(-1) == -1
--- a/cranelift/filetests/src/runtest_environment.rs
+++ b/cranelift/filetests/src/runtest_environment.rs
@@ -1,6 +1,5 @@
 use anyhow::anyhow;
-use cranelift_codegen::data_value::DataValue;
-use cranelift_codegen::ir::Type;
+use cranelift_codegen::ir::{ArgumentPurpose, Function};
 use cranelift_reader::parse_heap_command;
 use cranelift_reader::{Comment, HeapCommand};

@@ -45,68 +44,37 @@ impl RuntestEnvironment {
        !self.heaps.is_empty()
    }

-    /// Allocates a struct to be injected into the test.
-    pub fn runtime_struct(&self) -> RuntestContext {
-        RuntestContext::new(&self)
-    }
-}
-
-type HeapMemory = Vec<u8>;
-
-/// A struct that provides info about the environment to the test
-#[derive(Debug, Clone)]
-pub struct RuntestContext {
-    /// Store the heap memory alongside the context info so that we don't accidentally deallocate
-    /// it too early.
-    #[allow(dead_code)]
-    heaps: Vec<HeapMemory>,
-
-    /// This is the actual struct that gets passed into the `vmctx`  argument of the tests.
-    /// It has a specific memory layout that all tests agree with.
-    ///
-    /// Currently we only have to store heap info, so we store the heap start and end addresses in
-    /// a 64 bit slot for each heap.
-    ///
-    /// ┌────────────┐
-    /// │heap0: start│
-    /// ├────────────┤
-    /// │heap0: end  │
-    /// ├────────────┤
-    /// │heap1: start│
-    /// ├────────────┤
-    /// │heap1: end  │
-    /// ├────────────┤
-    /// │etc...      │
-    /// └────────────┘
-    context_struct: Vec<u64>,
-}
-
-impl RuntestContext {
-    pub fn new(env: &RuntestEnvironment) -> Self {
-        let heaps: Vec<HeapMemory> = env
-            .heaps
+    /// Allocates memory for heaps
+    pub fn allocate_memory(&self) -> Vec<HeapMemory> {
+        self.heaps
            .iter()
            .map(|cmd| {
                let size: u64 = cmd.size.into();
                vec![0u8; size as usize]
            })
-            .collect();
-
-        let context_struct = heaps
-            .iter()
-            .flat_map(|heap| [heap.as_ptr(), heap.as_ptr().wrapping_add(heap.len())])
-            .map(|p| p as usize as u64)
-            .collect();
-
-        Self {
-            heaps,
-            context_struct,
-        }
+            .collect()
    }

-    /// Creates a [DataValue] with a target isa pointer type to the context struct.
-    pub fn pointer(&self, ty: Type) -> DataValue {
-        let ptr = self.context_struct.as_ptr() as usize as i128;
-        DataValue::from_integer(ptr, ty).expect("Failed to cast pointer to native target size")
+    /// Validates the signature of a [Function] ensuring that if this environment is active, the
+    /// function has a `vmctx` argument
+    pub fn validate_signature(&self, func: &Function) -> Result<(), String> {
+        let first_arg_is_vmctx = func
+            .signature
+            .params
+            .first()
+            .map(|p| p.purpose == ArgumentPurpose::VMContext)
+            .unwrap_or(false);
+
+        if !first_arg_is_vmctx && self.is_active() {
+            return Err(concat!(
+                "This test requests a heap, but the first argument is not `i64 vmctx`.\n",
+                "See docs/testing.md for more info on using heap annotations."
+            )
+            .to_string());
+        }
+
+        Ok(())
    }
 }
+
+pub(crate) type HeapMemory = Vec<u8>;
--- a/cranelift/filetests/src/test_interpret.rs
+++ b/cranelift/filetests/src/test_interpret.rs
@@ -3,10 +3,13 @@
 //! The `interpret` test command interprets each function on the host machine
 //! using [RunCommand](cranelift_reader::RunCommand)s.

+use crate::runtest_environment::RuntestEnvironment;
 use crate::subtest::{Context, SubTest};
+use cranelift_codegen::data_value::DataValue;
+use cranelift_codegen::ir::types::I64;
 use cranelift_codegen::{self, ir};
 use cranelift_interpreter::environment::FunctionStore;
-use cranelift_interpreter::interpreter::{Interpreter, InterpreterState};
+use cranelift_interpreter::interpreter::{HeapInit, Interpreter, InterpreterState};
 use cranelift_interpreter::step::ControlFlow;
 use cranelift_reader::{parse_run_command, TestCommand};
 use log::trace;
@@ -36,6 +39,7 @@ impl SubTest for TestInterpret {
    }

    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
+        let test_env = RuntestEnvironment::parse(&context.details.comments[..])?;
        for comment in context.details.comments.iter() {
            if let Some(command) = parse_run_command(comment.text, &func.signature)? {
                trace!("Parsed run command: {}", command);
@@ -44,11 +48,21 @@ impl SubTest for TestInterpret {
                env.add(func.name.to_string(), &func);

                command
-                    .run(|func_name, args| {
+                    .run(|func_name, run_args| {
+                        test_env.validate_signature(&func)?;
+
+                        let mut state = InterpreterState::default().with_function_store(env);
+
+                        let mut args = Vec::with_capacity(run_args.len());
+                        if test_env.is_active() {
+                            let vmctx_addr = register_heaps(&mut state, &test_env);
+                            args.push(vmctx_addr);
+                        }
+                        args.extend_from_slice(run_args);
+
                        // Because we have stored function names with a leading %, we need to re-add it.
                        let func_name = &format!("%{}", func_name);
-                        let state = InterpreterState::default().with_function_store(env);
-                        match Interpreter::new(state).call_by_name(func_name, args) {
+                        match Interpreter::new(state).call_by_name(func_name, &args) {
                            Ok(ControlFlow::Return(results)) => Ok(results.to_vec()),
                            Ok(_) => {
                                panic!("Unexpected returned control flow--this is likely a bug.")
@@ -62,3 +76,34 @@ impl SubTest for TestInterpret {
        Ok(())
    }
 }
+
+/// Build a VMContext struct with the layout described in docs/testing.md.
+pub fn register_heaps<'a>(
+    state: &mut InterpreterState<'a>,
+    test_env: &RuntestEnvironment,
+) -> DataValue {
+    let mem = test_env.allocate_memory();
+    let vmctx_struct = mem
+        .into_iter()
+        // This memory layout (a contiguous list of base + bound ptrs)
+        // is enforced by the RuntestEnvironment when parsing the heap
+        // directives. So we are safe to replicate that here.
+        .flat_map(|mem| {
+            let heap_len = mem.len() as u64;
+            let heap = state.register_heap(HeapInit::FromBacking(mem));
+            [
+                state.get_heap_address(I64, heap, 0).unwrap(),
+                state.get_heap_address(I64, heap, heap_len).unwrap(),
+            ]
+        })
+        .map(|addr| {
+            let mut mem = [0u8; 8];
+            addr.write_to_slice(&mut mem[..]);
+            mem
+        })
+        .flatten()
+        .collect();
+
+    let vmctx_heap = state.register_heap(HeapInit::FromBacking(vmctx_struct));
+    state.get_heap_address(I64, vmctx_heap, 0).unwrap()
+}
--- a/cranelift/filetests/src/test_run.rs
+++ b/cranelift/filetests/src/test_run.rs
@@ -3,10 +3,11 @@
 //! The `run` test command compiles each function on the host machine and executes it

 use crate::function_runner::SingleFunctionCompiler;
-use crate::runtest_environment::RuntestEnvironment;
+use crate::runtest_environment::{HeapMemory, RuntestEnvironment};
 use crate::subtest::{Context, SubTest};
+use cranelift_codegen::data_value::DataValue;
 use cranelift_codegen::ir;
-use cranelift_codegen::ir::ArgumentPurpose;
+use cranelift_codegen::ir::Type;
 use cranelift_reader::parse_run_command;
 use cranelift_reader::TestCommand;
 use log::trace;
@@ -64,25 +65,13 @@ impl SubTest for TestRun {
                let compiled_fn = compiler.compile(func.clone().into_owned())?;
                command
                    .run(|_, run_args| {
-                        let runtime_struct = test_env.runtime_struct();
-
-                        let first_arg_is_vmctx = func
-                            .signature
-                            .params
-                            .first()
-                            .map(|p| p.purpose == ArgumentPurpose::VMContext)
-                            .unwrap_or(false);
-
-                        if !first_arg_is_vmctx && test_env.is_active() {
-                            return Err(concat!(
-                                "This test requests a heap, but the first argument is not `i64 vmctx`.\n",
-                                "See docs/testing.md for more info on using heap annotations."
-                            ).to_string());
-                        }
+                        test_env.validate_signature(&func)?;
+                        let (_heaps, _ctx_struct, vmctx_ptr) =
+                            build_vmctx_struct(&test_env, context.isa.unwrap().pointer_type());

                        let mut args = Vec::with_capacity(run_args.len());
                        if test_env.is_active() {
-                            args.push(runtime_struct.pointer(context.isa.unwrap().pointer_type()));
+                            args.push(vmctx_ptr);
                        }
                        args.extend_from_slice(run_args);

@@ -94,3 +83,24 @@ impl SubTest for TestRun {
        Ok(())
    }
 }
+
+/// Build a VMContext struct with the layout described in docs/testing.md.
+pub fn build_vmctx_struct(
+    test_env: &RuntestEnvironment,
+    ptr_ty: Type,
+) -> (Vec<HeapMemory>, Vec<u64>, DataValue) {
+    let heaps = test_env.allocate_memory();
+
+    let context_struct: Vec<u64> = heaps
+        .iter()
+        .flat_map(|heap| [heap.as_ptr(), heap.as_ptr().wrapping_add(heap.len())])
+        .map(|p| p as usize as u64)
+        .collect();
+
+    let ptr = context_struct.as_ptr() as usize as i128;
+    let ptr_dv =
+        DataValue::from_integer(ptr, ptr_ty).expect("Failed to cast pointer to native target size");
+
+    // Return all these to make sure we don't deallocate the heaps too early
+    (heaps, context_struct, ptr_dv)
+}