Skip to content

Commit 165953e

Browse files
tlivelytlively
authored
Fix stringview subtyping (#6440)
The stringview types (`stringview_wtf8`, `stringview_wtf16`, and `stringview_iter`) are not subtypes of `any` even though they are supertypes of `none`. This breaks the type system invariant that types share a bottom type iff they share a top type, but we can work around that.
1 parent 19f8cc7 commit 165953e

File tree

3 files changed

+77
-36
lines changed

3 files changed

+77
-36
lines changed

‎src/tools/fuzzing/heap-types.cpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,12 +454,19 @@ struct HeapTypeGeneratorImpl {
454454
candidates.push_back(HeapType::any);
455455
break;
456456
case HeapType::string:
457+
candidates.push_back(HeapType::any);
458+
break;
457459
case HeapType::stringview_wtf8:
458460
case HeapType::stringview_wtf16:
459461
case HeapType::stringview_iter:
460-
candidates.push_back(HeapType::any);
461462
break;
462463
case HeapType::none:
464+
if (rand.oneIn(10)) {
465+
candidates.push_back(HeapType::stringview_wtf8);
466+
candidates.push_back(HeapType::stringview_wtf16);
467+
candidates.push_back(HeapType::stringview_iter);
468+
break;
469+
}
463470
return pickSubAny();
464471
case HeapType::nofunc:
465472
return pickSubFunc();

‎src/wasm/wasm-type.cpp‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ std::optional<HeapType> getBasicHeapTypeLUB(HeapType::BasicHeapType a,
456456
if (a == b) {
457457
return a;
458458
}
459-
if (HeapType(a).getBottom() != HeapType(b).getBottom()) {
459+
if (HeapType(a).getTop() != HeapType(b).getTop()) {
460460
return {};
461461
}
462462
if (HeapType(a).isBottom()) {
@@ -494,10 +494,12 @@ std::optional<HeapType> getBasicHeapTypeLUB(HeapType::BasicHeapType a,
494494
return {HeapType::any};
495495
case HeapType::array:
496496
case HeapType::string:
497+
return {HeapType::any};
497498
case HeapType::stringview_wtf8:
498499
case HeapType::stringview_wtf16:
499500
case HeapType::stringview_iter:
500-
return {HeapType::any};
501+
// Only joinable with bottom or self, both already handled.
502+
return std::nullopt;
501503
case HeapType::none:
502504
case HeapType::noext:
503505
case HeapType::nofunc:
@@ -1411,14 +1413,35 @@ HeapType::BasicHeapType HeapType::getBottom() const {
14111413
}
14121414

14131415
HeapType::BasicHeapType HeapType::getTop() const {
1416+
if (*this == HeapType::stringview_wtf8 ||
1417+
*this == HeapType::stringview_wtf16 ||
1418+
*this == HeapType::stringview_iter) {
1419+
// These types are their own top types even though they share a bottom type
1420+
// `none` with the anyref hierarchy. This means that technically there are
1421+
// multiple top types for `none`, but `any` is the canonical one.
1422+
return getBasic();
1423+
}
14141424
switch (getBottom()) {
14151425
case none:
14161426
return any;
14171427
case nofunc:
14181428
return func;
14191429
case noext:
14201430
return ext;
1421-
default:
1431+
case noexn:
1432+
return exn;
1433+
case ext:
1434+
case func:
1435+
case any:
1436+
case eq:
1437+
case i31:
1438+
case struct_:
1439+
case array:
1440+
case exn:
1441+
case string:
1442+
case stringview_wtf8:
1443+
case stringview_wtf16:
1444+
case stringview_iter:
14221445
break;
14231446
}
14241447
WASM_UNREACHABLE("unexpected type");
@@ -1693,13 +1716,13 @@ bool SubTyper::isSubType(HeapType a, HeapType b) {
16931716
if (b.isBasic()) {
16941717
switch (b.getBasic()) {
16951718
case HeapType::ext:
1696-
return a.getBottom() == HeapType::noext;
1719+
return a.getTop() == HeapType::ext;
16971720
case HeapType::func:
1698-
return a.getBottom() == HeapType::nofunc;
1721+
return a.getTop() == HeapType::func;
16991722
case HeapType::exn:
1700-
return a.getBottom() == HeapType::noexn;
1723+
return a.getTop() == HeapType::exn;
17011724
case HeapType::any:
1702-
return a.getBottom() == HeapType::none;
1725+
return a.getTop() == HeapType::any;
17031726
case HeapType::eq:
17041727
return a == HeapType::i31 || a == HeapType::none ||
17051728
a == HeapType::struct_ || a == HeapType::array || a.isStruct() ||

‎test/gtest/type-builder.cpp‎

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -545,23 +545,34 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
545545
if (a == b) {
546546
EXPECT_TRUE(HeapType::isSubType(a, b));
547547
EXPECT_TRUE(HeapType::isSubType(b, a));
548+
EXPECT_EQ(a.getTop(), b.getTop());
548549
EXPECT_EQ(a.getBottom(), b.getBottom());
549550
} else if (lub && *lub == b) {
550551
EXPECT_TRUE(HeapType::isSubType(a, b));
551552
EXPECT_FALSE(HeapType::isSubType(b, a));
553+
// This would hold except for the case of stringview types and none, where
554+
// stringview types are their own top types, but we return `any` as the
555+
// top type of none.
556+
// EXPECT_EQ(a.getTop(), b.getTop());
552557
EXPECT_EQ(a.getBottom(), b.getBottom());
553558
} else if (lub && *lub == a) {
554559
EXPECT_FALSE(HeapType::isSubType(a, b));
555560
EXPECT_TRUE(HeapType::isSubType(b, a));
561+
// EXPECT_EQ(a.getTop(), b.getTop());
556562
EXPECT_EQ(a.getBottom(), b.getBottom());
557563
} else if (lub) {
558564
EXPECT_FALSE(HeapType::isSubType(a, b));
559565
EXPECT_FALSE(HeapType::isSubType(b, a));
566+
// EXPECT_EQ(a.getTop(), b.getTop());
560567
EXPECT_EQ(a.getBottom(), b.getBottom());
561568
} else {
562569
EXPECT_FALSE(HeapType::isSubType(a, b));
563570
EXPECT_FALSE(HeapType::isSubType(b, a));
564-
EXPECT_NE(a.getBottom(), b.getBottom());
571+
EXPECT_NE(a.getTop(), b.getTop());
572+
// This would hold except for stringview types, which share a bottom with
573+
// the anyref hierarchy despite having no shared upper bound with its
574+
// types.
575+
// EXPECT_NE(a.getBottom(), b.getBottom());
565576
}
566577
};
567578

@@ -606,9 +617,9 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
606617
assertLUB(any, struct_, any);
607618
assertLUB(any, array, any);
608619
assertLUB(any, string, any);
609-
assertLUB(any, stringview_wtf8, any);
610-
assertLUB(any, stringview_wtf16, any);
611-
assertLUB(any, stringview_iter, any);
620+
assertLUB(any, stringview_wtf8, {});
621+
assertLUB(any, stringview_wtf16, {});
622+
assertLUB(any, stringview_iter, {});
612623
assertLUB(any, none, any);
613624
assertLUB(any, noext, {});
614625
assertLUB(any, nofunc, {});
@@ -621,9 +632,9 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
621632
assertLUB(eq, struct_, eq);
622633
assertLUB(eq, array, eq);
623634
assertLUB(eq, string, any);
624-
assertLUB(eq, stringview_wtf8, any);
625-
assertLUB(eq, stringview_wtf16, any);
626-
assertLUB(eq, stringview_iter, any);
635+
assertLUB(eq, stringview_wtf8, {});
636+
assertLUB(eq, stringview_wtf16, {});
637+
assertLUB(eq, stringview_iter, {});
627638
assertLUB(eq, none, eq);
628639
assertLUB(eq, noext, {});
629640
assertLUB(eq, nofunc, {});
@@ -635,9 +646,9 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
635646
assertLUB(i31, struct_, eq);
636647
assertLUB(i31, array, eq);
637648
assertLUB(i31, string, any);
638-
assertLUB(i31, stringview_wtf8, any);
639-
assertLUB(i31, stringview_wtf16, any);
640-
assertLUB(i31, stringview_iter, any);
649+
assertLUB(i31, stringview_wtf8, {});
650+
assertLUB(i31, stringview_wtf16, {});
651+
assertLUB(i31, stringview_iter, {});
641652
assertLUB(i31, none, i31);
642653
assertLUB(i31, noext, {});
643654
assertLUB(i31, nofunc, {});
@@ -648,9 +659,9 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
648659
assertLUB(struct_, struct_, struct_);
649660
assertLUB(struct_, array, eq);
650661
assertLUB(struct_, string, any);
651-
assertLUB(struct_, stringview_wtf8, any);
652-
assertLUB(struct_, stringview_wtf16, any);
653-
assertLUB(struct_, stringview_iter, any);
662+
assertLUB(struct_, stringview_wtf8, {});
663+
assertLUB(struct_, stringview_wtf16, {});
664+
assertLUB(struct_, stringview_iter, {});
654665
assertLUB(struct_, none, struct_);
655666
assertLUB(struct_, noext, {});
656667
assertLUB(struct_, nofunc, {});
@@ -660,9 +671,9 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
660671

661672
assertLUB(array, array, array);
662673
assertLUB(array, string, any);
663-
assertLUB(array, stringview_wtf8, any);
664-
assertLUB(array, stringview_wtf16, any);
665-
assertLUB(array, stringview_iter, any);
674+
assertLUB(array, stringview_wtf8, {});
675+
assertLUB(array, stringview_wtf16, {});
676+
assertLUB(array, stringview_iter, {});
666677
assertLUB(array, none, array);
667678
assertLUB(array, noext, {});
668679
assertLUB(array, nofunc, {});
@@ -671,9 +682,9 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
671682
assertLUB(array, defArray, array);
672683

673684
assertLUB(string, string, string);
674-
assertLUB(string, stringview_wtf8, any);
675-
assertLUB(string, stringview_wtf16, any);
676-
assertLUB(string, stringview_iter, any);
685+
assertLUB(string, stringview_wtf8, {});
686+
assertLUB(string, stringview_wtf16, {});
687+
assertLUB(string, stringview_iter, {});
677688
assertLUB(string, none, string);
678689
assertLUB(string, noext, {});
679690
assertLUB(string, nofunc, {});
@@ -682,31 +693,31 @@ TEST_F(TypeTest, TestHeapTypeRelations) {
682693
assertLUB(string, defArray, any);
683694

684695
assertLUB(stringview_wtf8, stringview_wtf8, stringview_wtf8);
685-
assertLUB(stringview_wtf8, stringview_wtf16, any);
686-
assertLUB(stringview_wtf8, stringview_iter, any);
696+
assertLUB(stringview_wtf8, stringview_wtf16, {});
697+
assertLUB(stringview_wtf8, stringview_iter, {});
687698
assertLUB(stringview_wtf8, none, stringview_wtf8);
688699
assertLUB(stringview_wtf8, noext, {});
689700
assertLUB(stringview_wtf8, nofunc, {});
690701
assertLUB(stringview_wtf8, defFunc, {});
691-
assertLUB(stringview_wtf8, defStruct, any);
692-
assertLUB(stringview_wtf8, defArray, any);
702+
assertLUB(stringview_wtf8, defStruct, {});
703+
assertLUB(stringview_wtf8, defArray, {});
693704

694705
assertLUB(stringview_wtf16, stringview_wtf16, stringview_wtf16);
695-
assertLUB(stringview_wtf16, stringview_iter, any);
706+
assertLUB(stringview_wtf16, stringview_iter, {});
696707
assertLUB(stringview_wtf16, none, stringview_wtf16);
697708
assertLUB(stringview_wtf16, noext, {});
698709
assertLUB(stringview_wtf16, nofunc, {});
699710
assertLUB(stringview_wtf16, defFunc, {});
700-
assertLUB(stringview_wtf16, defStruct, any);
701-
assertLUB(stringview_wtf16, defArray, any);
711+
assertLUB(stringview_wtf16, defStruct, {});
712+
assertLUB(stringview_wtf16, defArray, {});
702713

703714
assertLUB(stringview_iter, stringview_iter, stringview_iter);
704715
assertLUB(stringview_iter, none, stringview_iter);
705716
assertLUB(stringview_iter, noext, {});
706717
assertLUB(stringview_iter, nofunc, {});
707718
assertLUB(stringview_iter, defFunc, {});
708-
assertLUB(stringview_iter, defStruct, any);
709-
assertLUB(stringview_iter, defArray, any);
719+
assertLUB(stringview_iter, defStruct, {});
720+
assertLUB(stringview_iter, defArray, {});
710721

711722
assertLUB(none, none, none);
712723
assertLUB(none, noext, {});

0 commit comments

Comments
 (0)