From 4cd8b61c5d4817f753a54ef9f501db66969e310f Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Tue, 25 Jun 2024 09:35:17 -0700 Subject: [threads] Validate shared-to-unshared edges in heap types (#6698) Add spec tests checking validation for structs and arrays. --- src/wasm-type.h | 2 + src/wasm/wasm-type.cpp | 69 +++++++++++++++++++++++--------- test/lit/passes/type-merging-shared.wast | 8 ++-- test/spec/shared-array.wast | 69 ++++++++++++++++++++++++++++++++ test/spec/shared-struct.wast | 69 ++++++++++++++++++++++++++++++++ 5 files changed, 195 insertions(+), 22 deletions(-) create mode 100644 test/spec/shared-array.wast create mode 100644 test/spec/shared-struct.wast diff --git a/src/wasm-type.h b/src/wasm-type.h index 95c3605a5..51d945b53 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -671,6 +671,8 @@ struct TypeBuilder { ForwardChildReference, // A continuation reference that does not refer to a function type. InvalidFuncType, + // A non-shared field of a shared heap type. + InvalidUnsharedField, }; struct Error { diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index ae6fd4b30..92c68d8be 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -1697,6 +1697,8 @@ std::ostream& operator<<(std::ostream& os, TypeBuilder::ErrorReason reason) { return os << "Heap type has an undeclared child"; case TypeBuilder::ErrorReason::InvalidFuncType: return os << "Continuation has invalid function type"; + case TypeBuilder::ErrorReason::InvalidUnsharedField: + return os << "Heap type has an invalid unshared field"; } WASM_UNREACHABLE("Unexpected error reason"); } @@ -2628,6 +2630,53 @@ bool isValidSupertype(const HeapTypeInfo& sub, const HeapTypeInfo& super) { WASM_UNREACHABLE("unknown kind"); } +std::optional +validateType(HeapTypeInfo& info, std::unordered_set& seenTypes) { + if (auto* super = info.supertype) { + // The supertype must be canonical (i.e. defined in a previous rec group) + // or have already been defined in this rec group. + if (super->isTemp && !seenTypes.count(HeapType(uintptr_t(super)))) { + return TypeBuilder::ErrorReason::ForwardSupertypeReference; + } + // The supertype must have a valid structure. + if (!isValidSupertype(info, *super)) { + return TypeBuilder::ErrorReason::InvalidSupertype; + } + } + if (info.isContinuation()) { + if (!info.continuation.type.isSignature()) { + return TypeBuilder::ErrorReason::InvalidFuncType; + } + } + if (info.share == Shared) { + switch (info.kind) { + case HeapTypeInfo::SignatureKind: + // TODO: Figure out and enforce shared function rules. + break; + case HeapTypeInfo::ContinuationKind: + if (!info.continuation.type.isShared()) { + return TypeBuilder::ErrorReason::InvalidFuncType; + } + break; + case HeapTypeInfo::StructKind: + for (auto& field : info.struct_.fields) { + if (field.type.isRef() && !field.type.getHeapType().isShared()) { + return TypeBuilder::ErrorReason::InvalidUnsharedField; + } + } + break; + case HeapTypeInfo::ArrayKind: { + auto elem = info.array.element.type; + if (elem.isRef() && !elem.getHeapType().isShared()) { + return TypeBuilder::ErrorReason::InvalidUnsharedField; + } + break; + } + } + } + return std::nullopt; +} + void updateReferencedHeapTypes( std::unique_ptr& info, const std::unordered_map& canonicalized) { @@ -2695,24 +2744,8 @@ buildRecGroup(std::unique_ptr&& groupInfo, std::unordered_set seenTypes; for (size_t i = 0; i < typeInfos.size(); ++i) { auto& info = typeInfos[i]; - if (auto* super = info->supertype) { - // The supertype must be canonical (i.e. defined in a previous rec group) - // or have already been defined in this rec group. - if (super->isTemp && !seenTypes.count(HeapType(uintptr_t(super)))) { - return {TypeBuilder::Error{ - i, TypeBuilder::ErrorReason::ForwardSupertypeReference}}; - } - // The supertype must have a valid structure. - if (!isValidSupertype(*info, *super)) { - return { - TypeBuilder::Error{i, TypeBuilder::ErrorReason::InvalidSupertype}}; - } - } - if (info->isContinuation()) { - if (!info->continuation.type.isSignature()) { - return { - TypeBuilder::Error{i, TypeBuilder::ErrorReason::InvalidFuncType}}; - } + if (auto err = validateType(*info, seenTypes)) { + return {TypeBuilder::Error{i, *err}}; } seenTypes.insert(asHeapType(info)); } diff --git a/test/lit/passes/type-merging-shared.wast b/test/lit/passes/type-merging-shared.wast index 2457cd572..e02815079 100644 --- a/test/lit/passes/type-merging-shared.wast +++ b/test/lit/passes/type-merging-shared.wast @@ -79,11 +79,11 @@ (module ;; Shared and unshared basic heap types similarly cannot be merged. ;; CHECK: (rec - ;; CHECK-NEXT: (type $A' (shared (struct (field anyref)))) + ;; CHECK-NEXT: (type $A' (struct (field anyref))) - ;; CHECK: (type $A (shared (struct (field (ref null (shared any)))))) - (type $A (shared (struct (ref null (shared any))))) - (type $A' (shared (struct (ref null any)))) + ;; CHECK: (type $A (struct (field (ref null (shared any))))) + (type $A (struct (ref null (shared any)))) + (type $A' (struct (ref null any))) ;; CHECK: (type $2 (func)) diff --git a/test/spec/shared-array.wast b/test/spec/shared-array.wast new file mode 100644 index 000000000..a687c1d36 --- /dev/null +++ b/test/spec/shared-array.wast @@ -0,0 +1,69 @@ +;; Shared array declaration syntax +(module + (type (shared (array i8))) + (type (sub final (shared (array i8)))) + (rec + (type (sub final (shared (array i8)))) + ) + + (global (ref 0) (array.new_default 1 (i32.const 1))) + (global (ref 1) (array.new_default 2 (i32.const 1))) + (global (ref 2) (array.new_default 0 (i32.const 1))) +) + +;; Shared arrays are distinct from non-shared arrays +(assert_invalid + (module + (type (shared (array i8))) + (type (array i8)) + + (global (ref 0) (array.new_default 1 (i32.const 1))) + ) + "not a subtype" +) + +(assert_invalid + (module + (type (shared (array i8))) + (type (array i8)) + + (global (ref 1) (array.new 0)) + ) + "not a subtype" +) + +;; Shared arrays may not be subtypes of non-shared arrays +(assert_invalid + (module + (type (sub (array i8))) + (type (sub 0 (shared (array i8)))) + ) + "invalid supertype" +) + +;; Non-shared arrays may not be subtypes of shared arrays +(assert_invalid + (module + (type (sub (shared (array i8)))) + (type (sub 0 (array i8))) + ) + "invalid supertype" +) + +;; Shared arrays may not contain non-shared references +(assert_invalid + (module + (type (shared (array anyref))) + ) + "invalid field" +) + +;; But they may contain shared references +(module + (type (shared (array (ref null (shared any))))) +) + +;; Non-shared arrays may contain shared references +(module + (type (array (ref null (shared any)))) +) diff --git a/test/spec/shared-struct.wast b/test/spec/shared-struct.wast new file mode 100644 index 000000000..b2c82caff --- /dev/null +++ b/test/spec/shared-struct.wast @@ -0,0 +1,69 @@ +;; Shared struct declaration syntax +(module + (type (shared (struct))) + (type (sub final (shared (struct)))) + (rec + (type (sub final (shared (struct)))) + ) + + (global (ref 0) (struct.new 1)) + (global (ref 1) (struct.new 2)) + (global (ref 2) (struct.new 0)) +) + +;; Shared structs are distinct from non-shared structs +(assert_invalid + (module + (type (shared (struct))) + (type (struct)) + + (global (ref 0) (struct.new 1)) + ) + "not a subtype" +) + +(assert_invalid + (module + (type (shared (struct))) + (type (struct)) + + (global (ref 1) (struct.new 0)) + ) + "not a subtype" +) + +;; Shared structs may not be subtypes of non-shared structs +(assert_invalid + (module + (type (sub (struct))) + (type (sub 0 (shared (struct)))) + ) + "invalid supertype" +) + +;; Non-shared structs may not be subtypes of shared structs +(assert_invalid + (module + (type (sub (shared (struct)))) + (type (sub 0 (struct))) + ) + "invalid supertype" +) + +;; Shared structs may not contain non-shared references +(assert_invalid + (module + (type (shared (struct anyref))) + ) + "invalid field" +) + +;; But they may contain shared references +(module + (type (shared (struct (ref null (shared any))))) +) + +;; Non-shared structs may contain shared references +(module + (type (struct (ref null (shared any)))) +) -- cgit v1.2.3