From c06ac1cb15711dc74db3565d3b31282aab6ca83f Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 5 Dec 2022 16:41:23 -0800 Subject: [PATCH] cranelift: Remove the `ir::Heap` entity from CLIF --- cranelift/codegen/meta/src/shared/settings.rs | 15 - cranelift/codegen/src/ir/dfg.rs | 7 +- cranelift/codegen/src/ir/entities.rs | 63 ---- cranelift/codegen/src/ir/function.rs | 14 +- cranelift/codegen/src/ir/heap.rs | 67 ----- cranelift/codegen/src/ir/immediates.rs | 13 - cranelift/codegen/src/ir/mod.rs | 6 +- cranelift/codegen/src/opts.rs | 6 +- cranelift/codegen/src/verifier/mod.rs | 44 --- cranelift/codegen/src/write.rs | 7 - cranelift/docs/ir.md | 144 +--------- cranelift/docs/testing.md | 76 ----- cranelift/filetests/src/lib.rs | 1 - .../filetests/src/runtest_environment.rs | 50 +--- cranelift/filetests/src/test_interpret.rs | 56 +--- cranelift/filetests/src/test_run.rs | 26 +- cranelift/frontend/src/frontend.rs | 13 +- cranelift/fuzzgen/src/function_generator.rs | 4 +- cranelift/interpreter/src/interpreter.rs | 184 +----------- cranelift/interpreter/src/state.rs | 18 +- cranelift/reader/src/heap_command.rs | 71 ----- cranelift/reader/src/lexer.rs | 2 - cranelift/reader/src/lib.rs | 4 +- cranelift/reader/src/parser.rs | 271 +----------------- cranelift/reader/src/sourcemap.rs | 21 +- 25 files changed, 39 insertions(+), 1144 deletions(-) delete mode 100644 cranelift/codegen/src/ir/heap.rs delete mode 100644 cranelift/reader/src/heap_command.rs diff --git a/cranelift/codegen/meta/src/shared/settings.rs b/cranelift/codegen/meta/src/shared/settings.rs index b9fbef55c9e2..e51cd5a5ac56 100644 --- a/cranelift/codegen/meta/src/shared/settings.rs +++ b/cranelift/codegen/meta/src/shared/settings.rs @@ -139,21 +139,6 @@ pub(crate) fn define() -> SettingGroup { false, ); - settings.add_bool( - "use_pinned_reg_as_heap_base", - "Use the pinned register as the heap base.", - r#" - Enabling this requires the enable_pinned_reg setting to be set to true. It enables a custom - legalization of the `heap_addr` instruction so it will use the pinned register as the heap - base, instead of fetching it from a global value. - - Warning! Enabling this means that the pinned register *must* be maintained to contain the - heap base address at all times, during the lifetime of a function. Using the pinned - register for other purposes when this is set is very likely to cause crashes. - "#, - false, - ); - settings.add_bool( "enable_simd", "Enable the use of SIMD instructions.", diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs index 48eb4fa910e3..9d551ccb17f0 100644 --- a/cranelift/codegen/src/ir/dfg.rs +++ b/cranelift/codegen/src/ir/dfg.rs @@ -4,11 +4,10 @@ use crate::entity::{self, PrimaryMap, SecondaryMap}; use crate::ir; use crate::ir::builder::ReplaceBuilder; use crate::ir::dynamic_type::{DynamicTypeData, DynamicTypes}; -use crate::ir::immediates::HeapImmData; use crate::ir::instructions::{BranchInfo, CallInfo, InstructionData}; use crate::ir::{types, ConstantData, ConstantPool, Immediate}; use crate::ir::{ - Block, DynamicType, FuncRef, HeapImm, Inst, SigRef, Signature, Type, Value, + Block, DynamicType, FuncRef, Inst, SigRef, Signature, Type, Value, ValueLabelAssignments, ValueList, ValueListPool, }; use crate::ir::{ExtFuncData, RelSourceLoc}; @@ -84,9 +83,6 @@ pub struct DataFlowGraph { /// Stores large immediates that otherwise will not fit on InstructionData pub immediates: PrimaryMap, - - /// Out-of-line heap access immediates that don't fit in `InstructionData`. - pub heap_imms: PrimaryMap, } impl DataFlowGraph { @@ -105,7 +101,6 @@ impl DataFlowGraph { values_labels: None, constants: ConstantPool::new(), immediates: PrimaryMap::new(), - heap_imms: PrimaryMap::new(), } } diff --git a/cranelift/codegen/src/ir/entities.rs b/cranelift/codegen/src/ir/entities.rs index dc63d0ec37d6..8573b4cd7d9d 100644 --- a/cranelift/codegen/src/ir/entities.rs +++ b/cranelift/codegen/src/ir/entities.rs @@ -368,60 +368,6 @@ impl SigRef { } } -/// An opaque reference to a [heap](https://en.wikipedia.org/wiki/Memory_management#DYNAMIC). -/// -/// Heaps are used to access dynamically allocated memory through -/// [`heap_addr`](super::InstBuilder::heap_addr). -/// -/// To create a heap, use [`FunctionBuilder::create_heap`](https://docs.rs/cranelift-frontend/*/cranelift_frontend/struct.FunctionBuilder.html#method.create_heap). -/// -/// While the order is stable, it is arbitrary. -#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] -#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] -pub struct Heap(u32); -entity_impl!(Heap, "heap"); - -impl Heap { - /// Create a new heap reference from its number. - /// - /// This method is for use by the parser. - pub fn with_number(n: u32) -> Option { - if n < u32::MAX { - Some(Self(n)) - } else { - None - } - } -} - -/// An opaque reference to some out-of-line immediates for `heap_{load,store}` -/// instructions. -/// -/// These immediates are too large to store in -/// [`InstructionData`](super::instructions::InstructionData) and therefore must -/// be tracked separately in -/// [`DataFlowGraph::heap_imms`](super::dfg::DataFlowGraph). `HeapImm` provides -/// a way to reference values stored there. -/// -/// While the order is stable, it is arbitrary. -#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] -#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] -pub struct HeapImm(u32); -entity_impl!(HeapImm, "heap_imm"); - -impl HeapImm { - /// Create a new `HeapImm` reference from its number. - /// - /// This method is for use by the parser. - pub fn with_number(n: u32) -> Option { - if n < u32::MAX { - Some(Self(n)) - } else { - None - } - } -} - /// An opaque reference to a [WebAssembly /// table](https://developer.mozilla.org/en-US/docs/WebAssembly/Understanding_the_text_format#WebAssembly_tables). /// @@ -477,8 +423,6 @@ pub enum AnyEntity { FuncRef(FuncRef), /// A function call signature. SigRef(SigRef), - /// A heap. - Heap(Heap), /// A table. Table(Table), /// A function's stack limit @@ -500,7 +444,6 @@ impl fmt::Display for AnyEntity { Self::Constant(r) => r.fmt(f), Self::FuncRef(r) => r.fmt(f), Self::SigRef(r) => r.fmt(f), - Self::Heap(r) => r.fmt(f), Self::Table(r) => r.fmt(f), Self::StackLimit => write!(f, "stack_limit"), } @@ -579,12 +522,6 @@ impl From for AnyEntity { } } -impl From for AnyEntity { - fn from(r: Heap) -> Self { - Self::Heap(r) - } -} - impl From for AnyEntity { fn from(r: Table) -> Self { Self::Table(r) diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index 06f0bcd7567b..39a0ab4dd7cb 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -8,8 +8,8 @@ use crate::ir; use crate::ir::JumpTables; use crate::ir::{ instructions::BranchInfo, Block, DynamicStackSlot, DynamicStackSlotData, DynamicType, - ExtFuncData, FuncRef, GlobalValue, GlobalValueData, Heap, HeapData, Inst, InstructionData, - JumpTable, JumpTableData, Opcode, SigRef, StackSlot, StackSlotData, Table, TableData, Type, + ExtFuncData, FuncRef, GlobalValue, GlobalValueData, Inst, InstructionData, JumpTable, + JumpTableData, Opcode, SigRef, StackSlot, StackSlotData, Table, TableData, Type, }; use crate::ir::{DataFlowGraph, Layout, Signature}; use crate::ir::{DynamicStackSlots, SourceLocs, StackSlots}; @@ -170,9 +170,6 @@ pub struct FunctionStencil { /// Global values referenced. pub global_values: PrimaryMap, - /// Heaps referenced. - pub heaps: PrimaryMap, - /// Tables referenced. pub tables: PrimaryMap, @@ -205,7 +202,6 @@ impl FunctionStencil { self.sized_stack_slots.clear(); self.dynamic_stack_slots.clear(); self.global_values.clear(); - self.heaps.clear(); self.tables.clear(); self.jump_tables.clear(); self.dfg.clear(); @@ -261,11 +257,6 @@ impl FunctionStencil { .concrete() } - /// Declares a heap accessible to the function. - pub fn create_heap(&mut self, data: HeapData) -> Heap { - self.heaps.push(data) - } - /// Declares a table accessible to the function. pub fn create_table(&mut self, data: TableData) -> Table { self.tables.push(data) @@ -447,7 +438,6 @@ impl Function { sized_stack_slots: StackSlots::new(), dynamic_stack_slots: DynamicStackSlots::new(), global_values: PrimaryMap::new(), - heaps: PrimaryMap::new(), tables: PrimaryMap::new(), jump_tables: PrimaryMap::new(), dfg: DataFlowGraph::new(), diff --git a/cranelift/codegen/src/ir/heap.rs b/cranelift/codegen/src/ir/heap.rs deleted file mode 100644 index 7d62915af54d..000000000000 --- a/cranelift/codegen/src/ir/heap.rs +++ /dev/null @@ -1,67 +0,0 @@ -//! Heaps. - -use crate::ir::immediates::Uimm64; -use crate::ir::{GlobalValue, Type}; -use core::fmt; - -#[cfg(feature = "enable-serde")] -use serde::{Deserialize, Serialize}; - -/// Information about a heap declaration. -#[derive(Clone, PartialEq, Hash)] -#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] -pub struct HeapData { - /// The address of the start of the heap's storage. - pub base: GlobalValue, - - /// Guaranteed minimum heap size in bytes. Heap accesses before `min_size` don't need bounds - /// checking. - pub min_size: Uimm64, - - /// Size in bytes of the offset-guard pages following the heap. - pub offset_guard_size: Uimm64, - - /// Heap style, with additional style-specific info. - pub style: HeapStyle, - - /// The index type for the heap. - pub index_type: Type, -} - -/// Style of heap including style-specific information. -#[derive(Clone, PartialEq, Hash)] -#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] -pub enum HeapStyle { - /// A dynamic heap can be relocated to a different base address when it is grown. - Dynamic { - /// Global value providing the current bound of the heap in bytes. - bound_gv: GlobalValue, - }, - - /// A static heap has a fixed base address and a number of not-yet-allocated pages before the - /// offset-guard pages. - Static { - /// Heap bound in bytes. The offset-guard pages are allocated after the bound. - bound: Uimm64, - }, -} - -impl fmt::Display for HeapData { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str(match self.style { - HeapStyle::Dynamic { .. } => "dynamic", - HeapStyle::Static { .. } => "static", - })?; - - write!(f, " {}, min {}", self.base, self.min_size)?; - match self.style { - HeapStyle::Dynamic { bound_gv } => write!(f, ", bound {}", bound_gv)?, - HeapStyle::Static { bound } => write!(f, ", bound {}", bound)?, - } - write!( - f, - ", offset_guard {}, index_type {}", - self.offset_guard_size, self.index_type - ) - } -} diff --git a/cranelift/codegen/src/ir/immediates.rs b/cranelift/codegen/src/ir/immediates.rs index 54f737b19d1b..3b3f7032353b 100644 --- a/cranelift/codegen/src/ir/immediates.rs +++ b/cranelift/codegen/src/ir/immediates.rs @@ -4,7 +4,6 @@ //! Each type here should have a corresponding definition in the //! `cranelift-codegen/meta/src/shared/immediates` crate in the meta language. -use crate::ir; use alloc::vec::Vec; use core::cmp::Ordering; use core::convert::TryFrom; @@ -1178,18 +1177,6 @@ impl Not for Ieee64 { } } -/// Out-of-line heap access immediates. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] -pub struct HeapImmData { - /// The memory flags for the heap access. - pub flags: ir::MemFlags, - /// The heap being accessed. - pub heap: ir::Heap, - /// The static offset added to the heap access's index. - pub offset: Uimm32, -} - #[cfg(test)] mod tests { use super::*; diff --git a/cranelift/codegen/src/ir/mod.rs b/cranelift/codegen/src/ir/mod.rs index 23f952738e35..73496d06aed8 100644 --- a/cranelift/codegen/src/ir/mod.rs +++ b/cranelift/codegen/src/ir/mod.rs @@ -11,7 +11,6 @@ mod extfunc; mod extname; pub mod function; mod globalvalue; -mod heap; pub mod immediates; pub mod instructions; pub mod jumptable; @@ -37,8 +36,8 @@ pub use crate::ir::constant::{ConstantData, ConstantPool}; pub use crate::ir::dfg::{DataFlowGraph, ValueDef}; pub use crate::ir::dynamic_type::{dynamic_to_fixed, DynamicTypeData, DynamicTypes}; pub use crate::ir::entities::{ - Block, Constant, DynamicStackSlot, DynamicType, FuncRef, GlobalValue, Heap, HeapImm, Immediate, - Inst, JumpTable, SigRef, StackSlot, Table, UserExternalNameRef, Value, + Block, Constant, DynamicStackSlot, DynamicType, FuncRef, GlobalValue, Immediate, Inst, + JumpTable, SigRef, StackSlot, Table, UserExternalNameRef, Value, }; pub use crate::ir::extfunc::{ AbiParam, ArgumentExtension, ArgumentPurpose, ExtFuncData, Signature, @@ -46,7 +45,6 @@ pub use crate::ir::extfunc::{ pub use crate::ir::extname::{ExternalName, UserExternalName, UserFuncName}; pub use crate::ir::function::{DisplayFunctionAnnotations, Function}; pub use crate::ir::globalvalue::GlobalValueData; -pub use crate::ir::heap::{HeapData, HeapStyle}; pub use crate::ir::instructions::{ InstructionData, InstructionImms, Opcode, ValueList, ValueListPool, VariableArgs, }; diff --git a/cranelift/codegen/src/opts.rs b/cranelift/codegen/src/opts.rs index 7e77370e29d6..5c6e9fddc384 100644 --- a/cranelift/codegen/src/opts.rs +++ b/cranelift/codegen/src/opts.rs @@ -8,9 +8,9 @@ pub use crate::ir::condcodes::{FloatCC, IntCC}; pub use crate::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64, Uimm8}; pub use crate::ir::types::*; pub use crate::ir::{ - dynamic_to_fixed, AtomicRmwOp, Block, Constant, DynamicStackSlot, FuncRef, GlobalValue, Heap, - HeapImm, Immediate, InstructionImms, JumpTable, MemFlags, Opcode, StackSlot, Table, TrapCode, - Type, Value, + dynamic_to_fixed, AtomicRmwOp, Block, Constant, DynamicStackSlot, FuncRef, GlobalValue, + Immediate, InstructionImms, JumpTable, MemFlags, Opcode, StackSlot, Table, TrapCode, Type, + Value, }; use crate::isle_common_prelude_methods; use crate::machinst::isle::*; diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 63396c3f5f9d..18736c3af761 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -406,49 +406,6 @@ impl<'a> Verifier<'a> { Ok(()) } - fn verify_heaps(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { - if let Some(isa) = self.isa { - for (heap, heap_data) in &self.func.heaps { - let base = heap_data.base; - if !self.func.global_values.is_valid(base) { - return errors.nonfatal((heap, format!("invalid base global value {}", base))); - } - - let pointer_type = isa.pointer_type(); - let base_type = self.func.global_values[base].global_type(isa); - if base_type != pointer_type { - errors.report(( - heap, - format!( - "heap base has type {}, which is not the pointer type {}", - base_type, pointer_type - ), - )); - } - - if let ir::HeapStyle::Dynamic { bound_gv, .. } = heap_data.style { - if !self.func.global_values.is_valid(bound_gv) { - return errors - .nonfatal((heap, format!("invalid bound global value {}", bound_gv))); - } - - let bound_type = self.func.global_values[bound_gv].global_type(isa); - if pointer_type != bound_type { - errors.report(( - heap, - format!( - "heap pointer type {} differs from the type of its bound, {}", - pointer_type, bound_type - ), - )); - } - } - } - } - - Ok(()) - } - fn verify_tables(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { if let Some(isa) = self.isa { for (table, table_data) in &self.func.tables { @@ -1734,7 +1691,6 @@ impl<'a> Verifier<'a> { pub fn run(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> { self.verify_global_values(errors)?; - self.verify_heaps(errors)?; self.verify_tables(errors)?; self.verify_jump_tables(errors)?; self.typecheck_entry_block_params(errors)?; diff --git a/cranelift/codegen/src/write.rs b/cranelift/codegen/src/write.rs index 87fa5a2b63b6..8934c8a7ebde 100644 --- a/cranelift/codegen/src/write.rs +++ b/cranelift/codegen/src/write.rs @@ -56,13 +56,6 @@ pub trait FuncWriter { self.write_entity_definition(w, func, gv.into(), gv_data)?; } - for (heap, heap_data) in &func.heaps { - if !heap_data.index_type.is_invalid() { - any = true; - self.write_entity_definition(w, func, heap.into(), heap_data)?; - } - } - for (table, table_data) in &func.tables { if !table_data.index_type.is_invalid() { any = true; diff --git a/cranelift/docs/ir.md b/cranelift/docs/ir.md index 66d787295a94..6296dd49d577 100644 --- a/cranelift/docs/ir.md +++ b/cranelift/docs/ir.md @@ -577,148 +577,6 @@ GV = [colocated] symbol Name :arg Name: External name. :result GV: Global value. -### Heaps - -Code compiled from WebAssembly or asm.js runs in a sandbox where it can't access -all process memory. Instead, it is given a small set of memory areas to work -in, and all accesses are bounds checked. Cranelift models this through the -concept of *heaps*. - -A heap is declared in the function preamble and can be accessed with the -`heap_addr` instruction that [traps] on out-of-bounds accesses or -returns a pointer that is guaranteed to trap. Heap addresses can be smaller than -the native pointer size, for example unsigned `i32` offsets on a 64-bit -architecture. - -![Heap address space layout](./heap.svg) - -A heap appears as three consecutive ranges of address space: - -1. The *mapped pages* are the [accessible] memory range in the heap. A - heap may have a minimum guaranteed size which means that some mapped pages - are always present. -2. The *unmapped pages* is a possibly empty range of address space that may be - mapped in the future when the heap is grown. They are [addressable] but - not [accessible]. -3. The *offset-guard pages* is a range of address space that is guaranteed to - always cause a trap when accessed. It is used to optimize bounds checking for - heap accesses with a shared base pointer. They are [addressable] but - not [accessible]. - -The *heap bound* is the total size of the mapped and unmapped pages. This is -the bound that `heap_addr` checks against. Memory accesses inside the -heap bounds can trap if they hit an unmapped page (which is not -[accessible]). - -Two styles of heaps are supported, *static* and *dynamic*. They behave -differently when resized. - -#### Static heaps - -A *static heap* starts out with all the address space it will ever need, so it -never moves to a different address. At the base address is a number of mapped -pages corresponding to the heap's current size. Then follows a number of -unmapped pages where the heap can grow up to its maximum size. After the -unmapped pages follow the offset-guard pages which are also guaranteed to -generate a trap when accessed. - -H = static Base, min MinBytes, bound BoundBytes, offset_guard OffsetGuardBytes - Declare a static heap in the preamble. - - :arg Base: Global value holding the heap's base address. - :arg MinBytes: Guaranteed minimum heap size in bytes. Accesses below this - size will never trap. - :arg BoundBytes: Fixed heap bound in bytes. This defines the amount of - address space reserved for the heap, not including the offset-guard - pages. - :arg OffsetGuardBytes: Size of the offset-guard pages in bytes. - -#### Dynamic heaps - -A *dynamic heap* can be relocated to a different base address when it is -resized, and its bound can move dynamically. The offset-guard pages move when -the heap is resized. The bound of a dynamic heap is stored in a global value. - -H = dynamic Base, min MinBytes, bound BoundGV, offset_guard OffsetGuardBytes - Declare a dynamic heap in the preamble. - - :arg Base: Global value holding the heap's base address. - :arg MinBytes: Guaranteed minimum heap size in bytes. Accesses below this - size will never trap. - :arg BoundGV: Global value containing the current heap bound in bytes. - :arg OffsetGuardBytes: Size of the offset-guard pages in bytes. - -#### Heap examples - -Some Wasm VMs prefer to use fixed heaps with a 4 GB bound and 2 GB of -offset-guard pages when running WebAssembly code on 64-bit CPUs. The combination -of a 4 GB fixed bound and 1-byte bounds checks means that no code needs to be -generated for bounds checks at all: - -``` -test verifier - -function %add_members(i32, i64 vmctx) -> f32 { - gv0 = vmctx - gv1 = load.i64 notrap aligned gv0+64 - heap0 = static gv1, min 0x1000, bound 0x1_0000_0000, offset_guard 0x8000_0000 - -block0(v0: i32, v5: i64): - v1 = heap_addr.i64 heap0, v0, 1 - v2 = load.f32 v1+16 - v3 = load.f32 v1+20 - v4 = fadd v2, v3 - return v4 -} -``` - -A static heap can also be used for 32-bit code when the WebAssembly module -declares a small upper bound on its memory. A 1 MB static bound with a single 4 -KB offset-guard page still has opportunities for sharing bounds checking code: - -``` -test verifier - -function %add_members(i32, i32 vmctx) -> f32 { - gv0 = vmctx - gv1 = load.i32 notrap aligned gv0+64 - heap0 = static gv1, min 0x1000, bound 0x10_0000, offset_guard 0x1000 - -block0(v0: i32, v5: i32): - v1 = heap_addr.i32 heap0, v0, 1 - v2 = load.f32 v1+16 - v3 = load.f32 v1+20 - v4 = fadd v2, v3 - return v4 -} -``` - -If the upper bound on the heap size is too large, a dynamic heap is required -instead. - -Finally, a runtime environment that simply allocates a heap with -`malloc()` may not have any offset-guard pages at all. In that case, -full bounds checking is required for each access: - -``` -test verifier - -function %add_members(i32, i64 vmctx) -> f32 { - gv0 = vmctx - gv1 = load.i64 notrap aligned gv0+64 - gv2 = load.i32 notrap aligned gv0+72 - heap0 = dynamic gv1, min 0x1000, bound gv2, offset_guard 0 - -block0(v0: i32, v6: i64): - v1 = heap_addr.i64 heap0, v0, 20 - v2 = load.f32 v1+16 - v3 = heap_addr.i64 heap0, v0, 24 - v4 = load.f32 v3+20 - v5 = fadd v2, v4 - return v5 -} -``` - ### Tables Code compiled from WebAssembly often needs access to objects outside of its @@ -746,7 +604,7 @@ T = dynamic Base, min MinElements, bound BoundGV, element_size ElementSize :arg Base: Global value holding the table's base address. :arg MinElements: Guaranteed minimum table size in elements. - :arg BoundGV: Global value containing the current heap bound in elements. + :arg BoundGV: Global value containing the current table bound in elements. :arg ElementSize: Size of each element. ### Constant materialization diff --git a/cranelift/docs/testing.md b/cranelift/docs/testing.md index 4dc60548084c..ce3cd6361d74 100644 --- a/cranelift/docs/testing.md +++ b/cranelift/docs/testing.md @@ -374,79 +374,3 @@ pointers are always 8 bytes, and laid out sequentially in memory. Even for 32 bi Currently, we only support requesting heaps, however this is a generic mechanism that should be able to introduce any sort of environment support that we may need later. (e.g. tables, global values, external functions) - -##### `heap` directive - -The `heap` directive allows a test to request a heap to be allocated and passed to the test via the environment struct. - - -A sample heap annotation is the following: -``` -; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8 -``` - -This indicates the following: -* `static`: We have requested a non-resizable and non-movable static heap. -* `size=0x1000`: It has to have a size of 4096 bytes. -* `ptr=vmctx+0`: The pointer to the address to the start of this heap is placed at offset 0 in the `vmctx` struct -* `bound=vmctx+8`: The pointer to the address to the end of this heap is placed at offset 8 in the `vmctx` struct - -The `ptr` and `bound` arguments make explicit the placement of the pointers to the start and end of the heap memory in -the environment struct. `vmctx+0` means that at offset 0 of the environment struct there will be the pointer to the start -similarly, at offset 8 the pointer to the end. - - -You can combine multiple heap annotations, in which case, their pointers are laid out sequentially in memory in -the order that the annotations appear in the source file. - -``` -; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8 -; heap: dynamic, size=0x1000, ptr=vmctx+16, bound=vmctx+24 -``` - -An invalid or unexpected offset will raise an error when the test is run. - -See the diagram below, on how the `vmctx` struct ends up if with multiple heaps: - -``` - ┌─────────────────────┐ vmctx+0 - │heap0: start address │ - ├─────────────────────┤ vmctx+8 - │heap0: end address │ - ├─────────────────────┤ vmctx+16 - │heap1: start address │ - ├─────────────────────┤ vmctx+24 - │heap1: end address │ - ├─────────────────────┤ vmctx+32 - │etc... │ - └─────────────────────┘ -``` - -With this setup, you can now use the global values to load heaps, and load / store to them. - -Example: - -``` -function %heap_load_store(i64 vmctx, i64, i32) -> i32 { - gv0 = vmctx - gv1 = load.i64 notrap aligned gv0+0 - gv2 = load.i64 notrap aligned gv0+8 - heap0 = dynamic gv1, bound gv2, offset_guard 0, index_type i64 - -block0(v0: i64, v1: i64, v2: i32): - v3 = heap_addr.i64 heap0, v1, 4 - store.i32 v2, v3 - v4 = load.i32 v3 - return v4 -} -; heap: static, size=0x1000, ptr=vmctx+0, bound=vmctx+8 -; run: %heap_load_store(0, 1) == 1 -``` - - -### `test interpret` - -Test the CLIF interpreter - -This test supports the same commands as `test run`, but runs the code in the cranelift -interpreter instead of the host machine. diff --git a/cranelift/filetests/src/lib.rs b/cranelift/filetests/src/lib.rs index 8bff0eb29d91..ed5ef7d26ce9 100644 --- a/cranelift/filetests/src/lib.rs +++ b/cranelift/filetests/src/lib.rs @@ -34,7 +34,6 @@ pub mod function_runner; mod match_directive; mod runner; mod runone; -mod runtest_environment; mod subtest; mod test_alias_analysis; diff --git a/cranelift/filetests/src/runtest_environment.rs b/cranelift/filetests/src/runtest_environment.rs index 5a7fe1564492..8a32e8161809 100644 --- a/cranelift/filetests/src/runtest_environment.rs +++ b/cranelift/filetests/src/runtest_environment.rs @@ -1,60 +1,18 @@ use anyhow::anyhow; use cranelift_codegen::ir::{ArgumentPurpose, Function}; -use cranelift_reader::parse_heap_command; -use cranelift_reader::{Comment, HeapCommand}; +use cranelift_reader::Comment; /// Stores info about the expected environment for a test function. #[derive(Debug, Clone)] -pub struct RuntestEnvironment { - pub heaps: Vec, -} +pub struct RuntestEnvironment {} impl RuntestEnvironment { /// Parse the environment from a set of comments pub fn parse(comments: &[Comment]) -> anyhow::Result { - let mut env = RuntestEnvironment { heaps: Vec::new() }; - - for comment in comments.iter() { - if let Some(heap_command) = parse_heap_command(comment.text)? { - let heap_index = env.heaps.len() as u64; - let expected_ptr = heap_index * 16; - if Some(expected_ptr) != heap_command.ptr_offset.map(|p| p.into()) { - return Err(anyhow!( - "Invalid ptr offset, expected vmctx+{}", - expected_ptr - )); - } - - let expected_bound = (heap_index * 16) + 8; - if Some(expected_bound) != heap_command.bound_offset.map(|p| p.into()) { - return Err(anyhow!( - "Invalid bound offset, expected vmctx+{}", - expected_bound - )); - } - - env.heaps.push(heap_command); - }; - } - + let mut env = RuntestEnvironment {}; Ok(env) } - pub fn is_active(&self) -> bool { - !self.heaps.is_empty() - } - - /// Allocates memory for heaps - pub fn allocate_memory(&self) -> Vec { - self.heaps - .iter() - .map(|cmd| { - let size: u64 = cmd.size.into(); - vec![0u8; size as usize] - }) - .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> { @@ -76,5 +34,3 @@ impl RuntestEnvironment { Ok(()) } } - -pub(crate) type HeapMemory = Vec; diff --git a/cranelift/filetests/src/test_interpret.rs b/cranelift/filetests/src/test_interpret.rs index 14f191cfa9d1..dcfd33f331aa 100644 --- a/cranelift/filetests/src/test_interpret.rs +++ b/cranelift/filetests/src/test_interpret.rs @@ -4,17 +4,14 @@ //! using [RunCommand](cranelift_reader::RunCommand)s. use crate::runone::FileUpdate; -use crate::runtest_environment::RuntestEnvironment; use crate::subtest::SubTest; -use anyhow::{anyhow, Context}; -use cranelift_codegen::data_value::DataValue; -use cranelift_codegen::ir::types::I64; +use anyhow::Context; use cranelift_codegen::ir::Function; use cranelift_codegen::isa::TargetIsa; use cranelift_codegen::settings::Flags; use cranelift_codegen::{self, ir}; use cranelift_interpreter::environment::FunctionStore; -use cranelift_interpreter::interpreter::{HeapInit, Interpreter, InterpreterState}; +use cranelift_interpreter::interpreter::{Interpreter, InterpreterState}; use cranelift_interpreter::step::ControlFlow; use cranelift_reader::{parse_run_command, Details, TestCommand, TestFile}; use log::{info, trace}; @@ -62,10 +59,7 @@ impl SubTest for TestInterpret { for (func, details) in &testfile.functions { info!("Test: {}({}) interpreter", self.name(), func.name); - let test_env = RuntestEnvironment::parse(&details.comments[..])?; - test_env.validate_signature(&func).map_err(|s| anyhow!(s))?; - - run_test(&test_env, &func_store, func, details).context(self.name())?; + run_test(&func_store, func, details).context(self.name())?; } Ok(()) @@ -80,12 +74,7 @@ impl SubTest for TestInterpret { } } -fn run_test( - test_env: &RuntestEnvironment, - func_store: &FunctionStore, - func: &Function, - details: &Details, -) -> anyhow::Result<()> { +fn run_test(func_store: &FunctionStore, func: &Function, details: &Details) -> anyhow::Result<()> { for comment in details.comments.iter() { if let Some(command) = parse_run_command(comment.text, &func.signature)? { trace!("Parsed run command: {}", command); @@ -94,14 +83,10 @@ fn run_test( .run(|func_name, run_args| { // Rebuild the interpreter state on every run to ensure that we don't accidentally depend on // some leftover state - let mut state = + let state = InterpreterState::default().with_function_store(func_store.clone()); 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. @@ -119,34 +104,3 @@ fn run_test( } 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() -} diff --git a/cranelift/filetests/src/test_run.rs b/cranelift/filetests/src/test_run.rs index 9af1a9622cb8..febc30155a29 100644 --- a/cranelift/filetests/src/test_run.rs +++ b/cranelift/filetests/src/test_run.rs @@ -4,7 +4,6 @@ use crate::function_runner::{CompiledTestFile, TestFileCompiler}; use crate::runone::FileUpdate; -use crate::runtest_environment::{HeapMemory, RuntestEnvironment}; use crate::subtest::{Context, SubTest}; use anyhow::Context as _; use cranelift_codegen::data_value::DataValue; @@ -117,22 +116,16 @@ fn run_test( func: &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); command .run(|_, run_args| { - test_env.validate_signature(&func)?; - let (_heaps, _ctx_struct, vmctx_ptr) = - build_vmctx_struct(&test_env, context.isa.unwrap().pointer_type()); + let (_ctx_struct, _vmctx_ptr) = + build_vmctx_struct(context.isa.unwrap().pointer_type()); let mut args = Vec::with_capacity(run_args.len()); - if test_env.is_active() { - args.push(vmctx_ptr); - } args.extend_from_slice(run_args); let trampoline = testfile.get_trampoline(func).unwrap(); @@ -215,22 +208,13 @@ impl SubTest for TestRun { } /// Build a VMContext struct with the layout described in docs/testing.md. -pub fn build_vmctx_struct( - test_env: &RuntestEnvironment, - ptr_ty: Type, -) -> (Vec, Vec, DataValue) { - let heaps = test_env.allocate_memory(); - - let context_struct: Vec = heaps - .iter() - .flat_map(|heap| [heap.as_ptr(), heap.as_ptr().wrapping_add(heap.len())]) - .map(|p| p as usize as u64) - .collect(); +pub fn build_vmctx_struct(ptr_ty: Type) -> (Vec, DataValue) { + let context_struct: Vec = Vec::new(); 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) + (context_struct, ptr_dv) } diff --git a/cranelift/frontend/src/frontend.rs b/cranelift/frontend/src/frontend.rs index 89949c679c17..2de86095779b 100644 --- a/cranelift/frontend/src/frontend.rs +++ b/cranelift/frontend/src/frontend.rs @@ -8,10 +8,10 @@ use cranelift_codegen::ir; use cranelift_codegen::ir::condcodes::IntCC; use cranelift_codegen::ir::{ types, AbiParam, Block, DataFlowGraph, DynamicStackSlot, DynamicStackSlotData, ExtFuncData, - ExternalName, FuncRef, Function, GlobalValue, GlobalValueData, Heap, HeapData, Inst, - InstBuilder, InstBuilderBase, InstructionData, JumpTable, JumpTableData, LibCall, MemFlags, - RelSourceLoc, SigRef, Signature, StackSlot, StackSlotData, Type, Value, ValueLabel, - ValueLabelAssignments, ValueLabelStart, + ExternalName, FuncRef, Function, GlobalValue, GlobalValueData, Inst, InstBuilder, + InstBuilderBase, InstructionData, JumpTable, JumpTableData, LibCall, MemFlags, RelSourceLoc, + SigRef, Signature, StackSlot, StackSlotData, Type, Value, ValueLabel, ValueLabelAssignments, + ValueLabelStart, }; use cranelift_codegen::isa::TargetFrontendConfig; use cranelift_codegen::packed_option::PackedOption; @@ -512,11 +512,6 @@ impl<'a> FunctionBuilder<'a> { self.func.create_global_value(data) } - /// Declares a heap accessible to the function. - pub fn create_heap(&mut self, data: HeapData) -> Heap { - self.func.create_heap(data) - } - /// Returns an object with the [`InstBuilder`](cranelift_codegen::ir::InstBuilder) /// trait that allows to conveniently append an instruction to the current `Block` being built. pub fn ins<'short>(&'short mut self) -> FuncInstBuilder<'short, 'a> { diff --git a/cranelift/fuzzgen/src/function_generator.rs b/cranelift/fuzzgen/src/function_generator.rs index 4872b4e8b114..4145ab9b0cc1 100644 --- a/cranelift/fuzzgen/src/function_generator.rs +++ b/cranelift/fuzzgen/src/function_generator.rs @@ -1210,8 +1210,8 @@ where builder: &mut FunctionBuilder, min_size: u32, ) -> Result<(Value, Offset32)> { - // TODO: Currently our only source of addresses is stack_addr, but we should - // add heap_addr, global_value, symbol_value eventually + // TODO: Currently our only source of addresses is stack_addr, but we + // should add global_value, symbol_value eventually let (addr, available_size) = { let (ss, slot_size) = self.stack_slot_with_size(min_size)?; let max_offset = slot_size.saturating_sub(min_size); diff --git a/cranelift/interpreter/src/interpreter.rs b/cranelift/interpreter/src/interpreter.rs index cc58cf29ba99..1a0dff17d0d4 100644 --- a/cranelift/interpreter/src/interpreter.rs +++ b/cranelift/interpreter/src/interpreter.rs @@ -12,13 +12,13 @@ use crate::value::{Value, ValueError}; use cranelift_codegen::data_value::DataValue; use cranelift_codegen::ir::condcodes::{FloatCC, IntCC}; use cranelift_codegen::ir::{ - ArgumentPurpose, Block, FuncRef, Function, GlobalValue, GlobalValueData, Heap, LibCall, - StackSlot, TrapCode, Type, Value as ValueRef, + ArgumentPurpose, Block, FuncRef, Function, GlobalValue, GlobalValueData, LibCall, StackSlot, + TrapCode, Type, Value as ValueRef, }; use log::trace; use smallvec::SmallVec; use std::collections::HashSet; -use std::convert::{TryFrom, TryInto}; +use std::convert::TryFrom; use std::fmt::Debug; use std::iter; use thiserror::Error; @@ -177,21 +177,6 @@ pub enum InterpreterError { FuelExhausted, } -pub type HeapBacking = Vec; - -/// Represents a registered heap with an interpreter. -#[derive(Debug, Clone, Copy, PartialEq)] -pub struct HeapId(u32); - -/// Options for initializing a heap memory region -#[derive(Debug)] -pub enum HeapInit { - /// A zero initialized heap with `size` bytes - Zeroed(usize), - /// Initializes the heap with the backing memory unchanged. - FromBacking(HeapBacking), -} - pub type LibCallValues = SmallVec<[V; 1]>; pub type LibCallHandler = fn(LibCall, LibCallValues) -> Result, TrapCode>; @@ -203,7 +188,6 @@ pub struct InterpreterState<'a> { /// Number of bytes from the bottom of the stack where the current frame's stack space is pub frame_offset: usize, pub stack: Vec, - pub heaps: Vec, pub iflags: HashSet, pub fflags: HashSet, pub pinned_reg: DataValue, @@ -217,7 +201,6 @@ impl Default for InterpreterState<'_> { frame_stack: vec![], frame_offset: 0, stack: Vec::with_capacity(1024), - heaps: Vec::new(), iflags: HashSet::new(), fflags: HashSet::new(), pinned_reg: DataValue::U64(0), @@ -236,57 +219,6 @@ impl<'a> InterpreterState<'a> { self } - /// Registers a static heap and returns a reference to it - /// - /// This heap reference can be used to generate a heap pointer, which - /// can be used inside the interpreter to load / store values into the heap. - /// - /// ```rust - /// # use cranelift_codegen::ir::types::I64; - /// # use cranelift_interpreter::interpreter::{InterpreterState, HeapInit}; - /// let mut state = InterpreterState::default(); - /// let heap0 = state.register_heap(HeapInit::Zeroed(1024)); - /// - /// let backing = Vec::from([10u8; 24]); - /// let heap1 = state.register_heap(HeapInit::FromBacking(backing)); - /// ``` - pub fn register_heap(&mut self, init: HeapInit) -> HeapId { - let heap_id = HeapId(self.heaps.len() as u32); - - self.heaps.push(match init { - HeapInit::Zeroed(size) => iter::repeat(0).take(size).collect(), - HeapInit::FromBacking(backing) => backing, - }); - - heap_id - } - - /// Returns a heap address that can be used inside the interpreter - /// - /// ```rust - /// # use cranelift_codegen::ir::types::I64; - /// # use cranelift_interpreter::interpreter::{InterpreterState, HeapInit}; - /// let mut state = InterpreterState::default(); - /// let heap_id = state.register_heap(HeapInit::Zeroed(1024)); - /// let heap_base = state.get_heap_address(I64, heap_id, 0); - /// let heap_bound = state.get_heap_address(I64, heap_id, 1024); - /// ``` - pub fn get_heap_address( - &self, - ty: Type, - heap_id: HeapId, - offset: u64, - ) -> Result { - let size = AddressSize::try_from(ty)?; - let heap_id = heap_id.0 as u64; - let addr = Address::from_parts(size, AddressRegion::Heap, heap_id, offset)?; - - self.validate_address(&addr)?; - let dv = addr.try_into()?; - - Ok(dv) - } - fn current_frame_mut(&mut self) -> &mut Frame<'a> { let num_frames = self.frame_stack.len(); match num_frames { @@ -398,33 +330,6 @@ impl<'a> State<'a, DataValue> for InterpreterState<'a> { Address::from_parts(size, AddressRegion::Stack, 0, final_offset) } - /// Builds an [Address] for the [Heap] referenced in the currently executing function. - /// - /// A CLIF Heap is essentially a GlobalValue and some metadata about that memory - /// region, such as bounds. Since heaps are based on Global Values it means that - /// once that GV is resolved we can essentially end up anywhere in memory. - /// - /// To build an [Address] we perform GV resolution, and try to ensure that we end up - /// in a valid region of memory. - fn heap_address( - &self, - size: AddressSize, - heap: Heap, - offset: u64, - ) -> Result { - let heap_data = &self.get_current_function().heaps[heap]; - let heap_base = self.resolve_global_value(heap_data.base)?; - let mut addr = Address::try_from(heap_base)?; - addr.size = size; - addr.offset += offset; - - // After resolving the address can point anywhere, we need to check if it's - // still valid. - self.validate_address(&addr)?; - - Ok(addr) - } - fn checked_load(&self, addr: Address, ty: Type) -> Result { let load_size = ty.bytes() as usize; let addr_start = addr.offset as usize; @@ -438,14 +343,6 @@ impl<'a> State<'a, DataValue> for InterpreterState<'a> { &self.stack[addr_start..addr_end] } - AddressRegion::Heap => { - let heap_mem = match self.heaps.get(addr.entry as usize) { - Some(mem) if addr_end <= mem.len() => mem, - _ => return Err(MemoryError::OutOfBoundsLoad { addr, load_size }), - }; - - &heap_mem[addr_start..addr_end] - } _ => unimplemented!(), }; @@ -465,14 +362,6 @@ impl<'a> State<'a, DataValue> for InterpreterState<'a> { &mut self.stack[addr_start..addr_end] } - AddressRegion::Heap => { - let heap_mem = match self.heaps.get_mut(addr.entry as usize) { - Some(mem) if addr_end <= mem.len() => mem, - _ => return Err(MemoryError::OutOfBoundsStore { addr, store_size }), - }; - - &mut heap_mem[addr_start..addr_end] - } _ => unimplemented!(), }; @@ -588,24 +477,7 @@ impl<'a> State<'a, DataValue> for InterpreterState<'a> { if addr.offset > stack_len { return Err(MemoryError::InvalidEntry { entry: addr.entry, - max: self.heaps.len() as u64, - }); - } - } - AddressRegion::Heap => { - let heap_len = self - .heaps - .get(addr.entry as usize) - .ok_or_else(|| MemoryError::InvalidEntry { - entry: addr.entry, - max: self.heaps.len() as u64, - }) - .map(|heap| heap.len() as u64)?; - - if addr.offset > heap_len { - return Err(MemoryError::InvalidOffset { - offset: addr.offset, - max: heap_len, + max: self.stack.len() as u64, }); } } @@ -629,7 +501,6 @@ mod tests { use super::*; use crate::step::CraneliftTrap; use cranelift_codegen::ir::immediates::Ieee32; - use cranelift_codegen::ir::types::I64; use cranelift_codegen::ir::TrapCode; use cranelift_reader::parse_functions; use smallvec::smallvec; @@ -995,53 +866,6 @@ mod tests { assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapOutOfBounds)); } - /// Most heap tests are in .clif files using the filetest machinery. However, this is a sanity - /// check that the heap mechanism works without the rest of the filetest infrastructure - #[test] - fn heap_sanity_test() { - let code = " - function %heap_load_store(i64 vmctx) -> i8 { - gv0 = vmctx - gv1 = load.i64 notrap aligned gv0+0 - ; gv2/3 do nothing, but makes sure we understand the iadd_imm mechanism - gv2 = iadd_imm.i64 gv1, 1 - gv3 = iadd_imm.i64 gv2, -1 - heap0 = static gv3, min 0x1000, bound 0x1_0000_0000, offset_guard 0, index_type i64 - - block0(v0: i64): - v1 = iconst.i64 0 - v2 = iconst.i64 123 - v3 = heap_addr.i64 heap0, v1, 0, 8 - store.i64 v2, v3 - v4 = load.i64 v3 - v5 = icmp eq v2, v4 - return v5 - }"; - - let func = parse_functions(code).unwrap().into_iter().next().unwrap(); - let mut env = FunctionStore::default(); - env.add(func.name.to_string(), &func); - let mut state = InterpreterState::default().with_function_store(env); - - let heap0 = state.register_heap(HeapInit::Zeroed(0x1000)); - let base_addr = state.get_heap_address(I64, heap0, 0).unwrap(); - - // Build a vmctx struct by writing the base pointer at index 0 - let mut vmctx_struct = vec![0u8; 8]; - base_addr.write_to_slice(&mut vmctx_struct[..]); - - // This is our vmctx "heap" - let vmctx = state.register_heap(HeapInit::FromBacking(vmctx_struct)); - let vmctx_addr = state.get_heap_address(I64, vmctx, 0).unwrap(); - - let result = Interpreter::new(state) - .call_by_name("%heap_load_store", &[vmctx_addr]) - .unwrap() - .unwrap_return(); - - assert_eq!(result, vec![DataValue::I8(1)]) - } - #[test] fn srem_trap() { let code = "function %test() -> i64 { diff --git a/cranelift/interpreter/src/state.rs b/cranelift/interpreter/src/state.rs index 48fe3ba6f087..53c35d2450e1 100644 --- a/cranelift/interpreter/src/state.rs +++ b/cranelift/interpreter/src/state.rs @@ -4,7 +4,7 @@ use crate::address::{Address, AddressSize}; use crate::interpreter::LibCallHandler; use cranelift_codegen::data_value::DataValue; use cranelift_codegen::ir::condcodes::{FloatCC, IntCC}; -use cranelift_codegen::ir::{FuncRef, Function, GlobalValue, Heap, StackSlot, Type, Value}; +use cranelift_codegen::ir::{FuncRef, Function, GlobalValue, StackSlot, Type, Value}; use cranelift_entity::PrimaryMap; use smallvec::SmallVec; use thiserror::Error; @@ -69,13 +69,6 @@ pub trait State<'a, V> { slot: StackSlot, offset: u64, ) -> Result; - /// Computes a heap address - fn heap_address( - &self, - size: AddressSize, - heap: Heap, - offset: u64, - ) -> Result; /// Retrieve a value `V` from memory at the given `address`, checking if it belongs either to the /// stack or to one of the heaps; the number of bytes loaded corresponds to the specified [Type]. fn checked_load(&self, address: Address, ty: Type) -> Result; @@ -175,15 +168,6 @@ where unimplemented!() } - fn heap_address( - &self, - _size: AddressSize, - _heap: Heap, - _offset: u64, - ) -> Result { - unimplemented!() - } - fn checked_load(&self, _addr: Address, _ty: Type) -> Result { unimplemented!() } diff --git a/cranelift/reader/src/heap_command.rs b/cranelift/reader/src/heap_command.rs deleted file mode 100644 index 280482eff8f5..000000000000 --- a/cranelift/reader/src/heap_command.rs +++ /dev/null @@ -1,71 +0,0 @@ -//! Heap commands. -//! -//! Functions in a `.clif` file can have *heap commands* appended that control the heaps allocated -//! by the `test run` and `test interpret` infrastructure. -//! -//! The general syntax is: -//! - `; heap: , size=n` -//! -//! `heap_type` can have two values: -//! - `static`: This is a non resizable heap type with a fixed size -//! - `dynamic`: This is a resizable heap, which can grow -//! -//! `size=n` indicates the size of the heap. For dynamic heaps, it indicates the starting size of -//! the heap. - -use cranelift_codegen::ir::immediates::Uimm64; -use std::fmt::{self, Display, Formatter}; - -/// A heap command appearing in a test file. -/// -/// For parsing, see `Parser::parse_heap_command` -#[derive(PartialEq, Debug, Clone)] -pub struct HeapCommand { - /// Indicates the requested heap type - pub heap_type: HeapType, - /// Size of the heap. - /// - /// For dynamic heaps this is the starting size. For static heaps, this is the total size. - pub size: Uimm64, - /// Offset of the heap pointer from the vmctx base - /// - /// This is done for verification purposes only - pub ptr_offset: Option, - /// Offset of the bound pointer from the vmctx base - /// - /// This is done for verification purposes only - pub bound_offset: Option, -} - -impl Display for HeapCommand { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "heap: {}, size={}", self.heap_type, self.size)?; - - if let Some(offset) = self.ptr_offset { - write!(f, ", ptr=vmctx+{}", offset)? - } - - if let Some(offset) = self.bound_offset { - write!(f, ", bound=vmctx+{}", offset)? - } - - Ok(()) - } -} - -/// CLIF Representation of a heap type. e.g.: `static` -#[allow(missing_docs)] -#[derive(Debug, PartialEq, Clone)] -pub enum HeapType { - Static, - Dynamic, -} - -impl Display for HeapType { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - match self { - HeapType::Static => write!(f, "static"), - HeapType::Dynamic => write!(f, "dynamic"), - } - } -} diff --git a/cranelift/reader/src/lexer.rs b/cranelift/reader/src/lexer.rs index 6a23dd32b91a..ccfcf59b90e4 100644 --- a/cranelift/reader/src/lexer.rs +++ b/cranelift/reader/src/lexer.rs @@ -40,7 +40,6 @@ pub enum Token<'a> { StackSlot(u32), // ss3 DynamicStackSlot(u32), // dss4 GlobalValue(u32), // gv3 - Heap(u32), // heap2 Table(u32), // table2 JumpTable(u32), // jt2 Constant(u32), // const2 @@ -348,7 +347,6 @@ impl<'a> Lexer<'a> { "dss" => Some(Token::DynamicStackSlot(number)), "dt" => Some(Token::DynamicType(number)), "gv" => Some(Token::GlobalValue(number)), - "heap" => Some(Token::Heap(number)), "table" => Some(Token::Table(number)), "jt" => Some(Token::JumpTable(number)), "const" => Some(Token::Constant(number)), diff --git a/cranelift/reader/src/lib.rs b/cranelift/reader/src/lib.rs index 33ac7b2e6754..d0d506d1859d 100644 --- a/cranelift/reader/src/lib.rs +++ b/cranelift/reader/src/lib.rs @@ -26,10 +26,9 @@ )] pub use crate::error::{Location, ParseError, ParseResult}; -pub use crate::heap_command::{HeapCommand, HeapType}; pub use crate::isaspec::{parse_options, IsaSpec, ParseOptionError}; pub use crate::parser::{ - parse_functions, parse_heap_command, parse_run_command, parse_test, ParseOptions, + parse_functions, parse_run_command, parse_test, ParseOptions, }; pub use crate::run_command::{Comparison, Invocation, RunCommand}; pub use crate::sourcemap::SourceMap; @@ -37,7 +36,6 @@ pub use crate::testcommand::{TestCommand, TestOption}; pub use crate::testfile::{Comment, Details, Feature, TestFile}; mod error; -mod heap_command; mod isaspec; mod lexer; mod parser; diff --git a/cranelift/reader/src/parser.rs b/cranelift/reader/src/parser.rs index 5fdddb2e8a4a..57629a02ff08 100644 --- a/cranelift/reader/src/parser.rs +++ b/cranelift/reader/src/parser.rs @@ -1,7 +1,6 @@ //! Parser for .clif files. use crate::error::{Location, ParseError, ParseResult}; -use crate::heap_command::{HeapCommand, HeapType}; use crate::isaspec; use crate::lexer::{LexError, Lexer, LocatedError, LocatedToken, Token}; use crate::run_command::{Comparison, Invocation, RunCommand}; @@ -11,9 +10,7 @@ use crate::testfile::{Comment, Details, Feature, TestFile}; use cranelift_codegen::data_value::DataValue; use cranelift_codegen::entity::{EntityRef, PrimaryMap}; use cranelift_codegen::ir::entities::{AnyEntity, DynamicType}; -use cranelift_codegen::ir::immediates::{ - Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64, -}; +use cranelift_codegen::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64}; use cranelift_codegen::ir::instructions::{InstructionData, InstructionFormat, VariableArgs}; use cranelift_codegen::ir::types::INVALID; use cranelift_codegen::ir::types::*; @@ -21,9 +18,8 @@ use cranelift_codegen::ir::{self, UserExternalNameRef}; use cranelift_codegen::ir::{ AbiParam, ArgumentExtension, ArgumentPurpose, Block, Constant, ConstantData, DynamicStackSlot, DynamicStackSlotData, DynamicTypeData, ExtFuncData, ExternalName, FuncRef, Function, - GlobalValue, GlobalValueData, Heap, HeapData, HeapStyle, JumpTable, JumpTableData, MemFlags, - Opcode, SigRef, Signature, StackSlot, StackSlotData, StackSlotKind, Table, TableData, Type, - UserFuncName, Value, + GlobalValue, GlobalValueData, JumpTable, JumpTableData, MemFlags, Opcode, SigRef, Signature, + StackSlot, StackSlotData, StackSlotKind, Table, TableData, Type, UserFuncName, Value, }; use cranelift_codegen::isa::{self, CallConv}; use cranelift_codegen::packed_option::ReservedValue; @@ -189,24 +185,6 @@ pub fn parse_run_command<'a>(text: &str, signature: &Signature) -> ParseResult(text: &str) -> ParseResult> { - let _tt = timing::parse_text(); - // We remove leading spaces and semi-colons for convenience here instead of at the call sites - // since this function will be attempting to parse a HeapCommand from a CLIF comment. - let trimmed_text = text.trim_start_matches(|c| c == ' ' || c == ';'); - let mut parser = Parser::new(trimmed_text); - match parser.token() { - Some(Token::Identifier("heap")) => parser.parse_heap_command().map(|c| Some(c)), - Some(_) | None => Ok(None), - } -} - pub struct Parser<'a> { lex: Lexer<'a>, @@ -340,24 +318,6 @@ impl Context { } } - // Allocate a heap slot. - fn add_heap(&mut self, heap: Heap, data: HeapData, loc: Location) -> ParseResult<()> { - self.map.def_heap(heap, loc)?; - while self.function.heaps.next_key().index() <= heap.index() { - self.function.create_heap(HeapData { - base: GlobalValue::reserved_value(), - min_size: Uimm64::new(0), - offset_guard_size: Uimm64::new(0), - style: HeapStyle::Static { - bound: Uimm64::new(0), - }, - index_type: INVALID, - }); - } - self.function.heaps[heap] = data; - Ok(()) - } - // Allocate a table slot. fn add_table(&mut self, table: Table, data: TableData, loc: Location) -> ParseResult<()> { while self.function.tables.next_key().index() <= table.index() { @@ -699,17 +659,6 @@ impl<'a> Parser<'a> { err!(self.loc, err_msg) } - // Match and consume a heap reference. - fn match_heap(&mut self, err_msg: &str) -> ParseResult { - if let Some(Token::Heap(heap)) = self.token() { - self.consume(); - if let Some(heap) = Heap::with_number(heap) { - return Ok(heap); - } - } - err!(self.loc, err_msg) - } - // Match and consume a table reference. fn match_table(&mut self, err_msg: &str) -> ParseResult
{ if let Some(Token::Table(table)) = self.token() { @@ -1519,11 +1468,6 @@ impl<'a> Parser<'a> { self.parse_global_value_decl() .and_then(|(gv, dat)| ctx.add_gv(gv, dat, self.loc)) } - Some(Token::Heap(..)) => { - self.start_gathering_comments(); - self.parse_heap_decl() - .and_then(|(heap, dat)| ctx.add_heap(heap, dat, self.loc)) - } Some(Token::Table(..)) => { self.start_gathering_comments(); self.parse_table_decl() @@ -1711,77 +1655,6 @@ impl<'a> Parser<'a> { Ok((gv, data)) } - // Parse a heap decl. - // - // heap-decl ::= * Heap(heap) "=" heap-desc - // heap-desc ::= heap-style heap-base { "," heap-attr } - // heap-style ::= "static" | "dynamic" - // heap-base ::= GlobalValue(base) - // heap-attr ::= "min" Imm64(bytes) - // | "bound" Imm64(bytes) - // | "offset_guard" Imm64(bytes) - // | "index_type" type - // - fn parse_heap_decl(&mut self) -> ParseResult<(Heap, HeapData)> { - let heap = self.match_heap("expected heap number: heap«n»")?; - self.match_token(Token::Equal, "expected '=' in heap declaration")?; - - let style_name = self.match_any_identifier("expected 'static' or 'dynamic'")?; - - // heap-desc ::= heap-style * heap-base { "," heap-attr } - // heap-base ::= * GlobalValue(base) - let base = match self.token() { - Some(Token::GlobalValue(base_num)) => match GlobalValue::with_number(base_num) { - Some(gv) => gv, - None => return err!(self.loc, "invalid global value number for heap base"), - }, - _ => return err!(self.loc, "expected heap base"), - }; - self.consume(); - - let mut data = HeapData { - base, - min_size: 0.into(), - offset_guard_size: 0.into(), - style: HeapStyle::Static { bound: 0.into() }, - index_type: ir::types::I32, - }; - - // heap-desc ::= heap-style heap-base * { "," heap-attr } - while self.optional(Token::Comma) { - match self.match_any_identifier("expected heap attribute name")? { - "min" => { - data.min_size = self.match_uimm64("expected integer min size")?; - } - "bound" => { - data.style = match style_name { - "dynamic" => HeapStyle::Dynamic { - bound_gv: self.match_gv("expected gv bound")?, - }, - "static" => HeapStyle::Static { - bound: self.match_uimm64("expected integer bound")?, - }, - t => return err!(self.loc, "unknown heap style '{}'", t), - }; - } - "offset_guard" => { - data.offset_guard_size = - self.match_uimm64("expected integer offset-guard size")?; - } - "index_type" => { - data.index_type = self.match_type("expected index type")?; - } - t => return err!(self.loc, "unknown heap attribute '{}'", t), - } - } - - // Collect any trailing comments. - self.token(); - self.claim_gathered_comments(heap); - - Ok((heap, data)) - } - // Parse a table decl. // // table-decl ::= * Table(table) "=" table-desc @@ -2435,86 +2308,6 @@ impl<'a> Parser<'a> { Ok(args) } - /// Parse a vmctx offset annotation - /// - /// vmctx-offset ::= "vmctx" "+" UImm64(offset) - fn parse_vmctx_offset(&mut self) -> ParseResult { - self.match_token(Token::Identifier("vmctx"), "expected a 'vmctx' token")?; - - // The '+' token here gets parsed as part of the integer text, so we can't just match_token it - // and `match_uimm64` doesn't support leading '+' tokens, so we can't use that either. - match self.token() { - Some(Token::Integer(text)) if text.starts_with('+') => { - self.consume(); - - text[1..] - .parse() - .map_err(|_| self.error("expected u64 decimal immediate")) - } - token => err!( - self.loc, - format!("Unexpected token {:?} after vmctx", token) - ), - } - } - - /// Parse a CLIF heap command. - /// - /// heap-command ::= "heap" ":" heap-type { "," heap-attr } - /// heap-attr ::= "size" "=" UImm64(bytes) - fn parse_heap_command(&mut self) -> ParseResult { - self.match_token(Token::Identifier("heap"), "expected a 'heap:' command")?; - self.match_token(Token::Colon, "expected a ':' after heap command")?; - - let mut heap_command = HeapCommand { - heap_type: self.parse_heap_type()?, - size: Uimm64::new(0), - ptr_offset: None, - bound_offset: None, - }; - - while self.optional(Token::Comma) { - let identifier = self.match_any_identifier("expected heap attribute name")?; - self.match_token(Token::Equal, "expected '=' after heap attribute name")?; - - match identifier { - "size" => { - heap_command.size = self.match_uimm64("expected integer size")?; - } - "ptr" => { - heap_command.ptr_offset = Some(self.parse_vmctx_offset()?); - } - "bound" => { - heap_command.bound_offset = Some(self.parse_vmctx_offset()?); - } - t => return err!(self.loc, "unknown heap attribute '{}'", t), - } - } - - if heap_command.size == Uimm64::new(0) { - return err!(self.loc, self.error("Expected a heap size to be specified")); - } - - Ok(heap_command) - } - - /// Parse a heap type. - /// - /// heap-type ::= "static" | "dynamic" - fn parse_heap_type(&mut self) -> ParseResult { - match self.token() { - Some(Token::Identifier("static")) => { - self.consume(); - Ok(HeapType::Static) - } - Some(Token::Identifier("dynamic")) => { - self.consume(); - Ok(HeapType::Dynamic) - } - _ => Err(self.error("expected a heap type, e.g. static or dynamic")), - } - } - /// Parse a CLIF run command. /// /// run-command ::= "run" [":" invocation comparison expected] @@ -3334,25 +3127,6 @@ mod tests { assert!(!is_warning); } - #[test] - fn duplicate_heap() { - let ParseError { - location, - message, - is_warning, - } = Parser::new( - "function %blocks() system_v { - heap0 = static gv0, min 0x1000, bound 0x10_0000, offset_guard 0x1000 - heap0 = static gv0, min 0x1000, bound 0x10_0000, offset_guard 0x1000", - ) - .parse_function() - .unwrap_err(); - - assert_eq!(location.line_number, 3); - assert_eq!(message, "duplicate entity: heap0"); - assert!(!is_warning); - } - #[test] fn duplicate_sig() { let ParseError { @@ -3759,45 +3533,6 @@ mod tests { assert!(parse("run: ", &sig(&[], &[])).is_err()); } - #[test] - fn parse_heap_commands() { - fn parse(text: &str) -> ParseResult { - Parser::new(text).parse_heap_command() - } - - // Check that we can parse and display the same set of heap commands. - fn assert_roundtrip(text: &str) { - assert_eq!(parse(text).unwrap().to_string(), text); - } - - assert_roundtrip("heap: static, size=10"); - assert_roundtrip("heap: dynamic, size=10"); - assert_roundtrip("heap: static, size=10, ptr=vmctx+10"); - assert_roundtrip("heap: static, size=10, bound=vmctx+11"); - assert_roundtrip("heap: static, size=10, ptr=vmctx+10, bound=vmctx+10"); - assert_roundtrip("heap: dynamic, size=10, ptr=vmctx+10"); - assert_roundtrip("heap: dynamic, size=10, bound=vmctx+11"); - assert_roundtrip("heap: dynamic, size=10, ptr=vmctx+10, bound=vmctx+10"); - - let static_heap = parse("heap: static, size=10, ptr=vmctx+8, bound=vmctx+2").unwrap(); - assert_eq!(static_heap.size, Uimm64::new(10)); - assert_eq!(static_heap.heap_type, HeapType::Static); - assert_eq!(static_heap.ptr_offset, Some(Uimm64::new(8))); - assert_eq!(static_heap.bound_offset, Some(Uimm64::new(2))); - let dynamic_heap = parse("heap: dynamic, size=0x10").unwrap(); - assert_eq!(dynamic_heap.size, Uimm64::new(16)); - assert_eq!(dynamic_heap.heap_type, HeapType::Dynamic); - assert_eq!(dynamic_heap.ptr_offset, None); - assert_eq!(dynamic_heap.bound_offset, None); - - assert!(parse("heap: static").is_err()); - assert!(parse("heap: dynamic").is_err()); - assert!(parse("heap: static size=0").is_err()); - assert!(parse("heap: dynamic size=0").is_err()); - assert!(parse("heap: static, size=10, ptr=10").is_err()); - assert!(parse("heap: static, size=10, bound=vmctx-10").is_err()); - } - #[test] fn parse_data_values() { fn parse(text: &str, ty: Type) -> DataValue { diff --git a/cranelift/reader/src/sourcemap.rs b/cranelift/reader/src/sourcemap.rs index 00425dc5863d..57be4d55ee41 100644 --- a/cranelift/reader/src/sourcemap.rs +++ b/cranelift/reader/src/sourcemap.rs @@ -10,8 +10,8 @@ use crate::error::{Location, ParseResult}; use crate::lexer::split_entity_name; use cranelift_codegen::ir::entities::{AnyEntity, DynamicType}; use cranelift_codegen::ir::{ - Block, Constant, DynamicStackSlot, FuncRef, GlobalValue, Heap, JumpTable, SigRef, StackSlot, - Table, Value, + Block, Constant, DynamicStackSlot, FuncRef, GlobalValue, JumpTable, SigRef, StackSlot, Table, + Value, }; use std::collections::HashMap; @@ -49,11 +49,6 @@ impl SourceMap { self.locations.contains_key(&gv.into()) } - /// Look up a heap entity. - pub fn contains_heap(&self, heap: Heap) -> bool { - self.locations.contains_key(&heap.into()) - } - /// Look up a table entity. pub fn contains_table(&self, table: Table) -> bool { self.locations.contains_key(&table.into()) @@ -111,13 +106,6 @@ impl SourceMap { Some(gv.into()) } }), - "heap" => Heap::with_number(num).and_then(|heap| { - if !self.contains_heap(heap) { - None - } else { - Some(heap.into()) - } - }), "table" => Table::with_number(num).and_then(|table| { if !self.contains_table(table) { None @@ -194,11 +182,6 @@ impl SourceMap { self.def_entity(entity.into(), loc) } - /// Define the heap `entity`. - pub fn def_heap(&mut self, entity: Heap, loc: Location) -> ParseResult<()> { - self.def_entity(entity.into(), loc) - } - /// Define the table `entity`. pub fn def_table(&mut self, entity: Table, loc: Location) -> ParseResult<()> { self.def_entity(entity.into(), loc)