Restrict all context.{get,set} in same component to use same elem type (#645)

* Restrict all context.{get,set} in same component to use same elem type
* Tweak Binary.md rules to use validation, instead of decoding, to gate i32/i64

Author: lukewagner

design/mvp/Binary.md

@@ -214,12 +214,9 @@ label'        ::= len:<u32> l:<label>                     => l    (if len = |l|)
                 | 0x01 t:<T>                              => t
 valtype       ::= i:<typeidx>                             => i
                 | pvt:<primvaltype>                       => pvt
-resourcetype  ::= 0x3f 0x7f f?:<core:funcidx>?            => (resource (rep i32) (dtor f)?)
-                | 0x3e 0x7f f:<core:funcidx>
-                            cb?:<core:funcidx>?           => (resource (rep i32) (dtor async f (callback cb)?)) 🚝
-                | 0x3f 0x7e f?:<core:funcidx>?            => (resource (rep i64) (dtor f)?) 🐘
-                | 0x3e 0x7e f:<core:funcidx>
-                            cb?:<core:funcidx>?           => (resource (rep i64) (dtor async f (callback cb)?)) 🚝🐘
+resourcetype  ::= 0x3f v:<valtype> f?:<core:funcidx>?     => (resource (rep v) (dtor f)?)
+                | 0x3e v:<valtype> f:<core:funcidx>
+                                   cb?:<core:funcidx>?    => (resource (rep v) (dtor async f (callback cb)?)) 🚝
 functype      ::= 0x40 ps:<paramlist> rs:<resultlist>     => (func ps rs)
                 | 0x43 ps:<paramlist> rs:<resultlist>     => (func async ps rs)
 paramlist     ::= lt*:vec(<labelvaltype>)                 => (param lt)*
@@ -302,10 +299,8 @@ canon    ::= 0x00 0x00 f:<core:funcidx> opts:<opts> ft:<typeidx> => (canon lift
            | 0x25                                                => (canon backpressure.dec (core func)) 🔀
            | 0x09 rs:<resultlist> opts:<opts>                    => (canon task.return rs opts (core func)) 🔀
            | 0x05                                                => (canon task.cancel (core func)) 🔀
-           | 0x0a 0x7f i:<u32>                                   => (canon context.get i32 i (core func)) 🔀
-           | 0x0a 0x7e i:<u32>                                   => (canon context.get i64 i (core func)) 🔀🐘
-           | 0x0b 0x7f i:<u32>                                   => (canon context.set i32 i (core func)) 🔀
-           | 0x0b 0x7e i:<u32>                                   => (canon context.set i64 i (core func)) 🔀🐘
+           | 0x0a v:<valtype> i:<u32>                            => (canon context.get v i (core func)) 🔀
+           | 0x0b v:<valtype> i:<u32>                            => (canon context.set v i (core func)) 🔀
            | 0x0c cancel?:<cancel?>                              => (canon thread.yield cancel? (core func)) 🔀
            | 0x06 async?:<async?>                                => (canon subtask.cancel async? (core func)) 🔀
            | 0x0d                                                => (canon subtask.drop (core func)) 🔀

design/mvp/CanonicalABI.md

@@ -3965,22 +3965,20 @@ For a canonical definition:
 ```
 validation specifies:
 * `$t` must be `i32` (see [here][thread-local storage]).
-  * 🐘 - `$t` may also be `i64`
+  * 🐘 - `$t` may also be `i64`. All `context.get` and `context.set` built-ins
+    defined in a single component must specify the same `$t`.
 * `$i` must be less than `2`
 * `$f` is given type `(func (result $t))`
 
 Calling `$f` invokes the following function, which reads the [thread-local
-storage] of the [current thread], taking only the low 32-bits if `$t` is `i32`:
+storage] of the [current thread].
 ```python
-MASK_32BIT = (1 << 32) - 1
-
 def canon_context_get(t, i):
   thread = current_thread()
   assert(t == 'i32' or t == 'i64')
   assert(i < len(thread.storage))
   result = thread.storage[i]
-  if t == 'i32':
-    result &= MASK_32BIT
+  assert(result < (2 ** (ptr_size(t) * 8)))
   return [result]
 ```
 
@@ -3993,7 +3991,8 @@ For a canonical definition:
 ```
 validation specifies:
 * `$t` must be `i32` (see [here][thread-local storage])
-  * 🐘 - `$t` may also be `i64`
+  * 🐘 - `$t` may also be `i64`. All `context.get` and `context.set` built-ins
+    defined in a single component must specify the same `$t`.
 * `$i` must be less than `2`
 * `$f` is given type `(func (param $v $t))`
 
@@ -4003,8 +4002,8 @@ storage] of the [current thread]:
 def canon_context_set(t, i, v):
   thread = current_thread()
   assert(t == 'i32' or t == 'i64')
-  assert(v <= MASK_32BIT or t == 'i64')
   assert(i < len(thread.storage))
+  assert(v < (2 ** (ptr_size(t) * 8)))
   thread.storage[i] = v
   return []
 ```

design/mvp/Concurrency.md

@@ -421,7 +421,7 @@ current thread's thread-local storage can be read and written from core wasm
 code by calling the [`context.get`] and [`context.set`] built-ins.
 
 The thread-local storage array's length is currently fixed to contain exactly
-2 `i64`s with the goal of allowing this array to be stored inline in whatever
+2 elements with the goal of allowing this array to be stored inline in whatever
 existing runtime data structure is already efficiently reachable from ambient
 compiled wasm code. Because module instantiation is declarative in the
 Component Model, the imported `context.{get,set}` built-ins can be inlined by
@@ -433,11 +433,11 @@ natural place to store:
    thread-local features
 
 Both of `context.{get,set}` take an immediate argument of `i32` or `i64` to
-indicate the return or argument type. `context.set i32` will zero the high
-bits of the stored value and `context.get i32` will only read the low bits of
-the stored value.  Generally it is expected that 32-bit components always use
-the `i32` immediate and 64-bit components always use the `i64` immediate, but
-mixing these calls is still valid.
+indicate the return or argument type. As part of component-level validation, all
+`context.{get,set}` definitions within a single component are required to
+specify the *same* thread-local element type, so that there is no mixing of
+types between loads and stores. This restriction would allow Core WebAssembly
+reference types to be used as thread-local storage element types in the future.
 
 When threads are created explicitly by `thread.new-indirect`, the lifetime of
 the thread-local storage array ends when the function passed to

design/mvp/Explainer.md

@@ -1566,14 +1566,9 @@ See the [concurrency explainer] for background.
 The `context.get` built-in returns the `i`th element of the [current thread]'s
 [thread-local storage] array. Validation currently restricts `i` to be less
 than 2 and `T` to be `i32` (or, with 🐘, `i64`), but these restrictions may
-be relaxed in the future.
-
-Mixing `i32` and `i64` results in truncating or unsigned extending the
-stored values:
-* If `context.get i32 i` is called after `context.set i64 i v`,
-  only the low 32-bits are read (returning `i32.wrap_i64 v`).
-* If `context.get i64 i` is called after `context.set i32 i v`,
-  the upper 32-bits will be zero (returning `i64.extend_i32_u v`).
+be relaxed in the future. Additionally, component-level validation requires
+that all `context.get` and `context.set` built-ins use the *same* `T`, so
+that there is no mixing of writes with one type and reads with another.
 
 For details, see [Thread-Local Storage] in the concurrency explainer and
 [`canon_context_get`] in the Canonical ABI explainer.
@@ -1588,14 +1583,10 @@ For details, see [Thread-Local Storage] in the concurrency explainer and
 The `context.set` built-in sets the `i`th element of the [current thread]'s
 [thread-local storage] array to the value `v`. Validation currently restricts
 `i` to be less than 2 and `T` to be `i32` (or, with 🐘, `i64`), but these
-restrictions may be relaxed in the future.
-
-Mixing `i32` and `i64` results in truncating or unsigned extending the
-stored values:
-* If `context.get i32 i` is called after `context.set i64 i v`,
-  only the low 32-bits are read (returning `i32.wrap_i64 v`).
-* If `context.get i64 i` is called after `context.set i32 i v`,
-  the upper 32-bits will be zero (returning `i64.extend_i32_u v`).
+restrictions may be relaxed in the future. Additionally, component-level
+validation requires that all `context.get` and `context.set` built-ins use the
+*same* `T`, so that there is no mixing of writes with one type and reads with
+another.
 
 For details, see [Thread-Local Storage] in the concurrency explainer and
 [`canon_context_set`] in the Canonical ABI explainer.

design/mvp/canonical-abi/definitions.py

@@ -2268,24 +2268,22 @@ def canon_resource_rep(rt, i):
   return [h.rep]
 
 ### 🔀 `canon context.get`
-MASK_32BIT = (1 << 32) - 1
 
 def canon_context_get(t, i):
   thread = current_thread()
   assert(t == 'i32' or t == 'i64')
   assert(i < len(thread.storage))
   result = thread.storage[i]
-  if t == 'i32':
-    result &= MASK_32BIT
+  assert(result < (2 ** (ptr_size(t) * 8)))
   return [result]
 
 ### 🔀 `canon context.set`
 
 def canon_context_set(t, i, v):
   thread = current_thread()
   assert(t == 'i32' or t == 'i64')
-  assert(v <= MASK_32BIT or t == 'i64')
   assert(i < len(thread.storage))
+  assert(v < (2 ** (ptr_size(t) * 8)))
   thread.storage[i] = v
   return []