Repository: bytecodealliance/wasm-tools
Pull request #1423 (2024-02-20 16:38:14)

Add pointer and length types to `WasmType`.
In anticipation of memory64, provenance in Rust, and guest bindings with instrumented pointers, add `Pointer` and `Length` types to `WasmType`, and use it in the ABI for list, strings, and argument/return value pointers. Consumers that don't have anything special to do with these can handle them both the same as `i32`.

And, because the variant type Canonical ABI can unify pointers with integer types like `i64`, add a `Pointer64` type as well, which represents a conceptual union of a pointer and an `i64`. Consumers that don't have anything special to do with this type can handle it the same as an `i64`.

sunfishcode: One awkward thing about this is that we no longer get signature deduping between two types that differ only in `i32` vs. pointer, as seen in the test change here.
pchickey: Can you say more about `provenance in Rust, and guest bindings with instrumented pointers`?

I'm apprehensive that this is the right way to solve this problem - right now the WasmType corresponds to the spec [`Number Types`](https://webassembly.github.io/spec/core/syntax/types.html#number-types) and these pointer and length types don't appear anywhere in the spec. I would need to understand the intended way these new AST nodes are used to give a more concrete suggestion, though.
alexcrichton: Not deduping some types I think is fine, everything is dedup'd at the runtime layer anyway typically. 

One thing that I'd find helpful to review this, which I think might also help address @pchickey's concern, could this be integrated with `wit-bindgen` to see what the changes there might be? I think that's the main consumer of `WasmType` apart from Wasmtime and Wasmtime would probaby look quite similar to `wit-bindgen`
sunfishcode: [Rust is adopting](https://github.com/rust-lang/rfcs/blob/master/text/3559-rust-has-provenance.md) a semantic model where pointers conceptually carry additional "provenance" information which describes what objects may be accessed. We should ideally make the bindings use pointer types like `*mut c_void` instead of `i32` when carrying pointer values at the Rust level. Pointers codegen down to `i32` at the Wasm level, but give the Rust compiler better information at the Rust level.

For instrumented pointers, I'm thinking about systems that use fat-pointer techniques to add spatial or temporal memory safety bookkeeping to pointers. Not everyone will want to do this, but for people who do, the bindings generator will want to know which values are pointers.

Here's a preview of what this looks like it wit-bindgen:

https://github.com/sunfishcode/wit-bindgen/commit/a939252a90195df6faa6a7e219af3c5b563ec697

It's not done yet; in particular, the `RET_AREA` variable needs bigger changes, but it does illustrate the high-level idea.

sunfishcode: `Pointer64` is for when there's eg. a `variant<string, i64>` and the Canonical ABI unifies the `string`'s pointer with the `i64`. Neither `*mut T` nor `i64` is sufficient, because `*mut T` might not be 64-bit, and `i64` doesn't carry the provenance. So it seems we need a `Pointer64` type which is like a union between a pointer and an `i64` and able to carry either.
alexcrichton: Ah I see, I misunderstood `Pointer64`. Perhaps that could be named `PointerOrI64` to help disambiguate a bit?

I'm also not sure how this will be represented in Rust. In the bindings you're representing it currently as `(*mut T, *mut T)` but I'm not sure that'll do the right thing of expanding to two arguments? (or if it does is it guaranteed to do so? I feel like I still haven't lived down "Alex made `wasm32-unknown-unknown` entirely incompatible with C" so I'm hesitant to rely on ABI lowerings as-is in rustc again.
sunfishcode: `PointerOrI64` makes sense.

Yeah, `(*mut T, *mut T)` isn't great. I still need to figure out something to do there, because it turns out `#[repr(transparent)]` on a `union` is unstable. But that's entirely on the wit-bindgen side.

Patch:
diff --git a/crates/wit-component/src/dummy.rs b/crates/wit-component/src/dummy.rs
index 25f6d2b7..5aba44f9 100644
--- a/crates/wit-component/src/dummy.rs
+++ b/crates/wit-component/src/dummy.rs
@@ -128,6 +128,9 @@ pub fn dummy_module(resolve: &Resolve, world: WorldId) -> Vec<u8> {
                 WasmType::I64 => dst.push_str("i64"),
                 WasmType::F32 => dst.push_str("f32"),
                 WasmType::F64 => dst.push_str("f64"),
+                WasmType::Pointer => dst.push_str("i32"),
+                WasmType::PointerOrI64 => dst.push_str("i64"),
+                WasmType::Length => dst.push_str("i32"),
             }
         }
         dst.push(')');
diff --git a/crates/wit-component/src/encoding.rs b/crates/wit-component/src/encoding.rs
index cb5122a1..c4bdb0fe 100644
--- a/crates/wit-component/src/encoding.rs
+++ b/crates/wit-component/src/encoding.rs
@@ -106,6 +106,9 @@ fn to_val_type(ty: &WasmType) -> ValType {
         WasmType::I64 => ValType::I64,
         WasmType::F32 => ValType::F32,
         WasmType::F64 => ValType::F64,
+        WasmType::Pointer => ValType::I32,
+        WasmType::PointerOrI64 => ValType::I64,
+        WasmType::Length => ValType::I32,
     }
 }
 
diff --git a/crates/wit-component/src/encoding/world.rs b/crates/wit-component/src/encoding/world.rs
index 47b90878..e9e2242c 100644
--- a/crates/wit-component/src/encoding/world.rs
+++ b/crates/wit-component/src/encoding/world.rs
@@ -224,6 +224,9 @@ impl<'a> ComponentWorld<'a> {
                 WasmType::I64 => ValType::I64,
                 WasmType::F32 => ValType::F32,
                 WasmType::F64 => ValType::F64,
+                WasmType::Pointer => ValType::I32,
+                WasmType::PointerOrI64 => ValType::I64,
+                WasmType::Length => ValType::I32,
             }
         }
     }
diff --git a/crates/wit-component/src/validation.rs b/crates/wit-component/src/validation.rs
index 0a760124..b410443d 100644
--- a/crates/wit-component/src/validation.rs
+++ b/crates/wit-component/src/validation.rs
@@ -23,6 +23,9 @@ fn wasm_sig_to_func_type(signature: WasmSignature) -> FuncType {
             WasmType::I64 => ValType::I64,
             WasmType::F32 => ValType::F32,
             WasmType::F64 => ValType::F64,
+            WasmType::Pointer => ValType::I32,
+            WasmType::PointerOrI64 => ValType::I64,
+            WasmType::Length => ValType::I32,
         }
     }
 
diff --git a/crates/wit-parser/src/abi.rs b/crates/wit-parser/src/abi.rs
index b542acf0..1c383384 100644
--- a/crates/wit-parser/src/abi.rs
+++ b/crates/wit-parser/src/abi.rs
@@ -1,6 +1,6 @@
 use crate::{Function, Handle, Int, Resolve, Type, TypeDefKind};
 
-/// A raw WebAssembly signature with params and results.
+/// A core WebAssembly signature with params and results.
 #[derive(Clone, Debug, Hash, Eq, PartialEq, PartialOrd, Ord)]
 pub struct WasmSignature {
     /// The WebAssembly parameters of this function.
@@ -31,6 +31,29 @@ pub enum WasmType {
     I64,
     F32,
     F64,
+
+    /// A pointer type. In core Wasm this typically lowers to either `i32` or
+    /// `i64` depending on the index type of the exported linear memory,
+    /// however bindings can use different source-level types to preserve
+    /// provenance.
+    ///
+    /// Users that don't do anything special for pointers can treat this as
+    /// `i32`.
+    Pointer,
+
+    /// A type for values which can be either pointers or 64-bit integers.
+    /// This occurs in variants, when pointers and non-pointers are unified.
+    ///
+    /// Users that don't do anything special for pointers can treat this as
+    /// `i64`.
+    PointerOrI64,
+
+    /// An array length type. In core Wasm this lowers to either `i32` or `i64`
+    /// depending on the index type of the exported linear memory.
+    ///
+    /// Users that don't do anything special for pointers can treat this as
+    /// `i32`.
+    Length,
     // NOTE: we don't lower interface types to any other Wasm type,
     // e.g. externref, so we don't need to define them here.
 }
@@ -39,10 +62,41 @@ fn join(a: WasmType, b: WasmType) -> WasmType {
     use WasmType::*;
 
     match (a, b) {
-        (I32, I32) | (I64, I64) | (F32, F32) | (F64, F64) => a,
+        (I32, I32)
+        | (I64, I64)
+        | (F32, F32)
+        | (F64, F64)
+        | (Pointer, Pointer)
+        | (PointerOrI64, PointerOrI64)
+        | (Length, Length) => a,
 
         (I32, F32) | (F32, I32) => I32,
 
+        // A length is at least an `i32`, maybe more, so it wins over
+        // 32-bit types.
+        (Length, I32 | F32) => Length,
+        (I32 | F32, Length) => Length,
+
+        // A length might be an `i64`, but might not be, so if we have
+        // 64-bit types, they win.
+        (Length, I64 | F64) => I64,
+        (I64 | F64, Length) => I64,
+
+        // Pointers have provenance and are at least an `i32`, so they
+        // win over 32-bit and length types.
+        (Pointer, I32 | F32 | Length) => Pointer,
+        (I32 | F32 | Length, Pointer) => Pointer,
+
+        // If we need 64 bits and provenance, we need to use the special
+        // `PointerOrI64`.
+        (Pointer, I64 | F64) => PointerOrI64,
+        (I64 | F64, Pointer) => PointerOrI64,
+
+        // PointerOrI64 wins over everything.
+        (PointerOrI64, _) => PointerOrI64,
+        (_, PointerOrI64) => PointerOrI64,
+
+        // Otherwise, `i64` wins.
         (_, I64 | F64) | (I64 | F64, _) => I64,
     }
 }
@@ -92,7 +146,7 @@ impl Resolve {
 
         if params.len() > MAX_FLAT_PARAMS {
             params.truncate(0);
-            params.push(WasmType::I32);
+            params.push(WasmType::Pointer);
             indirect_params = true;
         }
 
@@ -112,10 +166,10 @@ impl Resolve {
             results.truncate(0);
             match variant {
                 AbiVariant::GuestImport => {
-                    params.push(WasmType::I32);
+                    params.push(WasmType::Pointer);
                 }
                 AbiVariant::GuestExport => {
-                    results.push(WasmType::I32);
+                    results.push(WasmType::Pointer);
                 }
             }
         }
@@ -145,8 +199,8 @@ impl Resolve {
             Type::Float32 => result.push(WasmType::F32),
             Type::Float64 => result.push(WasmType::F64),
             Type::String => {
-                result.push(WasmType::I32);
-                result.push(WasmType::I32);
+                result.push(WasmType::Pointer);
+                result.push(WasmType::Length);
             }
 
             Type::Id(id) => match &self.types[*id].kind {
@@ -177,8 +231,8 @@ impl Resolve {
                 }
 
                 TypeDefKind::List(_) => {
-                    result.push(WasmType::I32);
-                    result.push(WasmType::I32);
+                    result.push(WasmType::Pointer);
+                    result.push(WasmType::Length);
                 }
 
                 TypeDefKind::Variant(v) => {


Test patch:
diff --git a/crates/wit-component/tests/components/adapt-memory-simple/component.wat b/crates/wit-component/tests/components/adapt-memory-simple/component.wat
index 59e9e163..f05ce418 100644
--- a/crates/wit-component/tests/components/adapt-memory-simple/component.wat
+++ b/crates/wit-component/tests/components/adapt-memory-simple/component.wat
@@ -23,17 +23,18 @@
   )
   (core module (;2;)
     (type (;0;) (func (param i32 i32)))
+    (type (;1;) (func (param i32 i32)))
     (func $indirect-new-log (;0;) (type 0) (param i32 i32)
       local.get 0
       local.get 1
       i32.const 0
       call_indirect (type 0)
     )
-    (func $adapt-old-log (;1;) (type 0) (param i32 i32)
+    (func $adapt-old-log (;1;) (type 1) (param i32 i32)
       local.get 0
       local.get 1
       i32.const 1
-      call_indirect (type 0)
+      call_indirect (type 1)
     )
     (table (;0;) 2 2 funcref)
     (export "0" (func $indirect-new-log))
@@ -45,8 +46,9 @@
   )
   (core module (;3;)
     (type (;0;) (func (param i32 i32)))
+    (type (;1;) (func (param i32 i32)))
     (import "" "0" (func (;0;) (type 0)))
-    (import "" "1" (func (;1;) (type 0)))
+    (import "" "1" (func (;1;) (type 1)))
     (import "" "$imports" (table (;0;) 2 2 funcref))
     (elem (;0;) (i32.const 0) func 0 1)
     (@producers