Skip to content

Commit c891c01

Browse files
kripkenkripken
authored
Note function signature param/result features for validation (#5542)
As with #5535, this was not noticed because it can only happen on very small modules where the param/result type appears nowhere else but in a function signature. Use generic heap type scanning, which also scans into struct and array types etc.
1 parent dea234b commit c891c01

File tree

3 files changed

+95
-42
lines changed

3 files changed

+95
-42
lines changed

‎src/wasm/wasm-type.cpp

Lines changed: 69 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,49 +1007,76 @@ Type Type::reinterpret() const {
10071007
FeatureSet Type::getFeatures() const {
10081008
auto getSingleFeatures = [](Type t) -> FeatureSet {
10091009
if (t.isRef()) {
1010-
// A reference type implies we need that feature. Some also require more,
1011-
// such as GC or exceptions.
1012-
auto heapType = t.getHeapType();
1013-
if (heapType.isBasic()) {
1014-
switch (heapType.getBasic()) {
1015-
case HeapType::ext:
1016-
case HeapType::func:
1017-
return FeatureSet::ReferenceTypes;
1018-
case HeapType::any:
1019-
case HeapType::eq:
1020-
case HeapType::i31:
1021-
case HeapType::struct_:
1022-
case HeapType::array:
1023-
return FeatureSet::ReferenceTypes | FeatureSet::GC;
1024-
case HeapType::string:
1025-
case HeapType::stringview_wtf8:
1026-
case HeapType::stringview_wtf16:
1027-
case HeapType::stringview_iter:
1028-
return FeatureSet::ReferenceTypes | FeatureSet::Strings;
1029-
case HeapType::none:
1030-
case HeapType::noext:
1031-
case HeapType::nofunc:
1032-
// Technically introduced in GC, but used internally as part of
1033-
// ref.null with just reference types.
1034-
return FeatureSet::ReferenceTypes;
1010+
// A reference type implies we need that feature. Some also require
1011+
// more, such as GC or exceptions, and may require us to look into child
1012+
// types.
1013+
struct ReferenceFeatureCollector
1014+
: HeapTypeChildWalker<ReferenceFeatureCollector> {
1015+
FeatureSet feats = FeatureSet::None;
1016+
1017+
void noteChild(HeapType* heapType) {
1018+
if (heapType->isBasic()) {
1019+
switch (heapType->getBasic()) {
1020+
case HeapType::ext:
1021+
case HeapType::func:
1022+
feats |= FeatureSet::ReferenceTypes;
1023+
return;
1024+
case HeapType::any:
1025+
case HeapType::eq:
1026+
case HeapType::i31:
1027+
case HeapType::struct_:
1028+
case HeapType::array:
1029+
feats |= FeatureSet::ReferenceTypes | FeatureSet::GC;
1030+
return;
1031+
case HeapType::string:
1032+
case HeapType::stringview_wtf8:
1033+
case HeapType::stringview_wtf16:
1034+
case HeapType::stringview_iter:
1035+
feats |= FeatureSet::ReferenceTypes | FeatureSet::Strings;
1036+
return;
1037+
case HeapType::none:
1038+
case HeapType::noext:
1039+
case HeapType::nofunc:
1040+
// Technically introduced in GC, but used internally as part of
1041+
// ref.null with just reference types.
1042+
feats |= FeatureSet::ReferenceTypes;
1043+
return;
1044+
}
1045+
}
1046+
1047+
if (heapType->isStruct() || heapType->isArray() ||
1048+
heapType->getRecGroup().size() > 1 || heapType->getSuperType()) {
1049+
feats |= FeatureSet::ReferenceTypes | FeatureSet::GC;
1050+
} else if (heapType->isSignature()) {
1051+
// This is a function reference, which requires reference types and
1052+
// possibly also multivalue (if it has multiple returns). Note that
1053+
// technically typed function references also require GC, however,
1054+
// we use these types internally regardless of the presence of GC
1055+
// (in particular, since during load of the wasm we don't know the
1056+
// features yet, so we apply the more refined types), so we don't
1057+
// add that in any case here.
1058+
feats |= FeatureSet::ReferenceTypes;
1059+
auto sig = heapType->getSignature();
1060+
if (sig.results.isTuple()) {
1061+
feats |= FeatureSet::Multivalue;
1062+
}
1063+
}
1064+
1065+
// In addition, scan their non-ref children, to add dependencies on
1066+
// things like SIMD.
1067+
for (auto child : heapType->getTypeChildren()) {
1068+
if (!child.isRef()) {
1069+
feats |= child.getFeatures();
1070+
}
1071+
}
10351072
}
1036-
}
1037-
if (heapType.isStruct() || heapType.isArray() ||
1038-
heapType.getRecGroup().size() > 1 || heapType.getSuperType()) {
1039-
return FeatureSet::ReferenceTypes | FeatureSet::GC;
1040-
}
1041-
// Otherwise, this is a function reference, which requires reference types
1042-
// and possibly also multivalue (if it has multiple returns).
1043-
// Note: Technically typed function references also require GC, however,
1044-
// we use these types internally regardless of the presence of GC (in
1045-
// particular, since during load of the wasm we don't know the features
1046-
// yet, so we apply the more refined types), so we don't add that in any
1047-
// case here.
1048-
FeatureSet feats = FeatureSet::ReferenceTypes;
1049-
if (heapType.getSignature().results.isTuple()) {
1050-
feats |= FeatureSet::Multivalue;
1051-
}
1052-
return feats;
1073+
};
1074+
1075+
ReferenceFeatureCollector collector;
1076+
auto heapType = t.getHeapType();
1077+
collector.walkRoot(&heapType);
1078+
collector.noteChild(&heapType);
1079+
return collector.feats;
10531080
}
10541081
TODO_SINGLE_COMPOUND(t);
10551082
switch (t.getBasic()) {

‎test/lit/validation/ref-func-simd.wast

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
;; A function type that uses a SIMD type requires the SIMD feature.
2+
;; RUN: not wasm-opt --enable-reference-types %s 2>&1 | filecheck %s
3+
4+
;; CHECK: all used types should be allowed
5+
6+
(module
7+
(type $simd-user (func (param v128)))
8+
9+
(global $g (ref null $simd-user) (ref.null func))
10+
)
11+
12+
;; But it passes with the feature enabled.
13+
;; RUN: wasm-opt --enable-reference-types --enable-simd %s

‎test/lit/validation/ref-gc-simd.wast

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
;; A gc type that uses a SIMD type requires the SIMD feature.
2+
;; RUN: not wasm-opt --enable-reference-types --enable-gc %s 2>&1 | filecheck %s
3+
4+
;; CHECK: all used types should be allowed
5+
6+
(module
7+
(type $simd-user (struct (field v128)))
8+
9+
(global $g (ref null $simd-user) (ref.null $simd-user))
10+
)
11+
12+
;; But it passes with the feature enabled.
13+
;; RUN: wasm-opt --enable-reference-types --enable-gc --enable-simd %s

0 commit comments

Comments
 (0)