Skip to content

Commit d3f752c

Browse files
committed
Perform (ref null none) loads that can trap
As an optimization, we were previously avoiding the load and instead simply materializing the null reference value, since that is the only value that inhabits `(ref null none)`. However, in certain situations, such as when loading from a table when Spectre mitigations are enabled, we rely on that load being attempted and resulting in a trap if the table index is out of bounds. This commit makes it so that we will always perform the load if our given `ir::MemFlags` can trap. Fixes bytecodealliance#10353
1 parent 8357599 commit d3f752c

File tree

2 files changed

+27
-0
lines changed

2 files changed

+27
-0
lines changed

crates/cranelift/src/gc/enabled/drc.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,13 +436,29 @@ impl GcCompiler for DrcCompiler {
436436
// null, or we are in dynamically unreachable code and should just trap.
437437
if let WasmHeapType::None = ty.heap_type {
438438
let null = builder.ins().iconst(reference_type, 0);
439+
440+
// If the `flags` can trap, then we need to do an actual load. We
441+
// might be relying on, e.g., this load trapping to raise a
442+
// out-of-bounds-table-index trap, rather than successfully loading
443+
// a null `noneref`.
444+
//
445+
// That said, while we will do the load, we won't use the loaded
446+
// value, and will still use our null constant below. This will
447+
// avoid an unnecessary load dependency, slightly improving the code
448+
// we ultimately emit. This probably doesn't matter, but it is easy
449+
// to do and can only improve things, so we do it.
450+
if flags.trap_code().is_some() {
451+
let _ = builder.ins().load(reference_type, flags, src, 0);
452+
}
453+
439454
if !ty.nullable {
440455
// NB: Don't use an unconditional trap instruction, since that
441456
// is a block terminator, and we still need to integrate with
442457
// the rest of the surrounding code.
443458
let zero = builder.ins().iconst(ir::types::I32, 0);
444459
builder.ins().trapz(zero, TRAP_INTERNAL_ASSERT);
445460
}
461+
446462
return Ok(null);
447463
};
448464

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
;;! gc = true
2+
3+
(module
4+
(table $t 10 (ref null none))
5+
(func (export "f") (result (ref null none))
6+
(i32.const 99)
7+
(table.get $t)
8+
)
9+
)
10+
11+
(assert_trap (invoke "f") "out of bounds table access")

0 commit comments

Comments
 (0)