Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cranelift: Add heap support to the interpreter #3302

Merged
merged 7 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 76 additions & 40 deletions cranelift/filetests/filetests/runtests/heap.clif
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
80 changes: 24 additions & 56 deletions cranelift/filetests/src/runtest_environment.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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();
.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();
/// 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);

Self {
heaps,
context_struct,
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());
}
}

/// 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")
Ok(())
}
}

pub(crate) type HeapMemory = Vec<u8>;
53 changes: 49 additions & 4 deletions cranelift/filetests/src/test_interpret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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.")
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this right, it is building a vmctx out of a linear layout of (base, length) tuples, but don't the heap directives at least for the runtests allow us to put the base and length pointers at arbitrary vmctx offsets? Do we need to consider that here? (If not, a comment would help!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had that design at the beginning but eventually settled on having the annotations in the ; heap directive but forcing them to a linear layout of tuples. This is enforced when parsing the heap directives.

I've added a comment clarifying this.

.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()
}
Loading