Skip to content

Commit

Permalink
Wasm GC: Fix is-null-or-i31ref checks (#10221)
Browse files Browse the repository at this point in the history 
In the case where we need to check for either null or i31refs, we were
attempting to fold the two checks together with a bitwise-and and a bitmask, but
that was incorrect and buggy. We need to actually do the two separate checks and
bitwise-or them together.

Fixes #10171
  • Loading branch information
@fitzgen
fitzgen authored Feb 12, 2025
1 parent b5b8257 commit c6b4eaf
Show file tree
Hide file tree
Showing 17 changed files with 221 additions and 206 deletions.
23 changes: 9 additions & 14 deletions crates/cranelift/src/gc/enabled.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ use cranelift_codegen::{
ir::{self, condcodes::IntCC, InstBuilder},
};
use cranelift_entity::packed_option::ReservedValue;
use cranelift_entity::Signed;
use cranelift_frontend::FunctionBuilder;
use smallvec::SmallVec;
use wasmtime_environ::{
wasm_unsupported, Collector, GcArrayLayout, GcLayout, GcStructLayout, ModuleInternedTypeIndex,
PtrSize, TypeIndex, VMGcKind, WasmHeapTopType, WasmHeapType, WasmRefType, WasmResult,
WasmStorageType, WasmValType, I31_DISCRIMINANT, NON_NULL_NON_I31_MASK,
WasmStorageType, WasmValType, I31_DISCRIMINANT,
};

#[cfg(feature = "gc-drc")]
Expand Down Expand Up @@ -1031,7 +1032,7 @@ pub fn translate_ref_test(
builder,
val,
Offset::Static(wasmtime_environ::VM_GC_HEADER_TYPE_INDEX_OFFSET),
BoundsCheck::Access(wasmtime_environ::VM_GC_HEADER_SIZE),
BoundsCheck::Access(func_env.offsets.size_of_vmshared_type_index().into()),
);
let actual_shared_ty = builder.ins().load(
ir::types::I32,
Expand Down Expand Up @@ -1361,6 +1362,7 @@ impl FuncEnvironment<'_> {
ty: WasmRefType,
gc_ref: ir::Value,
) -> ir::Value {
assert_eq!(builder.func.dfg.value_type(gc_ref), ir::types::I32);
assert!(ty.is_vmgcref_type_and_not_i31());

let might_be_i31 = match ty.heap_type {
Expand Down Expand Up @@ -1399,25 +1401,18 @@ impl FuncEnvironment<'_> {
(false, false) => builder.ins().iconst(ir::types::I32, 0),

// This GC reference is always non-null, but might be an i31.
(false, true) => builder.ins().band_imm(gc_ref, I31_DISCRIMINANT as i64),
(false, true) => builder.ins().band_imm(gc_ref, I31_DISCRIMINANT.signed()),

// This GC reference might be null, but can never be an i31.
(true, false) => builder.ins().icmp_imm(IntCC::Equal, gc_ref, 0),

// Fully general case: this GC reference could be either null or an
// i31.
(true, true) => {
// Mask for checking whether any bits are set, other than the
// `i31ref` discriminant, which should not be set. This folds
// the null and i31ref checks together into a single `band`.
let mask = builder.ins().iconst(
ir::types::I32,
(NON_NULL_NON_I31_MASK & u32::MAX as u64) as i64,
);
let is_non_null_and_non_i31 = builder.ins().band(gc_ref, mask);
builder
.ins()
.icmp_imm(ir::condcodes::IntCC::Equal, is_non_null_and_non_i31, 0)
let is_i31 = builder.ins().band_imm(gc_ref, I31_DISCRIMINANT.signed());
let is_null = builder.ins().icmp_imm(IntCC::Equal, gc_ref, 0);
let is_null = builder.ins().uextend(ir::types::I32, is_null);
builder.ins().bor(is_i31, is_null)
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions crates/environ/src/gc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ use core::alloc::Layout;
/// Discriminant to check whether GC reference is an `i31ref` or not.
pub const I31_DISCRIMINANT: u64 = 1;

/// A mask that can be used to check for non-null and non-i31ref GC references
/// with a single bitwise-and operation.
pub const NON_NULL_NON_I31_MASK: u64 = !I31_DISCRIMINANT;

/// The size of the `VMGcHeader` in bytes.
pub const VM_GC_HEADER_SIZE: u32 = 8;

Expand Down
3 changes: 1 addition & 2 deletions tests/disas/gc/drc/br-on-cast-fail.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@
;; @002e v17 = uextend.i64 v2
;; @002e v18 = iconst.i64 4
;; @002e v19 = uadd_overflow_trap v17, v18, user1 ; v18 = 4
;; @002e v20 = iconst.i64 8
;; @002e v21 = uadd_overflow_trap v19, v20, user1 ; v20 = 8
;; @002e v21 = uadd_overflow_trap v19, v18, user1 ; v18 = 4
;; @002e v16 = load.i64 notrap aligned readonly v0+48
;; @002e v22 = icmp ule v21, v16
;; @002e trapz v22, user1
Expand Down
3 changes: 1 addition & 2 deletions tests/disas/gc/drc/br-on-cast.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@
;; @002f v17 = uextend.i64 v2
;; @002f v18 = iconst.i64 4
;; @002f v19 = uadd_overflow_trap v17, v18, user1 ; v18 = 4
;; @002f v20 = iconst.i64 8
;; @002f v21 = uadd_overflow_trap v19, v20, user1 ; v20 = 8
;; @002f v21 = uadd_overflow_trap v19, v18, user1 ; v18 = 4
;; @002f v16 = load.i64 notrap aligned readonly v0+48
;; @002f v22 = icmp ule v21, v16
;; @002f trapz v22, user1
Expand Down
3 changes: 1 addition & 2 deletions tests/disas/gc/drc/ref-cast.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
;; @001e v17 = uextend.i64 v2
;; @001e v18 = iconst.i64 4
;; @001e v19 = uadd_overflow_trap v17, v18, user1 ; v18 = 4
;; @001e v20 = iconst.i64 8
;; @001e v21 = uadd_overflow_trap v19, v20, user1 ; v20 = 8
;; @001e v21 = uadd_overflow_trap v19, v18, user1 ; v18 = 4
;; @001e v16 = load.i64 notrap aligned readonly v0+48
;; @001e v22 = icmp ule v21, v16
;; @001e trapz v22, user1
Expand Down
3 changes: 1 addition & 2 deletions tests/disas/gc/drc/ref-test-concrete-type.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
;; @001d v17 = uextend.i64 v2
;; @001d v18 = iconst.i64 4
;; @001d v19 = uadd_overflow_trap v17, v18, user1 ; v18 = 4
;; @001d v20 = iconst.i64 8
;; @001d v21 = uadd_overflow_trap v19, v20, user1 ; v20 = 8
;; @001d v21 = uadd_overflow_trap v19, v18, user1 ; v18 = 4
;; @001d v16 = load.i64 notrap aligned readonly v0+48
;; @001d v22 = icmp ule v21, v16
;; @001d trapz v22, user1
Expand Down
68 changes: 35 additions & 33 deletions tests/disas/gc/drc/struct-get.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,56 +118,58 @@
;; @004e v9 = uextend.i64 v2
;; @004e v10 = iconst.i64 24
;; @004e v11 = uadd_overflow_trap v9, v10, user1 ; v10 = 24
;; v68 = iconst.i64 32
;; @004e v13 = uadd_overflow_trap v9, v68, user1 ; v68 = 32
;; v72 = iconst.i64 32
;; @004e v13 = uadd_overflow_trap v9, v72, user1 ; v72 = 32
;; @004e v8 = load.i64 notrap aligned readonly v0+48
;; @004e v14 = icmp ule v13, v8
;; @004e trapz v14, user1
;; @004e v6 = load.i64 notrap aligned readonly v0+40
;; @004e v15 = iadd v6, v11
;; @004e v16 = load.i32 notrap aligned little v15
;; v58 = stack_addr.i64 ss0
;; store notrap v16, v58
;; @004e v17 = iconst.i32 -2
;; @004e v18 = band v16, v17 ; v17 = -2
;; v60 = iconst.i32 0
;; @004e v19 = icmp eq v18, v60 ; v60 = 0
;; @004e brif v19, block5, block2
;; v60 = stack_addr.i64 ss0
;; store notrap v16, v60
;; v62 = iconst.i32 1
;; @004e v17 = band v16, v62 ; v62 = 1
;; v64 = iconst.i32 0
;; @004e v18 = icmp eq v16, v64 ; v64 = 0
;; @004e v19 = uextend.i32 v18
;; @004e v20 = bor v17, v19
;; @004e brif v20, block5, block2
;;
;; block2:
;; @004e v21 = load.i64 notrap aligned readonly v0+56
;; @004e v22 = load.i64 notrap aligned v21
;; @004e v23 = load.i64 notrap aligned v21+8
;; @004e v24 = icmp eq v22, v23
;; @004e brif v24, block3, block4
;; @004e v22 = load.i64 notrap aligned readonly v0+56
;; @004e v23 = load.i64 notrap aligned v22
;; @004e v24 = load.i64 notrap aligned v22+8
;; @004e v25 = icmp eq v23, v24
;; @004e brif v25, block3, block4
;;
;; block4:
;; @004e v29 = uextend.i64 v16
;; @004e v30 = iconst.i64 8
;; @004e v31 = uadd_overflow_trap v29, v30, user1 ; v30 = 8
;; @004e v33 = uadd_overflow_trap v31, v30, user1 ; v30 = 8
;; @004e v34 = icmp ule v33, v8
;; @004e trapz v34, user1
;; @004e v35 = iadd.i64 v6, v31
;; @004e v36 = load.i64 notrap aligned v35
;; v62 = iconst.i64 1
;; @004e v37 = iadd v36, v62 ; v62 = 1
;; @004e store notrap aligned v37, v35
;; v54 = load.i32 notrap v58
;; @004e store notrap aligned v54, v22
;; v65 = iconst.i64 4
;; @004e v49 = iadd.i64 v22, v65 ; v65 = 4
;; @004e store notrap aligned v49, v21
;; @004e v30 = uextend.i64 v16
;; @004e v31 = iconst.i64 8
;; @004e v32 = uadd_overflow_trap v30, v31, user1 ; v31 = 8
;; @004e v34 = uadd_overflow_trap v32, v31, user1 ; v31 = 8
;; @004e v35 = icmp ule v34, v8
;; @004e trapz v35, user1
;; @004e v36 = iadd.i64 v6, v32
;; @004e v37 = load.i64 notrap aligned v36
;; v66 = iconst.i64 1
;; @004e v38 = iadd v37, v66 ; v66 = 1
;; @004e store notrap aligned v38, v36
;; v55 = load.i32 notrap v60
;; @004e store notrap aligned v55, v23
;; v69 = iconst.i64 4
;; @004e v50 = iadd.i64 v23, v69 ; v69 = 4
;; @004e store notrap aligned v50, v22
;; @004e jump block5
;;
;; block3 cold:
;; @004e v51 = call fn0(v0, v16), stack_map=[i32 @ ss0+0]
;; @004e v52 = call fn0(v0, v16), stack_map=[i32 @ ss0+0]
;; @004e jump block5
;;
;; block5:
;; v52 = load.i32 notrap v58
;; v53 = load.i32 notrap v60
;; @0052 jump block1
;;
;; block1:
;; @0052 return v52
;; @0052 return v53
;; }
44 changes: 22 additions & 22 deletions tests/disas/gc/drc/struct-new-default.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,35 +31,35 @@
;; @0021 v12 = ireduce.i32 v11
;; @0021 v15 = uextend.i64 v12
;; @0021 v16 = iadd v14, v15
;; v47 = iconst.i64 16
;; @0021 v17 = iadd v16, v47 ; v47 = 16
;; v48 = iconst.i64 16
;; @0021 v17 = iadd v16, v48 ; v48 = 16
;; @0021 store notrap aligned little v3, v17 ; v3 = 0.0
;; v48 = iconst.i64 20
;; @0021 v18 = iadd v16, v48 ; v48 = 20
;; v49 = iconst.i64 20
;; @0021 v18 = iadd v16, v49 ; v49 = 20
;; @0021 istore8 notrap aligned little v4, v18 ; v4 = 0
;; v58 = iconst.i8 1
;; @0021 brif v58, block3, block2 ; v58 = 1
;; v51 = iconst.i32 1
;; @0021 brif v51, block3, block2 ; v51 = 1
;;
;; block2:
;; v65 = iconst.i64 0
;; @0021 v28 = iconst.i64 8
;; @0021 v29 = uadd_overflow_trap v65, v28, user1 ; v65 = 0, v28 = 8
;; @0021 v31 = uadd_overflow_trap v29, v28, user1 ; v28 = 8
;; @0021 v26 = load.i64 notrap aligned readonly v0+48
;; @0021 v32 = icmp ule v31, v26
;; @0021 trapz v32, user1
;; @0021 v33 = iadd.i64 v14, v29
;; @0021 v34 = load.i64 notrap aligned v33
;; v51 = iconst.i64 1
;; @0021 v35 = iadd v34, v51 ; v51 = 1
;; @0021 store notrap aligned v35, v33
;; v73 = iconst.i64 0
;; @0021 v29 = iconst.i64 8
;; @0021 v30 = uadd_overflow_trap v73, v29, user1 ; v73 = 0, v29 = 8
;; @0021 v32 = uadd_overflow_trap v30, v29, user1 ; v29 = 8
;; @0021 v27 = load.i64 notrap aligned readonly v0+48
;; @0021 v33 = icmp ule v32, v27
;; @0021 trapz v33, user1
;; @0021 v34 = iadd.i64 v14, v30
;; @0021 v35 = load.i64 notrap aligned v34
;; v53 = iconst.i64 1
;; @0021 v36 = iadd v35, v53 ; v53 = 1
;; @0021 store notrap aligned v36, v34
;; @0021 jump block3
;;
;; block3:
;; v66 = iconst.i32 0
;; v49 = iconst.i64 24
;; @0021 v19 = iadd.i64 v16, v49 ; v49 = 24
;; @0021 store notrap aligned little v66, v19 ; v66 = 0
;; v74 = iconst.i32 0
;; v50 = iconst.i64 24
;; @0021 v19 = iadd.i64 v16, v50 ; v50 = 24
;; @0021 store notrap aligned little v74, v19 ; v74 = 0
;; @0024 jump block1
;;
;; block1:
Expand Down
56 changes: 29 additions & 27 deletions tests/disas/gc/drc/struct-new.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
;; stack_limit = gv2
;;
;; block0(v0: i64, v1: i64, v2: f32, v3: i32, v4: i32):
;; v51 = stack_addr.i64 ss0
;; store notrap v4, v51
;; v53 = stack_addr.i64 ss0
;; store notrap v4, v53
;; @002a v8 = iconst.i32 -1342177280
;; @002a v9 = iconst.i32 0
;; @002a v6 = iconst.i32 32
Expand All @@ -33,38 +33,40 @@
;; @002a v12 = ireduce.i32 v11
;; @002a v15 = uextend.i64 v12
;; @002a v16 = iadd v14, v15
;; v52 = iconst.i64 16
;; @002a v17 = iadd v16, v52 ; v52 = 16
;; v54 = iconst.i64 16
;; @002a v17 = iadd v16, v54 ; v54 = 16
;; @002a store notrap aligned little v2, v17
;; v53 = iconst.i64 20
;; @002a v18 = iadd v16, v53 ; v53 = 20
;; v55 = iconst.i64 20
;; @002a v18 = iadd v16, v55 ; v55 = 20
;; @002a istore8 notrap aligned little v3, v18
;; v50 = load.i32 notrap v51
;; @002a v20 = iconst.i32 -2
;; @002a v21 = band v50, v20 ; v20 = -2
;; @002a v22 = icmp eq v21, v9 ; v9 = 0
;; @002a brif v22, block3, block2
;; v52 = load.i32 notrap v53
;; v58 = iconst.i32 1
;; @002a v20 = band v52, v58 ; v58 = 1
;; @002a v21 = icmp eq v52, v9 ; v9 = 0
;; @002a v22 = uextend.i32 v21
;; @002a v23 = bor v20, v22
;; @002a brif v23, block3, block2
;;
;; block2:
;; @002a v27 = uextend.i64 v50
;; @002a v28 = iconst.i64 8
;; @002a v29 = uadd_overflow_trap v27, v28, user1 ; v28 = 8
;; @002a v31 = uadd_overflow_trap v29, v28, user1 ; v28 = 8
;; @002a v26 = load.i64 notrap aligned readonly v0+48
;; @002a v32 = icmp ule v31, v26
;; @002a trapz v32, user1
;; @002a v33 = iadd.i64 v14, v29
;; @002a v34 = load.i64 notrap aligned v33
;; v58 = iconst.i64 1
;; @002a v35 = iadd v34, v58 ; v58 = 1
;; @002a store notrap aligned v35, v33
;; @002a v28 = uextend.i64 v52
;; @002a v29 = iconst.i64 8
;; @002a v30 = uadd_overflow_trap v28, v29, user1 ; v29 = 8
;; @002a v32 = uadd_overflow_trap v30, v29, user1 ; v29 = 8
;; @002a v27 = load.i64 notrap aligned readonly v0+48
;; @002a v33 = icmp ule v32, v27
;; @002a trapz v33, user1
;; @002a v34 = iadd.i64 v14, v30
;; @002a v35 = load.i64 notrap aligned v34
;; v62 = iconst.i64 1
;; @002a v36 = iadd v35, v62 ; v62 = 1
;; @002a store notrap aligned v36, v34
;; @002a jump block3
;;
;; block3:
;; v47 = load.i32 notrap v51
;; v54 = iconst.i64 24
;; @002a v19 = iadd.i64 v16, v54 ; v54 = 24
;; @002a store notrap aligned little v47, v19
;; v48 = load.i32 notrap v53
;; v56 = iconst.i64 24
;; @002a v19 = iadd.i64 v16, v56 ; v56 = 24
;; @002a store notrap aligned little v48, v19
;; @002d jump block1
;;
;; block1:
Expand Down
Loading

0 comments on commit c6b4eaf

Please sign in to comment.