Skip to content

Commit 14d117f

Browse files
kripkenkripken
authored
[CD] Fix TypeSSA on exact parameters in unreachable calls (#7628)
TypeSSA ignored unreachable instructions, but we do validate parts of them, like calls - a later unreachable param does not disable validation or other params (see new testcase). Remove the ignoring of such code in TypeSSA and add proper skipping of unreachable code, where necessary, in ChildTyper (as an option).
1 parent b3a14c2 commit 14d117f

File tree

4 files changed

+202
-51
lines changed

4 files changed

+202
-51
lines changed

‎src/ir/child-typer.h

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@ namespace wasm {
6565
// the types of children. For example, it does not report the constraint that
6666
// two non-reference children of `select` must have the same type because that
6767
// would require inspecting the types of those children.
68+
//
69+
// The skipUnreachable() hook function determines the behavior on code that
70+
// we cannot process due to unreachability. For example, an unreachable
71+
// StructNew has no struct type defined, so we cannot apply its heap type. By
72+
// default we do not skip such unreachable code, and error in such such cases if
73+
// the type is not provided (by passing the heap type as mentioned above, as a
74+
// parameter to the visit* method). Optimization passes can often skip
75+
// unreachable code (leaving it for DCE), while other operations might not.
6876
template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
6977
Module& wasm;
7078
Function* func;
@@ -109,6 +117,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
109117

110118
Type getLabelType(Name label) { return self().getLabelType(label); }
111119

120+
bool skipUnreachable() { return false; }
121+
112122
void visitNop(Nop* curr) {}
113123

114124
void visitBlock(Block* curr) {
@@ -197,6 +207,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
197207
}
198208

199209
void visitAtomicRMW(AtomicRMW* curr) {
210+
if (self().skipUnreachable() && curr->type == Type::unreachable) {
211+
return;
212+
}
200213
assert(curr->type == Type::i32 || curr->type == Type::i64);
201214
notePointer(&curr->ptr, curr->memory);
202215
note(&curr->value, curr->type);
@@ -823,6 +836,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
823836
void visitTupleExtract(TupleExtract* curr,
824837
std::optional<size_t> arity = std::nullopt) {
825838
if (!arity) {
839+
if (self().skipUnreachable() && !curr->tuple->type.isTuple()) {
840+
return;
841+
}
826842
assert(curr->tuple->type.isTuple());
827843
arity = curr->tuple->type.size();
828844
}
@@ -837,6 +853,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
837853

838854
void visitCallRef(CallRef* curr, std::optional<HeapType> ht = std::nullopt) {
839855
if (!ht) {
856+
if (self().skipUnreachable() && !curr->target->type.isRef()) {
857+
return;
858+
}
840859
ht = curr->target->type.getHeapType().getSignature();
841860
}
842861
auto params = ht->getSignature().params;
@@ -848,17 +867,26 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
848867
}
849868

850869
void visitRefTest(RefTest* curr) {
870+
if (self().skipUnreachable() && !curr->castType.isRef()) {
871+
return;
872+
}
851873
auto top = curr->castType.getHeapType().getTop();
852874
note(&curr->ref, Type(top, Nullable));
853875
}
854876

855877
void visitRefCast(RefCast* curr, std::optional<Type> target = std::nullopt) {
878+
if (self().skipUnreachable() && !curr->type.isRef()) {
879+
return;
880+
}
856881
auto top = curr->type.getHeapType().getTop();
857882
note(&curr->ref, Type(top, Nullable));
858883
if (curr->desc) {
859884
if (!target) {
860885
target = curr->type;
861886
}
887+
if (self().skipUnreachable() && !target->isRef()) {
888+
return;
889+
}
862890
auto desc = target->getHeapType().getDescriptorType();
863891
assert(desc);
864892
note(&curr->desc, Type(*desc, Nullable, curr->type.getExactness()));
@@ -868,6 +896,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
868896
void visitRefGetDesc(RefGetDesc* curr,
869897
std::optional<HeapType> ht = std::nullopt) {
870898
if (!ht) {
899+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
900+
return;
901+
}
871902
ht = curr->ref->type.getHeapType();
872903
}
873904
note(&curr->ref, Type(*ht, Nullable));
@@ -886,6 +917,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
886917
if (!target) {
887918
target = curr->castType;
888919
}
920+
if (self().skipUnreachable() && !target->isRef()) {
921+
return;
922+
}
889923
auto top = target->getHeapType().getTop();
890924
note(&curr->ref, Type(top, Nullable));
891925
if (curr->op == BrOnCastDesc || curr->op == BrOnCastDescFail) {
@@ -903,6 +937,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
903937
if (curr->isWithDefault()) {
904938
return;
905939
}
940+
if (self().skipUnreachable() && !curr->type.isRef()) {
941+
return;
942+
}
906943
const auto& fields = curr->type.getHeapType().getStruct().fields;
907944
assert(fields.size() == curr->operands.size());
908945
for (size_t i = 0; i < fields.size(); ++i) {
@@ -913,6 +950,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
913950
void visitStructGet(StructGet* curr,
914951
std::optional<HeapType> ht = std::nullopt) {
915952
if (!ht) {
953+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
954+
return;
955+
}
916956
ht = curr->ref->type.getHeapType();
917957
}
918958
note(&curr->ref, Type(*ht, Nullable));
@@ -921,6 +961,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
921961
void visitStructSet(StructSet* curr,
922962
std::optional<HeapType> ht = std::nullopt) {
923963
if (!ht) {
964+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
965+
return;
966+
}
924967
ht = curr->ref->type.getHeapType();
925968
}
926969
const auto& fields = ht->getStruct().fields;
@@ -932,6 +975,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
932975
void visitStructRMW(StructRMW* curr,
933976
std::optional<HeapType> ht = std::nullopt) {
934977
if (!ht) {
978+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
979+
return;
980+
}
935981
ht = curr->ref->type.getHeapType();
936982
}
937983
const auto& fields = ht->getStruct().fields;
@@ -943,6 +989,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
943989
void visitStructCmpxchg(StructCmpxchg* curr,
944990
std::optional<HeapType> ht = std::nullopt) {
945991
if (!ht) {
992+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
993+
return;
994+
}
946995
ht = curr->ref->type.getHeapType();
947996
}
948997
const auto& fields = ht->getStruct().fields;
@@ -954,6 +1003,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
9541003

9551004
void visitArrayNew(ArrayNew* curr) {
9561005
if (!curr->isWithDefault()) {
1006+
if (self().skipUnreachable() && !curr->type.isRef()) {
1007+
return;
1008+
}
9571009
note(&curr->init, curr->type.getHeapType().getArray().element.type);
9581010
}
9591011
note(&curr->size, Type::i32);
@@ -970,6 +1022,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
9701022
}
9711023

9721024
void visitArrayNewFixed(ArrayNewFixed* curr) {
1025+
if (self().skipUnreachable() && !curr->type.isRef()) {
1026+
return;
1027+
}
9731028
auto type = curr->type.getHeapType().getArray().element.type;
9741029
for (auto& expr : curr->values) {
9751030
note(&expr, type);
@@ -979,6 +1034,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
9791034
void visitArrayGet(ArrayGet* curr,
9801035
std::optional<HeapType> ht = std::nullopt) {
9811036
if (!ht) {
1037+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1038+
return;
1039+
}
9821040
ht = curr->ref->type.getHeapType();
9831041
}
9841042
note(&curr->ref, Type(*ht, Nullable));
@@ -988,6 +1046,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
9881046
void visitArraySet(ArraySet* curr,
9891047
std::optional<HeapType> ht = std::nullopt) {
9901048
if (!ht) {
1049+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1050+
return;
1051+
}
9911052
ht = curr->ref->type.getHeapType();
9921053
}
9931054
auto type = ht->getArray().element.type;
@@ -1004,9 +1065,15 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10041065
std::optional<HeapType> dest = std::nullopt,
10051066
std::optional<HeapType> src = std::nullopt) {
10061067
if (!dest) {
1068+
if (self().skipUnreachable() && !curr->destRef->type.isRef()) {
1069+
return;
1070+
}
10071071
dest = curr->destRef->type.getHeapType();
10081072
}
10091073
if (!src) {
1074+
if (self().skipUnreachable() && !curr->srcRef->type.isRef()) {
1075+
return;
1076+
}
10101077
src = curr->srcRef->type.getHeapType();
10111078
}
10121079
note(&curr->destRef, Type(*dest, Nullable));
@@ -1019,6 +1086,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10191086
void visitArrayFill(ArrayFill* curr,
10201087
std::optional<HeapType> ht = std::nullopt) {
10211088
if (!ht) {
1089+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1090+
return;
1091+
}
10221092
ht = curr->ref->type.getHeapType();
10231093
}
10241094
auto type = ht->getArray().element.type;
@@ -1031,6 +1101,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10311101
void visitArrayInitData(ArrayInitData* curr,
10321102
std::optional<HeapType> ht = std::nullopt) {
10331103
if (!ht) {
1104+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1105+
return;
1106+
}
10341107
ht = curr->ref->type.getHeapType();
10351108
}
10361109
note(&curr->ref, Type(*ht, Nullable));
@@ -1042,6 +1115,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10421115
void visitArrayInitElem(ArrayInitElem* curr,
10431116
std::optional<HeapType> ht = std::nullopt) {
10441117
if (!ht) {
1118+
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1119+
return;
1120+
}
10451121
ht = curr->ref->type.getHeapType();
10461122
}
10471123
note(&curr->ref, Type(*ht, Nullable));
@@ -1093,6 +1169,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10931169
void visitStringEncode(StringEncode* curr,
10941170
std::optional<HeapType> ht = std::nullopt) {
10951171
if (!ht) {
1172+
if (self().skipUnreachable() && !curr->array->type.isRef()) {
1173+
return;
1174+
}
10961175
ht = curr->array->type.getHeapType();
10971176
}
10981177
note(&curr->str, Type(HeapType::string, Nullable));
@@ -1129,9 +1208,15 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
11291208
std::optional<HeapType> src = std::nullopt,
11301209
std::optional<HeapType> dest = std::nullopt) {
11311210
if (!src.has_value()) {
1211+
if (self().skipUnreachable() && !curr->cont->type.isRef()) {
1212+
return;
1213+
}
11321214
src = curr->cont->type.getHeapType();
11331215
}
11341216
if (!dest.has_value()) {
1217+
if (self().skipUnreachable() && !curr->type.isRef()) {
1218+
return;
1219+
}
11351220
dest = curr->type.getHeapType();
11361221
}
11371222
auto sourceParams = src->getContinuation().type.getSignature().params;
@@ -1155,6 +1240,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
11551240

11561241
void visitResume(Resume* curr, std::optional<HeapType> ct = std::nullopt) {
11571242
if (!ct.has_value()) {
1243+
if (self().skipUnreachable() && !curr->cont->type.isRef()) {
1244+
return;
1245+
}
11581246
ct = curr->cont->type.getHeapType();
11591247
}
11601248
assert(ct->isContinuation());
@@ -1169,6 +1257,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
11691257
void visitResumeThrow(ResumeThrow* curr,
11701258
std::optional<HeapType> ct = std::nullopt) {
11711259
if (!ct.has_value()) {
1260+
if (self().skipUnreachable() && !curr->cont->type.isRef()) {
1261+
return;
1262+
}
11721263
ct = curr->cont->type.getHeapType();
11731264
}
11741265
assert(ct->isContinuation());
@@ -1183,6 +1274,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
11831274
void visitStackSwitch(StackSwitch* curr,
11841275
std::optional<HeapType> ct = std::nullopt) {
11851276
if (!ct.has_value()) {
1277+
if (self().skipUnreachable() && !curr->cont->type.isRef()) {
1278+
return;
1279+
}
11861280
ct = curr->cont->type.getHeapType();
11871281
}
11881282
assert(ct->isContinuation());

‎src/passes/TypeSSA.cpp

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -215,18 +215,6 @@ struct Analyzer
215215
return;
216216
}
217217

218-
// Also do not let unreachable instructions inhibit optimization, as long as
219-
// they are unreachable because of an unreachable child. (Some other
220-
// unreachable instructions, such as a return_call, can still require an
221-
// exact operand and may inhibit optimization.)
222-
if (curr->type == Type::unreachable) {
223-
for (auto* child : ChildIterator(curr)) {
224-
if (child->type == Type::unreachable) {
225-
return;
226-
}
227-
}
228-
}
229-
230218
struct ExactChildTyper : ChildTyper<ExactChildTyper> {
231219
Analyzer& parent;
232220
ExactChildTyper(Analyzer& parent)
@@ -249,6 +237,10 @@ struct Analyzer
249237
void noteAnyI16ArrayReferenceType(Expression**) {}
250238

251239
Type getLabelType(Name label) { WASM_UNREACHABLE("unexpected branch"); }
240+
241+
// Skip unreachable code. If we cannot compute a constraint due to
242+
// unreachability, we can ignore it.
243+
bool skipUnreachable() { return true; }
252244
} typer(*this);
253245
typer.visit(curr);
254246
}

0 commit comments

Comments
 (0)