Skip to content

Commit 29f0070

Browse files
authored
Validate the use of shared types (#8302)
We started validating that shared-everything is enabled when we defined shared types in #8298, but this missed the case where a non-shared type definition used a shared abstract heap type, which has no definition. Update the validation to check that the used types are allowed by the enabled feature set as well. Refactor the validation logic into several functions to avoid duplication of logic.
1 parent 2fe287c commit 29f0070

3 files changed

Lines changed: 91 additions & 36 deletions

File tree

src/wasm/wasm-type.cpp

Lines changed: 78 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,9 +2426,66 @@ bool isValidSupertype(const HeapTypeInfo& sub, const HeapTypeInfo& super) {
24262426
}
24272427

24282428
std::optional<TypeBuilder::ErrorReason>
2429-
validateType(HeapTypeInfo& info,
2430-
std::unordered_set<HeapType>& seenTypes,
2431-
FeatureSet features) {
2429+
validateType(Type type, FeatureSet feats, bool isShared) {
2430+
if (type.isRef()) {
2431+
auto heapType = type.getHeapType();
2432+
if (isShared && !heapType.isShared()) {
2433+
return TypeBuilder::ErrorReason::InvalidUnsharedField;
2434+
}
2435+
if (heapType.isShared() && !feats.hasSharedEverything()) {
2436+
return TypeBuilder::ErrorReason::InvalidSharedType;
2437+
}
2438+
}
2439+
return std::nullopt;
2440+
}
2441+
2442+
std::optional<TypeBuilder::ErrorReason>
2443+
validateStruct(const Struct& struct_, FeatureSet feats, bool isShared) {
2444+
for (auto& field : struct_.fields) {
2445+
if (auto err = validateType(field.type, feats, isShared)) {
2446+
return err;
2447+
}
2448+
}
2449+
return std::nullopt;
2450+
}
2451+
2452+
std::optional<TypeBuilder::ErrorReason>
2453+
validateArray(Array array, FeatureSet feats, bool isShared) {
2454+
return validateType(array.element.type, feats, isShared);
2455+
}
2456+
2457+
std::optional<TypeBuilder::ErrorReason>
2458+
validateSignature(Signature sig, FeatureSet feats, bool isShared) {
2459+
// Allow unshared parameters and results even in shared functions.
2460+
// TODO: Figure out and enforce shared function rules.
2461+
for (auto t : sig.params) {
2462+
if (auto err = validateType(t, feats, /*isShared=*/false)) {
2463+
return err;
2464+
}
2465+
}
2466+
for (auto t : sig.results) {
2467+
if (auto err = validateType(t, feats, /*isShared*/ false)) {
2468+
return err;
2469+
}
2470+
}
2471+
return std::nullopt;
2472+
}
2473+
2474+
std::optional<TypeBuilder::ErrorReason>
2475+
validateContinuation(Continuation cont, FeatureSet feats, bool isShared) {
2476+
if (!cont.type.isSignature()) {
2477+
return TypeBuilder::ErrorReason::InvalidFuncType;
2478+
}
2479+
if (isShared != cont.type.isShared()) {
2480+
return TypeBuilder::ErrorReason::InvalidFuncType;
2481+
}
2482+
return std::nullopt;
2483+
}
2484+
2485+
std::optional<TypeBuilder::ErrorReason>
2486+
validateTypeInfo(HeapTypeInfo& info,
2487+
std::unordered_set<HeapType>& seenTypes,
2488+
FeatureSet features) {
24322489
if (auto* super = info.supertype) {
24332490
// The supertype must be canonical (i.e. defined in a previous rec group)
24342491
// or have already been defined in this rec group.
@@ -2466,11 +2523,6 @@ validateType(HeapTypeInfo& info,
24662523
return TypeBuilder::ErrorReason::MismatchedDescriptor;
24672524
}
24682525
}
2469-
if (info.isContinuation()) {
2470-
if (!info.continuation.type.isSignature()) {
2471-
return TypeBuilder::ErrorReason::InvalidFuncType;
2472-
}
2473-
}
24742526
if (info.share == Shared) {
24752527
if (!features.hasSharedEverything()) {
24762528
return TypeBuilder::ErrorReason::InvalidSharedType;
@@ -2481,32 +2533,23 @@ validateType(HeapTypeInfo& info,
24812533
if (info.descriptor && info.descriptor->share != Shared) {
24822534
return TypeBuilder::ErrorReason::InvalidUnsharedDescriptor;
24832535
}
2484-
switch (info.kind) {
2485-
case HeapTypeKind::Func:
2486-
// TODO: Figure out and enforce shared function rules.
2487-
break;
2488-
case HeapTypeKind::Cont:
2489-
if (!info.continuation.type.isShared()) {
2490-
return TypeBuilder::ErrorReason::InvalidFuncType;
2491-
}
2492-
break;
2493-
case HeapTypeKind::Struct:
2494-
for (auto& field : info.struct_.fields) {
2495-
if (field.type.isRef() && !field.type.getHeapType().isShared()) {
2496-
return TypeBuilder::ErrorReason::InvalidUnsharedField;
2497-
}
2498-
}
2499-
break;
2500-
case HeapTypeKind::Array: {
2501-
auto elem = info.array.element.type;
2502-
if (elem.isRef() && !elem.getHeapType().isShared()) {
2503-
return TypeBuilder::ErrorReason::InvalidUnsharedField;
2504-
}
2505-
break;
2506-
}
2507-
case HeapTypeKind::Basic:
2508-
WASM_UNREACHABLE("unexpected kind");
2509-
}
2536+
}
2537+
bool isShared = info.share == Shared;
2538+
switch (info.kind) {
2539+
case HeapTypeKind::Func:
2540+
return validateSignature(info.signature, features, isShared);
2541+
break;
2542+
case HeapTypeKind::Cont:
2543+
return validateContinuation(info.continuation, features, isShared);
2544+
break;
2545+
case HeapTypeKind::Struct:
2546+
return validateStruct(info.struct_, features, isShared);
2547+
break;
2548+
case HeapTypeKind::Array:
2549+
return validateArray(info.array, features, isShared);
2550+
break;
2551+
case HeapTypeKind::Basic:
2552+
WASM_UNREACHABLE("unexpected kind");
25102553
}
25112554
return std::nullopt;
25122555
}
@@ -2590,7 +2633,7 @@ buildRecGroup(std::unique_ptr<RecGroupInfo>&& groupInfo,
25902633
std::unordered_set<HeapType> seenTypes;
25912634
for (size_t i = 0; i < typeInfos.size(); ++i) {
25922635
auto& info = typeInfos[i];
2593-
if (auto err = validateType(*info, seenTypes, features)) {
2636+
if (auto err = validateTypeInfo(*info, seenTypes, features)) {
25942637
return {TypeBuilder::Error{i, *err}};
25952638
}
25962639
seenTypes.insert(asHeapType(info));

test/gtest/type-domains.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,7 @@ fuzztest::Domain<ContPlan> ContDef(TypeBuilderPlan plan) {
854854
bool shared = plan.kinds[plan.curr].shared;
855855
auto matches =
856856
AvailableMatchingIndices(std::move(plan), [&](auto kind, bool otherShared) {
857-
return kind == FuncKind && (!shared || otherShared);
857+
return kind == FuncKind && shared == otherShared;
858858
});
859859
if (matches.empty()) {
860860
return fuzztest::NullOpt<size_t>();
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
;; Test that shared abstract heap types require shared-everything threads
2+
3+
;; RUN: not wasm-opt %s 2>&1 | filecheck %s --check-prefix NO-SHARED
4+
;; RUN: wasm-opt %s --enable-reference-types --enable-gc --enable-shared-everything -o - -S | filecheck %s --check-prefix SHARED
5+
6+
;; NO-SHARED: invalid type: Shared types require shared-everything
7+
;; SHARED: (type $t (struct (field (ref null (shared any)))))
8+
9+
(module
10+
(type $t (struct (field (ref null (shared any)))))
11+
(global (ref null $t) (ref.null none))
12+
)

0 commit comments

Comments
 (0)