T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix legalization of heap_addrs with 32-bit indices. (#480)

Browse Source 
This makes several changes:
 - It adds an index_type to heap declarations, allowing heaps to specify the
   type for indexing. This also anticipates 64-bit heap support.

 - It adds a memory_type to deref global values, allowing deref globals to
   have types other than pointers. This is used to allow the bound variable
   in dynamic heaps to have type i32, to match the index type in heaps
   with i32 index type.

 - And, it fixes heap legalization to do the bounds check in the heap's
   index type.
This commit is contained in:
Dan Gohman
2018-08-28 13:37:33 -07:00
committed by GitHub
parent 00ddf3a7a6
commit eb439c9a68
13 changed files with 396 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

111
cranelift/filetests/isa/x86/legalize-heaps.clif Normal file
View File

@@ -0,0 +1,111 @@
test legalizer
target x86_64
; Test legalization for various forms of heap addresses.
function %heap_addrs(i32, i64, i64 vmctx) {
gv0 = vmctx+64
gv1 = vmctx+72
gv2 = vmctx+80
gv3 = vmctx+88
gv4 = deref(gv3): i32
heap0 = static gv0, min 0x1_0000, bound 0x1_0000_0000, guard 0x8000_0000, index_type i32
heap1 = static gv0, guard 0x1000, bound 0x1_0000, index_type i32
heap2 = static gv0, min 0x1_0000, bound 0x1_0000_0000, guard 0x8000_0000, index_type i64
heap3 = static gv0, guard 0x1000, bound 0x1_0000, index_type i64
heap4 = dynamic gv1, min 0x1_0000, bound gv4, guard 0x8000_0000, index_type i32
heap5 = dynamic gv1, bound gv4, guard 0x1000, index_type i32
heap6 = dynamic gv1, min 0x1_0000, bound gv2, guard 0x8000_0000, index_type i64
heap7 = dynamic gv1, bound gv2, guard 0x1000, index_type i64
; check: heap0 = static gv0, min 0x0001_0000, bound 0x0001_0000_0000, guard 0x8000_0000, index_type i32
; check: heap1 = static gv0, min 0, bound 0x0001_0000, guard 4096, index_type i32
; check: heap2 = static gv0, min 0x0001_0000, bound 0x0001_0000_0000, guard 0x8000_0000, index_type i64
; check: heap3 = static gv0, min 0, bound 0x0001_0000, guard 4096, index_type i64
; check: heap4 = dynamic gv1, min 0x0001_0000, bound gv4, guard 0x8000_0000, index_type i32
; check: heap5 = dynamic gv1, min 0, bound gv4, guard 4096, index_type i32
; check: heap6 = dynamic gv1, min 0x0001_0000, bound gv2, guard 0x8000_0000, index_type i64
; check: heap7 = dynamic gv1, min 0, bound gv2, guard 4096, index_type i64
ebb0(v0: i32, v1: i64, v3: i64):
; The fast-path; 32-bit index, static heap with a sufficient bound, no bounds check needed!
v4 = heap_addr.i64 heap0, v0, 0
; check: v12 = uextend.i64 v0
; check: v13 = iadd_imm v3, 64
; check: v4 = iadd v13, v12
v5 = heap_addr.i64 heap1, v0, 0
; check: v14 = icmp_imm ugt v0, 0x0001_0000
; check: brz v14, ebb1
; check: trap heap_oob
; check: ebb1:
; check: v15 = uextend.i64 v0
; check: v16 = iadd_imm.i64 v3, 64
; check: v5 = iadd v16, v15
v6 = heap_addr.i64 heap2, v1, 0
; check: v19 = iconst.i64 0x0001_0000_0000
; check: v17 = icmp.i64 ugt v1, v19
; check: brz v17, ebb2
; check: trap heap_oob
; check: ebb2:
; check: v18 = iadd_imm.i64 v3, 64
; check: v6 = iadd v18, v1
v7 = heap_addr.i64 heap3, v1, 0
; check: v20 = icmp_imm.i64 ugt v1, 0x0001_0000
; check: brz v20, ebb3
; check: trap heap_oob
; check: ebb3:
; check: v21 = iadd_imm.i64 v3, 64
; check: v7 = iadd v21, v1
v8 = heap_addr.i64 heap4, v0, 0
; check: v27 = iadd_imm.i64 v3, 88
; check: v28 = load.i32 notrap aligned v27
; check: v22 = iadd_imm v28, 0
; check: v23 = iadd_imm v22, 0
; check: v24 = icmp.i32 ugt v0, v23
; check: brz v24, ebb4
; check: trap heap_oob
; check: ebb4:
; check: v25 = uextend.i64 v0
; check: v26 = iadd_imm.i64 v3, 72
; check: v8 = iadd v26, v25
v9 = heap_addr.i64 heap5, v0, 0
; check: v34 = iadd_imm.i64 v3, 88
; check: v35 = load.i32 notrap aligned v34
; check: v29 = iadd_imm v35, 0
; check: v30 = iadd_imm v29, 0
; check: v31 = icmp.i32 ugt v0, v30
; check: brz v31, ebb5
; check: trap heap_oob
; check: ebb5:
; check: v32 = uextend.i64 v0
; check: v33 = iadd_imm.i64 v3, 72
; check: v9 = iadd v33, v32
v10 = heap_addr.i64 heap6, v1, 0
; check: v36 = iadd_imm.i64 v3, 80
; check: v37 = iadd_imm v36, 0
; check: v38 = icmp.i64 ugt v1, v37
; check: brz v38, ebb6
; check: trap heap_oob
; check: ebb6:
; check: v39 = iadd_imm.i64 v3, 72
; check: v10 = iadd v39, v1
v11 = heap_addr.i64 heap7, v1, 0
; check: v40 = iadd_imm.i64 v3, 80
; check: v41 = iadd_imm v40, 0
; check: v42 = icmp.i64 ugt v1, v41
; check: brz v42, ebb7
; check: trap heap_oob
; check: ebb7:
; check: v43 = iadd_imm.i64 v3, 72
; check: v11 = iadd v43, v1
return
}

2
cranelift/filetests/isa/x86/legalize-memory.clif
View File

@@ -17,7 +17,7 @@ ebb1(v1: i64):
function %deref(i64 vmctx) -> i64 {
gv1 = vmctx-16
gv2 = deref(gv1)+32
gv2 = deref(gv1)+32: i64
ebb1(v1: i64):
v2 = global_value.i64 gv2

4
cranelift/filetests/parser/memory.clif
View File

@@ -17,7 +17,7 @@ ebb0(v0: i64):
function %deref(i64 vmctx) -> i32 {
gv3 = vmctx+16
gv4 = deref(gv3)-32
gv4 = deref(gv3)-32: i32
; check: gv4 = deref(gv3)-32
ebb0(v0: i64):
v1 = global_value.i32 gv4
@@ -27,7 +27,7 @@ ebb0(v0: i64):
; Refer to a global value before it's been declared.
function %backref(i64 vmctx) -> i32 {
gv1 = deref(gv2)-32
gv1 = deref(gv2)-32: i32
; check: gv1 = deref(gv2)-32
gv2 = vmctx+16
; check: gv2 = vmctx+16

19
cranelift/filetests/verifier/globals.clif Normal file
View File

@@ -0,0 +1,19 @@
test verifier
target x86_64
function %deref_base_type(i64 vmctx) {
gv0 = vmctx+0
gv1 = deref(gv0): i32
gv2 = deref(gv1): i32 ; error: deref base gv1 has type i32, which is not the pointer type i64
ebb0(v0: i64):
return
}
function %global_value_wrong_type(i64 vmctx) {
gv0 = vmctx+0
ebb0(v0: i64):
v1 = global_value.i32 gv0 ; error: global_value instruction with type i32 references global value with type i64
return
}

45
cranelift/filetests/verifier/heap.clif Normal file
View File

@@ -0,0 +1,45 @@
test verifier
target x86_64
function %heap_base_type(i64 vmctx) {
gv0 = vmctx+0
gv1 = deref(gv0): i32
heap0 = static gv1, guard 0x1000, bound 0x1_0000, index_type i32 ; error: heap base has type i32, which is not the pointer type i64
ebb0(v0: i64):
return
}
function %invalid_base(i64 vmctx) {
gv0 = vmctx+0
heap0 = dynamic gv1, bound gv0, guard 0x1000, index_type i64 ; error: invalid base global value gv1
ebb0(v0: i64):
return
}
function %invalid_bound(i64 vmctx) {
gv0 = vmctx+0
heap0 = dynamic gv0, bound gv1, guard 0x1000, index_type i64 ; error: invalid bound global value gv1
ebb0(v0: i64):
return
}
function %heap_bound_type(i64 vmctx) {
gv0 = vmctx+0
gv1 = deref(gv0): i16
heap0 = dynamic gv1, bound gv1, guard 0x1000, index_type i32 ; error: heap index type i32 differs from the type of its bound, i16
ebb0(v0: i64):
return
}
function %heap_addr_index_type(i64 vmctx, i64) {
gv0 = vmctx+0
heap0 = static gv0, guard 0x1000, bound 0x1_0000, index_type i32
ebb0(v0: i64, v1: i64):
v2 = heap_addr.i64 heap0, v1, 0; error: index type i64 differs from heap index type i32
return
}

6
cranelift/filetests/verifier/memory.clif
View File

@@ -1,15 +1,15 @@
test verifier
function %deref_cycle() {
gv1 = deref(gv2)-32 ; error: deref cycle: [gv1, gv2]
gv2 = deref(gv1)
gv1 = deref(gv2)-32: i32 ; error: deref cycle: [gv1, gv2]
gv2 = deref(gv1): i32
ebb1:
return
}
function %self_cycle() {
gv0 = deref(gv0)-32 ; error: deref cycle: [gv0]
gv0 = deref(gv0)-32: i32 ; error: deref cycle: [gv0]
ebb1:
return

22
lib/codegen/src/ir/globalvalue.rs
View File

@@ -1,7 +1,8 @@
//! Global values.
use ir::immediates::Offset32;
use ir::{ExternalName, GlobalValue};
use ir::{ExternalName, GlobalValue, Type};
use isa::TargetIsa;
use std::fmt;
/// Information about a global value declaration.
@@ -18,13 +19,16 @@ pub enum GlobalValueData {
///
/// The `base` global value is assumed to contain a pointer. This global value is computed
/// by loading from memory at that pointer value, and then adding an offset. The memory must
/// be accessible, and naturally aligned to hold a pointer value.
/// be accessible, and naturally aligned to hold a value of the type.
Deref {
/// The base pointer global value.
base: GlobalValue,
/// Byte offset to be added to the loaded value.
offset: Offset32,
/// Type of the loaded value.
memory_type: Type,
},
/// Value is identified by a symbolic name. Cranelift itself does not interpret this name;
@@ -48,13 +52,25 @@ impl GlobalValueData {
_ => panic!("only symbols have names"),
}
}
/// Return the type of this global.
pub fn global_type(&self, isa: &TargetIsa) -> Type {
match *self {
GlobalValueData::VMContext { .. } | GlobalValueData::Sym { .. } => isa.pointer_type(),
GlobalValueData::Deref { memory_type, .. } => memory_type,
}
}
}
impl fmt::Display for GlobalValueData {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
GlobalValueData::VMContext { offset } => write!(f, "vmctx{}", offset),
GlobalValueData::Deref { base, offset } => write!(f, "deref({}){}", base, offset),
GlobalValueData::Deref {
base,
offset,
memory_type,
} => write!(f, "deref({}){}: {}", base, offset, memory_type),
GlobalValueData::Sym {
ref name,
colocated,

11
lib/codegen/src/ir/heap.rs
View File

@@ -1,7 +1,7 @@
//! Heaps.
use ir::immediates::Imm64;
use ir::GlobalValue;
use ir::{GlobalValue, Type};
use std::fmt;
/// Information about a heap declaration.
@@ -19,6 +19,9 @@ pub struct HeapData {
/// 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.
@@ -50,6 +53,10 @@ impl fmt::Display for HeapData {
HeapStyle::Dynamic { bound_gv } => write!(f, ", bound {}", bound_gv)?,
HeapStyle::Static { bound } => write!(f, ", bound {}", bound)?,
}
write!(f, ", guard {}", self.guard_size)
write!(
f,
", guard {}, index_type {}",
self.guard_size, self.index_type
)
}
}

29
lib/codegen/src/legalizer/globalvalue.rs
View File

@@ -13,7 +13,7 @@ pub fn expand_global_value(
inst: ir::Inst,
func: &mut ir::Function,
_cfg: &mut ControlFlowGraph,
_isa: &TargetIsa,
isa: &TargetIsa,
) {
// Unpack the instruction.
let gv = match func.dfg[inst] {
@@ -29,8 +29,12 @@ pub fn expand_global_value(
match func.global_values[gv] {
ir::GlobalValueData::VMContext { offset } => vmctx_addr(inst, func, offset.into()),
ir::GlobalValueData::Deref { base, offset } => deref_addr(inst, func, base, offset.into()),
ir::GlobalValueData::Sym { .. } => globalsym(inst, func, gv),
ir::GlobalValueData::Deref {
base,
offset,
memory_type,
} => deref_addr(inst, func, base, offset.into(), memory_type, isa),
ir::GlobalValueData::Sym { .. } => globalsym(inst, func, gv, isa),
}
}
@@ -46,11 +50,18 @@ fn vmctx_addr(inst: ir::Inst, func: &mut ir::Function, offset: i64) {
}
/// Expand a `global_value` instruction for a deref global.
fn deref_addr(inst: ir::Inst, func: &mut ir::Function, base: ir::GlobalValue, offset: i64) {
fn deref_addr(
inst: ir::Inst,
func: &mut ir::Function,
base: ir::GlobalValue,
offset: i64,
memory_type: ir::Type,
isa: &TargetIsa,
) {
// We need to load a pointer from the `base` global value, so insert a new `global_value`
// instruction. This depends on the iterative legalization loop. Note that the IR verifier
// detects any cycles in the `deref` globals.
let ptr_ty = func.dfg.value_type(func.dfg.first_result(inst));
let ptr_ty = isa.pointer_type();
let mut pos = FuncCursor::new(func).at_inst(inst);
pos.use_srcloc(inst);
@@ -59,12 +70,12 @@ fn deref_addr(inst: ir::Inst, func: &mut ir::Function, base: ir::GlobalValue, of
// Deref globals are required to be accessible and aligned.
mflags.set_notrap();
mflags.set_aligned();
let base_ptr = pos.ins().load(ptr_ty, mflags, base_addr, 0);
pos.func.dfg.replace(inst).iadd_imm(base_ptr, offset);
let loaded = pos.ins().load(memory_type, mflags, base_addr, 0);
pos.func.dfg.replace(inst).iadd_imm(loaded, offset);
}
/// Expand a `global_value` instruction for a symbolic name global.
fn globalsym(inst: ir::Inst, func: &mut ir::Function, gv: ir::GlobalValue) {
let ptr_ty = func.dfg.value_type(func.dfg.first_result(inst));
fn globalsym(inst: ir::Inst, func: &mut ir::Function, gv: ir::GlobalValue, isa: &TargetIsa) {
let ptr_ty = isa.pointer_type();
func.dfg.replace(inst).globalsym_addr(ptr_ty, gv);
}

2
lib/codegen/src/legalizer/heap.rs
View File

@@ -57,7 +57,7 @@ fn dynamic_addr(
pos.use_srcloc(inst);
// Start with the bounds check. Trap if `offset + access_size > bound`.
let bound = pos.ins().global_value(addr_ty, bound_gv);
let bound = pos.ins().global_value(offset_ty, bound_gv);
let oob;
if access_size == 1 {
// `offset > bound - 1` is the same as `offset >= bound`.

198
lib/codegen/src/verifier/mod.rs
View File

@@ -348,14 +348,33 @@ impl<'a> Verifier<'a> {
cur = base;
}
if let ir::GlobalValueData::VMContext { .. } = self.func.global_values[cur] {
if self
.func
.special_param(ir::ArgumentPurpose::VMContext)
.is_none()
{
report!(errors, cur, "undeclared vmctx reference {}", cur);
match self.func.global_values[gv] {
ir::GlobalValueData::VMContext { .. } => {
if self
.func
.special_param(ir::ArgumentPurpose::VMContext)
.is_none()
{
report!(errors, gv, "undeclared vmctx reference {}", gv);
}
}
ir::GlobalValueData::Deref { base, .. } => {
if let Some(isa) = self.isa {
let base_type = self.func.global_values[base].global_type(isa);
let pointer_type = isa.pointer_type();
if base_type != pointer_type {
report!(
errors,
gv,
"deref base {} has type {}, which is not the pointer type {}",
base,
base_type,
pointer_type
);
}
}
}
_ => {}
}
}
@@ -363,6 +382,57 @@ 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 nonfatal!(errors, heap, "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 {
report!(
errors,
heap,
"heap base has type {}, which is not the pointer type {}",
base_type,
pointer_type
);
}
match heap_data.style {
ir::HeapStyle::Dynamic { bound_gv, .. } => {
if !self.func.global_values.is_valid(bound_gv) {
return nonfatal!(
errors,
heap,
"invalid bound global value {}",
bound_gv
);
}
let index_type = heap_data.index_type;
let bound_type = self.func.global_values[bound_gv].global_type(isa);
if index_type != bound_type {
report!(
errors,
heap,
"heap index type {} differs from the type of its bound, {}",
index_type,
bound_type
);
}
}
_ => {}
}
}
}
Ok(())
}
fn ebb_integrity(
&self,
ebb: Ebb,
@@ -1255,51 +1325,82 @@ impl<'a> Verifier<'a> {
ctrl_type: Type,
errors: &mut VerifierErrors,
) -> VerifierStepResult<()> {
if let ir::InstructionData::Unary { opcode, arg } = self.func.dfg[inst] {
let arg_type = self.func.dfg.value_type(arg);
match opcode {
Opcode::Bextend | Opcode::Uextend | Opcode::Sextend | Opcode::Fpromote => {
if arg_type.lane_count() != ctrl_type.lane_count() {
return nonfatal!(
errors,
inst,
"input {} and output {} must have same number of lanes",
arg_type,
ctrl_type
);
match self.func.dfg[inst] {
ir::InstructionData::Unary { opcode, arg } => {
let arg_type = self.func.dfg.value_type(arg);
match opcode {
Opcode::Bextend | Opcode::Uextend | Opcode::Sextend | Opcode::Fpromote => {
if arg_type.lane_count() != ctrl_type.lane_count() {
return nonfatal!(
errors,
inst,
"input {} and output {} must have same number of lanes",
arg_type,
ctrl_type
);
}
if arg_type.lane_bits() >= ctrl_type.lane_bits() {
return nonfatal!(
errors,
inst,
"input {} must be smaller than output {}",
arg_type,
ctrl_type
);
}
}
if arg_type.lane_bits() >= ctrl_type.lane_bits() {
return nonfatal!(
errors,
inst,
"input {} must be smaller than output {}",
arg_type,
ctrl_type
);
Opcode::Breduce | Opcode::Ireduce | Opcode::Fdemote => {
if arg_type.lane_count() != ctrl_type.lane_count() {
return nonfatal!(
errors,
inst,
"input {} and output {} must have same number of lanes",
arg_type,
ctrl_type
);
}
if arg_type.lane_bits() <= ctrl_type.lane_bits() {
return nonfatal!(
errors,
inst,
"input {} must be larger than output {}",
arg_type,
ctrl_type
);
}
}
_ => {}
}
Opcode::Breduce | Opcode::Ireduce | Opcode::Fdemote => {
if arg_type.lane_count() != ctrl_type.lane_count() {
return nonfatal!(
errors,
inst,
"input {} and output {} must have same number of lanes",
arg_type,
ctrl_type
);
}
if arg_type.lane_bits() <= ctrl_type.lane_bits() {
return nonfatal!(
errors,
inst,
"input {} must be larger than output {}",
arg_type,
ctrl_type
);
}
}
_ => {}
}
ir::InstructionData::HeapAddr { heap, arg, .. } => {
let index_type = self.func.dfg.value_type(arg);
let heap_index_type = self.func.heaps[heap].index_type;
if index_type != heap_index_type {
return nonfatal!(
errors,
inst,
"index type {} differs from heap index type {}",
index_type,
heap_index_type
);
}
}
ir::InstructionData::UnaryGlobalValue { global_value, .. } => {
if let Some(isa) = self.isa {
let inst_type = self.func.dfg.value_type(self.func.dfg.first_result(inst));
let global_type = self.func.global_values[global_value].global_type(isa);
if inst_type != global_type {
return nonfatal!(
errors,
inst,
"global_value instruction with type {} references global value with type {}",
inst_type,
global_type
);
}
}
}
_ => {}
}
Ok(())
}
@@ -1509,6 +1610,7 @@ impl<'a> Verifier<'a> {
pub fn run(&self, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
self.verify_global_values(errors)?;
self.verify_heaps(errors)?;
self.typecheck_entry_block_params(errors)?;
for ebb in self.func.layout.ebbs() {

13
lib/reader/src/parser.rs
View File

@@ -174,6 +174,7 @@ impl<'a> Context<'a> {
style: HeapStyle::Static {
bound: Imm64::new(0),
},
index_type: VOID,
});
}
self.function.heaps[heap] = data;
@@ -1128,7 +1129,13 @@ impl<'a> Parser<'a> {
let base = self.match_gv("expected global value: gv«n»")?;
self.match_token(Token::RPar, "expected ')' in 'deref' global value decl")?;
let offset = self.optional_offset32()?;
GlobalValueData::Deref { base, offset }
self.match_token(Token::Colon, "expected ':' in 'deref' global value decl")?;
let memory_type = self.match_type("expected deref type")?;
GlobalValueData::Deref {
base,
offset,
memory_type,
}
}
"globalsym" => {
let colocated = self.optional(Token::Identifier("colocated"));
@@ -1177,6 +1184,7 @@ impl<'a> Parser<'a> {
min_size: 0.into(),
guard_size: 0.into(),
style: HeapStyle::Static { bound: 0.into() },
index_type: ir::types::I32,
};
// heap-desc ::= heap-style heap-base * { "," heap-attr }
@@ -1199,6 +1207,9 @@ impl<'a> Parser<'a> {
"guard" => {
data.guard_size = self.match_imm64("expected integer guard size")?;
}
"index_type" => {
data.index_type = self.match_type("expected index type")?;
}
t => return err!(self.loc, "unknown heap attribute '{}'", t),
}
}

4
lib/wasm/src/environ/dummy.rs
View File

@@ -177,6 +177,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
let gv = func.create_global_value(ir::GlobalValueData::Deref {
base: addr,
offset: 0.into(),
memory_type: self.pointer_type(),
});
func.create_heap(ir::HeapData {
@@ -186,6 +187,7 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
style: ir::HeapStyle::Static {
bound: 0x1_0000_0000.into(),
},
index_type: I32,
})
}
@@ -196,12 +198,14 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ
let base_gv = func.create_global_value(ir::GlobalValueData::Deref {
base: base_gv_addr,
offset: 0.into(),
memory_type: self.pointer_type(),
});
let bound_gv_addr =
func.create_global_value(ir::GlobalValueData::VMContext { offset: 0.into() });
let bound_gv = func.create_global_value(ir::GlobalValueData::Deref {
base: bound_gv_addr,
offset: 0.into(),
memory_type: self.pointer_type(),
});
func.create_table(ir::TableData {