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

Change VMMemoryDefinition::current_length to usize (#3134)

Browse Source 
* Change VMMemoryDefinition::current_length to `usize`

This commit changes the definition of
`VMMemoryDefinition::current_length` to `usize` from its previous
definition of `u32`. This is a pretty impactful change because it also
changes the cranelift semantics of "dynamic" heaps where the bound
global value specifier must now match the pointer type for the platform
rather than the index type for the heap.

The motivation for this change is that the `current_length` field (or
bound for the heap) is intended to reflect the current size of the heap.
This is bound by `usize` on the host platform rather than `u32` or`
u64`. The previous choice of `u32` couldn't represent a 4GB memory
because we couldn't put a number representing 4GB into the
`current_length` field. By using `usize`, which reflects the host's
memory allocation, this should better reflect the size of the heap and
allows Wasmtime to support a full 4GB heap for a wasm program (instead
of 4GB minus one page).

This commit also updates the legalization of the `heap_addr` clif
instruction to appropriately cast the address to the platform's pointer
type, handling bounds checks along the way. The practical impact for
today's targets is that a `uextend` is happening sooner than it happened
before, but otherwise there is no intended impact of this change. In the
future when 64-bit memories are supported there will likely need to be
fancier logic which handles offsets a bit differently (especially in the
case of a 64-bit memory on a 32-bit host).

The clif `filetest` changes should show the differences in codegen, and
the Wasmtime changes are largely removing casts here and there.

Closes #3022

* Add tests for memory.size at maximum memory size

* Add a dfg helper method
This commit is contained in:
Alex Crichton
2021-08-02 13:09:40 -05:00
committed by GitHub
parent 87fefd8a21
commit 63a3bbbf5a
19 changed files with 146 additions and 140 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
cranelift/codegen/src/ir/dfg.rs
View File

@@ -7,8 +7,8 @@ use crate::ir::extfunc::ExtFuncData;
use crate::ir::instructions::{BranchInfo, CallInfo, InstructionData};
use crate::ir::{types, ConstantData, ConstantPool, Immediate};
use crate::ir::{
Block, FuncRef, Inst, SigRef, Signature, Type, Value, ValueLabelAssignments, ValueList,
ValueListPool,
Block, FuncRef, Inst, SigRef, Signature, SourceLoc, Type, Value, ValueLabelAssignments,
ValueList, ValueListPool,
};
use crate::isa::TargetIsa;
use crate::packed_option::ReservedValue;
@@ -153,6 +153,14 @@ impl DataFlowGraph {
self.values_labels = Some(HashMap::new());
}
}
/// Inserts a `ValueLabelAssignments::Alias` for `to_alias` if debug info
/// collection is enabled.
pub fn add_value_label_alias(&mut self, to_alias: Value, from: SourceLoc, value: Value) {
if let Some(values_labels) = self.values_labels.as_mut() {
values_labels.insert(to_alias, ir::ValueLabelAssignments::Alias { from, value });
}
}
}
/// Resolve value aliases.

60
cranelift/codegen/src/legalizer/heap.rs
View File

@@ -64,8 +64,10 @@ fn dynamic_addr(
let mut pos = FuncCursor::new(func).at_inst(inst);
pos.use_srcloc(inst);
let offset = cast_offset_to_pointer_ty(offset, offset_ty, addr_ty, &mut pos);
// Start with the bounds check. Trap if `offset + access_size > bound`.
let bound = pos.ins().global_value(offset_ty, bound_gv);
let bound = pos.ins().global_value(addr_ty, bound_gv);
let (cc, lhs, bound) = if access_size == 1 {
// `offset > bound - 1` is the same as `offset >= bound`.
(IntCC::UnsignedGreaterThanOrEqual, offset, bound)
@@ -76,7 +78,7 @@ fn dynamic_addr(
(IntCC::UnsignedGreaterThan, offset, adj_bound)
} else {
// We need an overflow check for the adjusted offset.
let access_size_val = pos.ins().iconst(offset_ty, access_size as i64);
let access_size_val = pos.ins().iconst(addr_ty, access_size as i64);
let (adj_offset, overflow) = pos.ins().iadd_ifcout(offset, access_size_val);
pos.ins().trapif(
isa.unsigned_add_overflow_condition(),
@@ -100,7 +102,6 @@ fn dynamic_addr(
heap,
addr_ty,
offset,
offset_ty,
pos.func,
spectre_oob_comparison,
);
@@ -111,7 +112,7 @@ fn static_addr(
isa: &dyn TargetIsa,
inst: ir::Inst,
heap: ir::Heap,
offset: ir::Value,
mut offset: ir::Value,
access_size: u32,
bound: u64,
func: &mut ir::Function,
@@ -156,6 +157,7 @@ fn static_addr(
// `bound - access_size >= 4GB` we can omit a bounds check.
let limit = bound - access_size;
let mut spectre_oob_comparison = None;
offset = cast_offset_to_pointer_ty(offset, offset_ty, addr_ty, &mut pos);
if offset_ty != ir::types::I32 || limit < 0xffff_ffff {
let (cc, lhs, limit_imm) = if limit & 1 == 1 {
// Prefer testing `offset >= limit - 1` when limit is odd because an even number is
@@ -169,7 +171,7 @@ fn static_addr(
let oob = pos.ins().icmp_imm(cc, lhs, limit_imm);
pos.ins().trapnz(oob, ir::TrapCode::HeapOutOfBounds);
if isa.flags().enable_heap_access_spectre_mitigation() {
let limit = pos.ins().iconst(offset_ty, limit_imm);
let limit = pos.ins().iconst(addr_ty, limit_imm);
spectre_oob_comparison = Some((cc, lhs, limit));
}
}
@@ -180,20 +182,46 @@ fn static_addr(
heap,
addr_ty,
offset,
offset_ty,
pos.func,
spectre_oob_comparison,
);
}
fn cast_offset_to_pointer_ty(
offset: ir::Value,
offset_ty: ir::Type,
addr_ty: ir::Type,
pos: &mut FuncCursor,
) -> ir::Value {
if offset_ty == addr_ty {
return offset;
}
// Note that using 64-bit heaps on a 32-bit host is not currently supported,
// would require at least a bounds check here to ensure that the truncation
// from 64-to-32 bits doesn't lose any upper bits. For now though we're
// mostly interested in the 32-bit-heaps-on-64-bit-hosts cast.
assert!(offset_ty.bits() < addr_ty.bits());
// Convert `offset` to `addr_ty`.
let extended_offset = pos.ins().uextend(addr_ty, offset);
// Add debug value-label alias so that debuginfo can name the extended
// value as the address
let loc = pos.srcloc();
pos.func
.dfg
.add_value_label_alias(extended_offset, loc, offset);
extended_offset
}
/// Emit code for the base address computation of a `heap_addr` instruction.
fn compute_addr(
isa: &dyn TargetIsa,
inst: ir::Inst,
heap: ir::Heap,
addr_ty: ir::Type,
mut offset: ir::Value,
offset_ty: ir::Type,
offset: ir::Value,
func: &mut ir::Function,
// If we are performing Spectre mitigation with conditional selects, the
// values to compare and the condition code that indicates an out-of bounds
@@ -201,24 +229,10 @@ fn compute_addr(
// speculatively safe address (a zero / null pointer) instead.
spectre_oob_comparison: Option<(IntCC, ir::Value, ir::Value)>,
) {
debug_assert_eq!(func.dfg.value_type(offset), addr_ty);
let mut pos = FuncCursor::new(func).at_inst(inst);
pos.use_srcloc(inst);
// Convert `offset` to `addr_ty`.
if offset_ty != addr_ty {
let labels_value = offset;
offset = pos.ins().uextend(addr_ty, offset);
if let Some(values_labels) = pos.func.dfg.values_labels.as_mut() {
values_labels.insert(
offset,
ir::ValueLabelAssignments::Alias {
from: pos.func.srclocs[inst],
value: labels_value,
},
);
}
}
// Add the heap base address base
let base = if isa.flags().enable_pinned_reg() && isa.flags().use_pinned_reg_as_heap_base() {
pos.ins().get_pinned_reg(isa.pointer_type())

7
cranelift/codegen/src/verifier/mod.rs
View File

@@ -439,14 +439,13 @@ impl<'a> Verifier<'a> {
.nonfatal((heap, format!("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 {
if pointer_type != bound_type {
errors.report((
heap,
format!(
"heap index type {} differs from the type of its bound, {}",
index_type, bound_type
"heap pointer type {} differs from the type of its bound, {}",
pointer_type, bound_type
),
));
}