Skip to content

Commit de421db

Browse files
authored
Fix TypeMerging bug with indirectly reachable public types (WebAssembly#7031)
TypeMerging works by representing the type definition graph as a partitioned DFA and then refining the partitions to find mergeable types. WebAssembly#7023 was due to a bug where the DFA included edges from public types to their children, but did not necessarily include corresponding states for those children. One way to fix the bug would have been to traverse the type graph, finding all reachable public types and creating DFA states for them, but that might be expensive in cases where there are large graphs of public types. Instead, fix the problem by removing the edges from public types to their children entirely. Types reachable from public types are also public and therefore are not eligible to be merged, so these edges were never necessary for correctness. Fixes WebAssembly#7023.
1 parent dcc70bb commit de421db

File tree

2 files changed

+32
-5
lines changed

2 files changed

+32
-5
lines changed

src/passes/TypeMerging.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -504,11 +504,19 @@ std::vector<HeapType> TypeMerging::getPublicChildren(HeapType type) {
504504

505505
DFA::State<HeapType> TypeMerging::makeDFAState(HeapType type) {
506506
std::vector<HeapType> succs;
507-
for (auto child : type.getHeapTypeChildren()) {
508-
// Both private and public heap type children participate in the DFA and are
509-
// eligible to be successors.
510-
if (!child.isBasic()) {
511-
succs.push_back(getMerged(child));
507+
// Both private and public heap type children participate in the DFA and are
508+
// eligible to be successors, except that public types are terminal states
509+
// that do not have successors. This is sufficient because public types are
510+
// always in their own singleton partitions, so they already differentiate
511+
// types that reach them without needing to consider their children. In the
512+
// other direction, including the children is not necessary to differentiate
513+
// types reached by the public types because all such reachable types are also
514+
// public and not eligible to be merged.
515+
if (privateTypes.count(type)) {
516+
for (auto child : type.getHeapTypeChildren()) {
517+
if (!child.isBasic()) {
518+
succs.push_back(getMerged(child));
519+
}
512520
}
513521
}
514522
return {type, std::move(succs)};

test/lit/passes/type-merging.wast

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,25 @@
985985
(global $g2 (ref $C') (struct.new_default $D2'))
986986
)
987987

988+
;; Regression test for a bug where public types not directly reachable from
989+
;; private types were included as successors but not given states in the DFA.
990+
(module
991+
;; CHECK: (type $public-child (struct))
992+
(type $public-child (struct))
993+
;; CHECK: (type $public (struct (field (ref $public-child))))
994+
(type $public (struct (ref $public-child)))
995+
;; CHECK: (type $private (struct (field (ref $public))))
996+
(type $private (struct (ref $public)))
997+
998+
;; CHECK: (global $public (ref null $public) (ref.null none))
999+
(global $public (ref null $public) (ref.null none))
1000+
;; CHECK: (global $private (ref null $private) (ref.null none))
1001+
(global $private (ref null $private) (ref.null none))
1002+
1003+
;; CHECK: (export "public" (global $public))
1004+
(export "public" (global $public))
1005+
)
1006+
9881007
;; Check that a ref.test inhibits merging (ref.cast is already checked above).
9891008
(module
9901009
;; CHECK: (rec

0 commit comments

Comments
 (0)