Skip to content

Commit 2602030

Browse files
authored
Fix propagation bug in Unsubtyping (#8468)
Unsubtyping has to propagate a `subtypesExposedToJS` marker from supertypes to subtypes. It previously did this by propagating from supertype to subtype whenever a new subtyping relationship was found. But subtyping trees are not always built from top to bottom. In cases where the newly discovered subtype already had subtypes, the previous propagation scheme would fail to propagate to those transitive subtypes. Fix the problem by propagating to the full subtype tree rooted at a newly discovered subtype. Use a DFS over the tree and avoid traversing subtrees that have already been marked to avoid introducing possible quadratic behavior.
1 parent dc89132 commit 2602030

2 files changed

Lines changed: 67 additions & 3 deletions

File tree

src/passes/Unsubtyping.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -421,8 +421,8 @@ struct TypeTree {
421421
for (auto child : node.children) {
422422
std::cerr << " " << ModuleHeapType(wasm, nodes[child].type);
423423
}
424-
if (node.exposedToJS) {
425-
std::cerr << ", exposed to JS";
424+
if (node.subtypesExposedToJS) {
425+
std::cerr << ", subtypes exposed to JS";
426426
}
427427
std::cerr << '\n';
428428
}
@@ -877,8 +877,22 @@ struct Unsubtyping : Pass, Noter<Unsubtyping> {
877877
types.setSupertype(sub, super);
878878

879879
// If the supertype is exposed to JS, the subtype potentially is as well.
880+
// `sub` may be the root of some existing subtype tree, and we have to
881+
// propagate the exposure to JS to all those existing subtypes. We could
882+
// just iterate over subtypes(), but manually traverse using
883+
// immediateSubtypes() so we can avoid visiting subtrees that have already
884+
// been marked.
880885
if (types.areSubtypesExposedToJS(super)) {
881-
noteExposedToJS(sub);
886+
std::vector<HeapType> work{{sub}};
887+
while (!work.empty()) {
888+
auto curr = work.back();
889+
work.pop_back();
890+
if (!types.areSubtypesExposedToJS(curr)) {
891+
noteExposedToJS(curr);
892+
auto subtypes = types.immediateSubtypes(curr);
893+
work.insert(work.end(), subtypes.begin(), subtypes.end());
894+
}
895+
}
882896
}
883897

884898
// Complete the descriptor squares to the left and right of the new

test/lit/passes/unsubtyping-jsinterop.wast

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,3 +1027,53 @@
10271027
(local.get $sub-out)
10281028
)
10291029
)
1030+
1031+
(module
1032+
;; Regression test for a bug where we were not fully propagating exposure to
1033+
;; JS, resulting missing prototype-configuring descriptors depending on the
1034+
;; order in which subtypes were processed. In this example, if $struct <:
1035+
;; $super was processed before $super <: any, then exposure to JS would not be
1036+
;; propagated down to $struct, so its descriptor would be removed.
1037+
(rec
1038+
;; CHECK: (rec
1039+
;; CHECK-NEXT: (type $super (sub (struct)))
1040+
(type $super (sub (struct)))
1041+
;; CHECK: (type $struct (sub $super (descriptor $desc) (struct)))
1042+
(type $struct (sub $super (descriptor $desc) (struct)))
1043+
;; CHECK: (type $desc (describes $struct) (struct (field externref)))
1044+
(type $desc (describes $struct) (struct (field externref)))
1045+
)
1046+
1047+
;; CHECK: (type $3 (func (result anyref)))
1048+
1049+
;; CHECK: (global $any (mut anyref) (ref.null none))
1050+
(global $any (mut anyref) (ref.null none))
1051+
;; CHECK: (global $super (mut (ref null $super)) (ref.null none))
1052+
(global $super (mut (ref null $super)) (ref.null none))
1053+
;; CHECK: (global $struct (ref null $struct) (ref.null none))
1054+
(global $struct (ref null $struct) (ref.null none))
1055+
1056+
;; any exposed to JS.
1057+
;; CHECK: (@binaryen.js.called)
1058+
;; CHECK-NEXT: (func $expose-anyref (type $3) (result anyref)
1059+
;; CHECK-NEXT: (global.set $any
1060+
;; CHECK-NEXT: (global.get $super)
1061+
;; CHECK-NEXT: )
1062+
;; CHECK-NEXT: (global.set $super
1063+
;; CHECK-NEXT: (global.get $struct)
1064+
;; CHECK-NEXT: )
1065+
;; CHECK-NEXT: (unreachable)
1066+
;; CHECK-NEXT: )
1067+
(@binaryen.js.called)
1068+
(func $expose-anyref (result anyref)
1069+
;; $super <: aby
1070+
(global.set $any
1071+
(global.get $super)
1072+
)
1073+
;; $struct <: $super
1074+
(global.set $super
1075+
(global.get $struct)
1076+
)
1077+
(unreachable)
1078+
)
1079+
)

0 commit comments

Comments
 (0)