Use 128 bit fat pointers for continuation objects (bytecodealliance#186)

This PR changes the representation introduced in bytecodealliance#182 , where
continuation objects were turned into tagged pointers, containing a
pointer to a `VMContRef` as well as a 16bit sequence counter to perform
linearity checks.

In this PR, the representation is changed from 64bit tagged pointers to
128bit fat pointers, where 64bit are used for the pointer and the
sequence counter.

Some implementation details:
- The design introduced in bytecodealliance#182, where we use `disassemble_contobj` and
`assemble_contobj` to go from effectively `Optional<VMContObj>` to
`Optional<VMContRef>` is preserved.
- The feature `unsafe_disable_continuation_linearity_check` is
preserved: If it is enabled, we do not use fat (or tagged) pointers at
all, and all revision checks are disabled.
- Overflow checks for the revision counter are no longer necessary and
have been removed.
- In wasm, we now use the SIMD type `I8X16` for any value of type `(ref
$continuation)` and `(ref null $continuation)`. See the comment on
`vm_contobj_type` in shared.rs for why we cannot use `I128` or `I64X2`
instead.
- Some `translate_*` functions in the `FuncEnvironment` trait now need
to take a `FunctionBuilder` parameter, instead of `FuncCursor`, which
slightly increases the footprint of this PR.
- The implementation of `table.fill` for continuation tables was
missing. I've added this and in the process extended `cont_table.wast`
to be generally more exhaustive.
- For those libcalls that take a parameter that is a variant type
including `VMContObj`, I've introduced dedicated versions for the
`VMContObj` case, namely `table_fill_cont_obj` and `table_grow_cont_obj`
in libcalls.rs. These manually split the `VMContObj` into two parts.

.github/workflows/main.yml

@@ -692,7 +692,7 @@ jobs:
     # `unsafe_disable_continuation_linearity_check` makes the test
     # `cont_twice` fail.
     - run: |
-        (cargo test --features=unsafe_disable_continuation_linearity_check --test wast -- --exact Cranelift/tests/misc_testsuite/typed-continuations/cont_twice.wast && test $? -eq 101) || test $? -eq 101
+        (cargo run --features=unsafe_disable_continuation_linearity_check -- wast -W=exceptions,function-references,typed-continuations tests/misc_testsuite/typed-continuations/cont_twice.wast && test $? -eq 101) || test $? -eq 101
 
     # NB: the test job here is explicitly lacking in cancellation of this run if
     # something goes wrong. These take the longest anyway and otherwise if

cranelift/wasm/src/code_translator.rs

@@ -1228,11 +1228,11 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
         }
         Operator::RefNull { hty } => {
             let hty = environ.convert_heap_type(*hty);
-            state.push1(environ.translate_ref_null(builder.cursor(), hty)?)
+            state.push1(environ.translate_ref_null(builder, hty)?)
         }
         Operator::RefIsNull => {
             let value = state.pop1();
-            state.push1(environ.translate_ref_is_null(builder.cursor(), value)?);
+            state.push1(environ.translate_ref_is_null(builder, value)?);
         }
         Operator::RefFunc { function_index } => {
             let index = FuncIndex::from_u32(*function_index);
@@ -1553,12 +1553,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             let table_index = TableIndex::from_u32(*index);
             let delta = state.pop1();
             let init_value = state.pop1();
-            state.push1(environ.translate_table_grow(
-                builder.cursor(),
-                table_index,
-                delta,
-                init_value,
-            )?);
+            state.push1(environ.translate_table_grow(builder, table_index, delta, init_value)?);
         }
         Operator::TableGet { table: index } => {
             let table_index = TableIndex::from_u32(*index);
@@ -1592,7 +1587,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             let len = state.pop1();
             let val = state.pop1();
             let dest = state.pop1();
-            environ.translate_table_fill(builder.cursor(), table_index, dest, val, len)?;
+            environ.translate_table_fill(builder, table_index, dest, val, len)?;
         }
         Operator::TableInit {
             elem_index,
@@ -2433,7 +2428,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
         Operator::BrOnNull { relative_depth } => {
             let r = state.pop1();
             let (br_destination, inputs) = translate_br_if_args(*relative_depth, state);
-            let is_null = environ.translate_ref_is_null(builder.cursor(), r)?;
+            let is_null = environ.translate_ref_is_null(builder, r)?;
             let else_block = builder.create_block();
             canonicalise_brif(builder, is_null, br_destination, inputs, else_block, &[]);
 
@@ -2448,7 +2443,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             // Peek the value val from the stack.
             // If val is ref.null ht, then: pop the value val from the stack.
             // Else: Execute the instruction (br relative_depth).
-            let is_null = environ.translate_ref_is_null(builder.cursor(), state.peek1())?;
+            let is_null = environ.translate_ref_is_null(builder, state.peek1())?;
             let (br_destination, inputs) = translate_br_if_args(*relative_depth, state);
             let else_block = builder.create_block();
             canonicalise_brif(builder, is_null, else_block, &[], br_destination, inputs);
@@ -2487,7 +2482,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
         }
         Operator::RefAsNonNull => {
             let r = state.pop1();
-            let is_null = environ.translate_ref_is_null(builder.cursor(), r)?;
+            let is_null = environ.translate_ref_is_null(builder, r)?;
             builder.ins().trapnz(is_null, ir::TrapCode::NullReference);
             state.push1(r);
         }

cranelift/wasm/src/environ/spec.rs

@@ -366,7 +366,7 @@ pub trait FuncEnvironment: TargetEnvironment {
     /// Translate a `table.grow` WebAssembly instruction.
     fn translate_table_grow(
         &mut self,
-        pos: FuncCursor,
+        builder: &mut FunctionBuilder,
         table_index: TableIndex,
         delta: ir::Value,
         init_value: ir::Value,
@@ -403,7 +403,7 @@ pub trait FuncEnvironment: TargetEnvironment {
     /// Translate a `table.fill` WebAssembly instruction.
     fn translate_table_fill(
         &mut self,
-        pos: FuncCursor,
+        builder: &mut FunctionBuilder,
         table_index: TableIndex,
         dst: ir::Value,
         val: ir::Value,
@@ -435,11 +435,11 @@ pub trait FuncEnvironment: TargetEnvironment {
     /// `translate_ref_is_null` as well.
     fn translate_ref_null(
         &mut self,
-        mut pos: FuncCursor,
+        builder: &mut FunctionBuilder,
         ty: WasmHeapType,
     ) -> WasmResult<ir::Value> {
         let _ = ty;
-        Ok(pos.ins().null(self.reference_type(ty)))
+        Ok(builder.ins().null(self.reference_type(ty)))
     }
 
     /// Translate a `ref.is_null` WebAssembly instruction.
@@ -452,11 +452,11 @@ pub trait FuncEnvironment: TargetEnvironment {
     /// `translate_ref_null` as well.
     fn translate_ref_is_null(
         &mut self,
-        mut pos: FuncCursor,
+        builder: &mut FunctionBuilder,
         value: ir::Value,
     ) -> WasmResult<ir::Value> {
-        let is_null = pos.ins().is_null(value);
-        Ok(pos.ins().uextend(ir::types::I32, is_null))
+        let is_null = builder.ins().is_null(value);
+        Ok(builder.ins().uextend(ir::types::I32, is_null))
     }
 
     /// Translate a `ref.func` WebAssembly instruction.

cranelift/wasm/src/func_translator.rs

@@ -220,7 +220,7 @@ fn declare_locals<FE: FuncEnvironment + ?Sized>(
             let hty = environ.convert_heap_type(rt.heap_type());
             let ty = environ.reference_type(hty);
             let init = if rt.is_nullable() {
-                Some(environ.translate_ref_null(builder.cursor(), hty)?)
+                Some(environ.translate_ref_null(builder, hty)?)
             } else {
                 None
             };

crates/cranelift/src/func_environ.rs

@@ -26,9 +26,9 @@ use wasmtime_environ::{FUNCREF_INIT_BIT, FUNCREF_MASK};
 
 cfg_if::cfg_if! {
     if #[cfg(feature = "wasmfx_baseline")] {
-        use crate::wasmfx::baseline as wasmfx_impl;
+        pub (crate) use crate::wasmfx::baseline as wasmfx_impl;
     } else {
-        use crate::wasmfx::optimized as wasmfx_impl;
+        pub(crate) use crate::wasmfx::optimized as wasmfx_impl;
     }
 }
 
@@ -1707,34 +1707,45 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
 
     fn translate_table_grow(
         &mut self,
-        mut pos: cranelift_codegen::cursor::FuncCursor<'_>,
+        builder: &mut FunctionBuilder,
         table_index: TableIndex,
         delta: ir::Value,
         init_value: ir::Value,
     ) -> WasmResult<ir::Value> {
         let ty = self.module.table_plans[table_index].table.wasm_ty.heap_type;
+        let vmctx = self.vmctx_val(&mut builder.cursor());
+        let table_index_arg = builder.ins().iconst(I32, table_index.as_u32() as i64);
+
+        let mut args = vec![vmctx, table_index_arg, delta];
         let grow = if ty.is_vmgcref_type() {
-            gc::gc_ref_table_grow_builtin(ty, self, &mut pos.func)?
+            args.push(init_value);
+            gc::gc_ref_table_grow_builtin(ty, self, &mut builder.func)?
         } else {
             debug_assert!(matches!(
                 ty.top(),
                 WasmHeapTopType::Func | WasmHeapTopType::Cont
             ));
             match ty.top() {
-                WasmHeapTopType::Func => self.builtin_functions.table_grow_func_ref(&mut pos.func),
-                WasmHeapTopType::Cont => self.builtin_functions.table_grow_cont_obj(&mut pos.func),
+                WasmHeapTopType::Func => {
+                    args.push(init_value);
+                    self.builtin_functions
+                        .table_grow_func_ref(&mut builder.func)
+                }
+                WasmHeapTopType::Cont => {
+                    let (revision, contref) =
+                        wasmfx_impl::disassemble_contobj(self, builder, init_value);
+                    args.extend_from_slice(&[contref, revision]);
+                    self.builtin_functions
+                        .table_grow_cont_obj(&mut builder.func)
+                }
+
                 _ => panic!("unsupported table type."),
             }
         };
 
-        let vmctx = self.vmctx_val(&mut pos);
-
-        let table_index_arg = pos.ins().iconst(I32, table_index.as_u32() as i64);
-        let call_inst = pos
-            .ins()
-            .call(grow, &[vmctx, table_index_arg, delta, init_value]);
+        let call_inst = builder.ins().call(grow, &args);
 
-        Ok(pos.func.dfg.first_result(call_inst))
+        Ok(builder.func.dfg.first_result(call_inst))
     }
 
     fn translate_table_get(
@@ -1802,7 +1813,12 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
                         pointer_type,
                         self.isa.flags().enable_table_access_spectre_mitigation(),
                     );
-                    Ok(builder.ins().load(pointer_type, flags, table_entry_addr, 0))
+                    Ok(builder.ins().load(
+                        wasmfx_impl::vm_contobj_type(self.pointer_type()),
+                        flags,
+                        table_entry_addr,
+                        0,
+                    ))
                 }
             },
         }
@@ -1897,28 +1913,38 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
 
     fn translate_table_fill(
         &mut self,
-        mut pos: cranelift_codegen::cursor::FuncCursor<'_>,
+        builder: &mut FunctionBuilder,
         table_index: TableIndex,
         dst: ir::Value,
         val: ir::Value,
         len: ir::Value,
     ) -> WasmResult<()> {
         let ty = self.module.table_plans[table_index].table.wasm_ty.heap_type;
+        let vmctx = self.vmctx_val(&mut builder.cursor());
+        let table_index_arg = builder.ins().iconst(I32, table_index.as_u32() as i64);
+        let mut args = vec![vmctx, table_index_arg, dst];
         let libcall = if ty.is_vmgcref_type() {
-            gc::gc_ref_table_fill_builtin(ty, self, &mut pos.func)?
+            args.push(val);
+            gc::gc_ref_table_fill_builtin(ty, self, &mut builder.func)?
         } else {
-            debug_assert!(matches!(
-                ty.top(),
-                WasmHeapTopType::Func | WasmHeapTopType::Cont
-            ));
-            self.builtin_functions.table_fill_func_ref(&mut pos.func)
+            match ty.top() {
+                WasmHeapTopType::Func => {
+                    args.push(val);
+                    self.builtin_functions
+                        .table_fill_func_ref(&mut builder.func)
+                }
+                WasmHeapTopType::Cont => {
+                    let (revision, contref) = wasmfx_impl::disassemble_contobj(self, builder, val);
+                    args.extend_from_slice(&[contref, revision]);
+                    self.builtin_functions
+                        .table_fill_cont_obj(&mut builder.func)
+                }
+                _ => panic!("unsupported table type"),
+            }
         };
+        args.push(len);
 
-        let vmctx = self.vmctx_val(&mut pos);
-
-        let table_index_arg = pos.ins().iconst(I32, table_index.as_u32() as i64);
-        pos.ins()
-            .call(libcall, &[vmctx, table_index_arg, dst, val, len]);
+        builder.ins().call(libcall, &args);
 
         Ok(())
     }
@@ -1960,35 +1986,47 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
 
     fn translate_ref_null(
         &mut self,
-        mut pos: cranelift_codegen::cursor::FuncCursor,
+        builder: &mut FunctionBuilder,
         ht: WasmHeapType,
     ) -> WasmResult<ir::Value> {
         Ok(match ht.top() {
-            WasmHeapTopType::Func => pos.ins().iconst(self.pointer_type(), 0),
-            WasmHeapTopType::Cont => pos.ins().iconst(self.pointer_type(), 0),
+            WasmHeapTopType::Func => builder.ins().iconst(self.pointer_type(), 0),
+            WasmHeapTopType::Cont => {
+                let zero = builder.ins().iconst(self.pointer_type(), 0);
+                // TODO do this nicer
+                wasmfx_impl::assemble_contobj(self, builder, zero, zero)
+            }
             WasmHeapTopType::Any | WasmHeapTopType::Extern => {
-                pos.ins().null(self.reference_type(ht))
+                builder.ins().null(self.reference_type(ht))
             }
         })
     }
 
     fn translate_ref_is_null(
         &mut self,
-        mut pos: cranelift_codegen::cursor::FuncCursor,
+        builder: &mut FunctionBuilder,
         value: ir::Value,
     ) -> WasmResult<ir::Value> {
-        let bool_is_null = match pos.func.dfg.value_type(value) {
+        let bool_is_null = match builder.func.dfg.value_type(value) {
             // `externref`
-            ty if ty.is_ref() => pos.ins().is_null(value),
-            // `funcref`
+            ty if ty.is_ref() => builder.ins().is_null(value),
+            // `funcref` or continuation (if not using fat pointers)
             ty if ty == self.pointer_type() => {
-                pos.ins()
+                builder
+                    .ins()
                     .icmp_imm(cranelift_codegen::ir::condcodes::IntCC::Equal, value, 0)
             }
+            // continuation
+            ty if ty == wasmfx_impl::vm_contobj_type(self.pointer_type()) => {
+                let (_revision, contref) = wasmfx_impl::disassemble_contobj(self, builder, value);
+                builder
+                    .ins()
+                    .icmp_imm(cranelift_codegen::ir::condcodes::IntCC::Equal, contref, 0)
+            }
             _ => unreachable!(),
         };
 
-        Ok(pos.ins().uextend(ir::types::I32, bool_is_null))
+        Ok(builder.ins().uextend(ir::types::I32, bool_is_null))
     }
 
     fn translate_ref_func(

crates/cranelift/src/lib.rs

@@ -217,7 +217,8 @@ fn wasm_call_signature(
 /// Returns the reference type to use for the provided wasm type.
 fn reference_type(wasm_ht: WasmHeapType, pointer_type: ir::Type) -> ir::Type {
     match wasm_ht.top() {
-        WasmHeapTopType::Func | WasmHeapTopType::Cont => pointer_type,
+        WasmHeapTopType::Func => pointer_type,
+        WasmHeapTopType::Cont => func_environ::wasmfx_impl::vm_contobj_type(pointer_type),
         WasmHeapTopType::Any | WasmHeapTopType::Extern => match pointer_type {
             ir::types::I32 => ir::types::R32,
             ir::types::I64 => ir::types::R64,

crates/cranelift/src/wasmfx/baseline.rs

@@ -8,9 +8,11 @@ use cranelift_codegen::ir::InstBuilder;
 use cranelift_frontend::{FunctionBuilder, Switch};
 use cranelift_wasm::FuncEnvironment;
 use cranelift_wasm::{FuncTranslationState, WasmResult, WasmValType};
-use shared::{assemble_contobj, disassemble_contobj};
 use wasmtime_environ::PtrSize;
 
+#[cfg_attr(not(feature = "wasmfx_baseline"), allow(unused_imports))]
+pub(crate) use shared::{assemble_contobj, disassemble_contobj, vm_contobj_type};
+
 fn get_revision<'a>(
     _env: &mut crate::func_environ::FuncEnvironment<'a>,
     builder: &mut FunctionBuilder,
@@ -42,10 +44,6 @@ fn compare_revision_and_increment<'a>(
             .trapz(evidence, ir::TrapCode::ContinuationAlreadyConsumed);
 
         let revision_plus1 = builder.ins().iadd_imm(revision, 1);
-        let overflow = builder
-            .ins()
-            .icmp_imm(IntCC::UnsignedLessThan, revision_plus1, 1 << 16);
-        builder.ins().trapz(overflow, ir::TrapCode::IntegerOverflow); // TODO(dhil): Consider introducing a designated trap code.
         builder.ins().store(mem_flags, revision_plus1, contref, 0);
         revision_plus1
     }