Skip to content

Commit b93e1bc

Browse files
authored
winch: Gracefully handle compilation errors (bytecodealliance#9851)
* winch: Gracefully handle compilation errors Closes: bytecodealliance#8096 This commit threads `anyhow::Result` through most of Winch's compilation process in order to gracefully handle compilation errors gracefully instead of panicking. The error classification is intentionally very granular, to avoid string allocation which could impact compilation performance. The errors are largely fit in two categories: * Unimplemented/Unsupported * Internal The firs category signals partial or no support for Wasmtime features and or Wasm proposals. These errors are meant to be temporary while such features or proposals are in development. The second category signals that a compilation invariant was not met. These errors are considered internal and their presence usually means a bug in the compiler. * Include `Result` in the MacroAssembler This commit updates the MacroAssembler trait to require returning `Result<T>` on every method in the interface, making it easier to detect partial support for Masm instructions.
1 parent 5ca3715 commit b93e1bc

File tree

17 files changed

+2556
-1783
lines changed

17 files changed

+2556
-1783
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

winch/codegen/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ regalloc2 = { workspace = true }
2525
gimli = { workspace = true }
2626
wasmtime-environ = { workspace = true }
2727
wasmtime-cranelift = { workspace = true }
28+
thiserror = { workspace = true }
2829

2930
[features]
3031
x64 = ["cranelift-codegen/x86"]

winch/codegen/src/codegen/bounds.rs

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::{
99
masm::{IntCmpKind, MacroAssembler, OperandSize, RegImm, TrapCode},
1010
stack::TypedReg,
1111
};
12+
use anyhow::Result;
1213
use wasmtime_environ::Signed;
1314

1415
/// A newtype to represent an immediate offset argument for a heap access.
@@ -86,30 +87,33 @@ pub(crate) fn load_dynamic_heap_bounds<M>(
8687
masm: &mut M,
8788
heap: &HeapData,
8889
ptr_size: OperandSize,
89-
) -> Bounds
90+
) -> Result<Bounds>
9091
where
9192
M: MacroAssembler,
9293
{
93-
let dst = context.any_gpr(masm);
94+
let dst = context.any_gpr(masm)?;
9495
match heap.memory.static_heap_size() {
9596
// Constant size, no need to perform a load.
96-
Some(size) => masm.mov(writable!(dst), RegImm::i64(size.signed()), ptr_size),
97+
Some(size) => masm.mov(writable!(dst), RegImm::i64(size.signed()), ptr_size)?,
9798

9899
None => {
99100
let scratch = scratch!(M);
100101
let base = if let Some(offset) = heap.import_from {
101-
let addr = masm.address_at_vmctx(offset);
102-
masm.load_ptr(addr, writable!(scratch));
102+
let addr = masm.address_at_vmctx(offset)?;
103+
masm.load_ptr(addr, writable!(scratch))?;
103104
scratch
104105
} else {
105106
vmctx!(M)
106107
};
107-
let addr = masm.address_at_reg(base, heap.current_length_offset);
108-
masm.load_ptr(addr, writable!(dst));
108+
let addr = masm.address_at_reg(base, heap.current_length_offset)?;
109+
masm.load_ptr(addr, writable!(dst))?;
109110
}
110111
}
111112

112-
Bounds::from_typed_reg(TypedReg::new(heap.index_type(), dst))
113+
Ok(Bounds::from_typed_reg(TypedReg::new(
114+
heap.index_type(),
115+
dst,
116+
)))
113117
}
114118

115119
/// This function ensures the following:
@@ -125,10 +129,10 @@ pub(crate) fn ensure_index_and_offset<M: MacroAssembler>(
125129
index: Index,
126130
offset: u64,
127131
heap_ty_size: OperandSize,
128-
) -> ImmOffset {
132+
) -> Result<ImmOffset> {
129133
match u32::try_from(offset) {
130134
// If the immediate offset fits in a u32, then we simply return.
131-
Ok(offs) => ImmOffset::from_u32(offs),
135+
Ok(offs) => Ok(ImmOffset::from_u32(offs)),
132136
// Else we adjust the index to be index = index + offset, including an
133137
// overflow check, and return 0 as the offset.
134138
Err(_) => {
@@ -138,9 +142,9 @@ pub(crate) fn ensure_index_and_offset<M: MacroAssembler>(
138142
RegImm::i64(offset as i64),
139143
heap_ty_size,
140144
TrapCode::HEAP_OUT_OF_BOUNDS,
141-
);
145+
)?;
142146

143-
ImmOffset::from_u32(0)
147+
Ok(ImmOffset::from_u32(0))
144148
}
145149
}
146150
}
@@ -157,28 +161,28 @@ pub(crate) fn load_heap_addr_checked<M, F>(
157161
index: Index,
158162
offset: ImmOffset,
159163
mut emit_check_condition: F,
160-
) -> Reg
164+
) -> Result<Reg>
161165
where
162166
M: MacroAssembler,
163-
F: FnMut(&mut M, Bounds, Index) -> IntCmpKind,
167+
F: FnMut(&mut M, Bounds, Index) -> Result<IntCmpKind>,
164168
{
165-
let cmp_kind = emit_check_condition(masm, bounds, index);
169+
let cmp_kind = emit_check_condition(masm, bounds, index)?;
166170

167-
masm.trapif(cmp_kind, TrapCode::HEAP_OUT_OF_BOUNDS);
168-
let addr = context.any_gpr(masm);
171+
masm.trapif(cmp_kind, TrapCode::HEAP_OUT_OF_BOUNDS)?;
172+
let addr = context.any_gpr(masm)?;
169173

170-
load_heap_addr_unchecked(masm, heap, index, offset, addr, ptr_size);
174+
load_heap_addr_unchecked(masm, heap, index, offset, addr, ptr_size)?;
171175
if !enable_spectre_mitigation {
172-
addr
176+
Ok(addr)
173177
} else {
174178
// Conditionally assign 0 to the register holding the base address if
175179
// the comparison kind is met.
176-
let tmp = context.any_gpr(masm);
177-
masm.mov(writable!(tmp), RegImm::i64(0), ptr_size);
178-
let cmp_kind = emit_check_condition(masm, bounds, index);
179-
masm.cmov(writable!(addr), tmp, cmp_kind, ptr_size);
180+
let tmp = context.any_gpr(masm)?;
181+
masm.mov(writable!(tmp), RegImm::i64(0), ptr_size)?;
182+
let cmp_kind = emit_check_condition(masm, bounds, index)?;
183+
masm.cmov(writable!(addr), tmp, cmp_kind, ptr_size)?;
180184
context.free_reg(tmp);
181-
addr
185+
Ok(addr)
182186
}
183187
}
184188

@@ -192,14 +196,15 @@ pub(crate) fn load_heap_addr_unchecked<M>(
192196
offset: ImmOffset,
193197
dst: Reg,
194198
ptr_size: OperandSize,
195-
) where
199+
) -> Result<()>
200+
where
196201
M: MacroAssembler,
197202
{
198203
let base = if let Some(offset) = heap.import_from {
199204
// If the WebAssembly memory is imported, load the address into
200205
// the scratch register.
201206
let scratch = scratch!(M);
202-
masm.load_ptr(masm.address_at_vmctx(offset), writable!(scratch));
207+
masm.load_ptr(masm.address_at_vmctx(offset)?, writable!(scratch))?;
203208
scratch
204209
} else {
205210
// Else if the WebAssembly memory is defined in the current module,
@@ -208,17 +213,18 @@ pub(crate) fn load_heap_addr_unchecked<M>(
208213
};
209214

210215
// Load the base of the memory into the `addr` register.
211-
masm.load_ptr(masm.address_at_reg(base, heap.offset), writable!(dst));
216+
masm.load_ptr(masm.address_at_reg(base, heap.offset)?, writable!(dst))?;
212217
// Start by adding the index to the heap base addr.
213218
let index_reg = index.as_typed_reg().reg;
214-
masm.add(writable!(dst), dst, index_reg.into(), ptr_size);
219+
masm.add(writable!(dst), dst, index_reg.into(), ptr_size)?;
215220

216221
if offset.as_u32() > 0 {
217222
masm.add(
218223
writable!(dst),
219224
dst,
220225
RegImm::i64(offset.as_u32() as i64),
221226
ptr_size,
222-
);
227+
)?;
223228
}
229+
Ok(())
224230
}

0 commit comments

Comments
 (0)