Skip to content

Commit

Permalink
Fix TypeMerging bug with indirectly reachable public types (WebAssemb…
Browse files Browse the repository at this point in the history
…ly#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.
  • Loading branch information
tlively authored Oct 24, 2024
1 parent dcc70bb commit de421db
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
18 changes: 13 additions & 5 deletions src/passes/TypeMerging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -504,11 +504,19 @@ std::vector<HeapType> TypeMerging::getPublicChildren(HeapType type) {

DFA::State<HeapType> TypeMerging::makeDFAState(HeapType type) {
std::vector<HeapType> succs;
for (auto child : type.getHeapTypeChildren()) {
// Both private and public heap type children participate in the DFA and are
// eligible to be successors.
if (!child.isBasic()) {
succs.push_back(getMerged(child));
// Both private and public heap type children participate in the DFA and are
// eligible to be successors, except that public types are terminal states
// that do not have successors. This is sufficient because public types are
// always in their own singleton partitions, so they already differentiate
// types that reach them without needing to consider their children. In the
// other direction, including the children is not necessary to differentiate
// types reached by the public types because all such reachable types are also
// public and not eligible to be merged.
if (privateTypes.count(type)) {
for (auto child : type.getHeapTypeChildren()) {
if (!child.isBasic()) {
succs.push_back(getMerged(child));
}
}
}
return {type, std::move(succs)};
Expand Down
19 changes: 19 additions & 0 deletions test/lit/passes/type-merging.wast
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,25 @@
(global $g2 (ref $C') (struct.new_default $D2'))
)

;; Regression test for a bug where public types not directly reachable from
;; private types were included as successors but not given states in the DFA.
(module
;; CHECK: (type $public-child (struct))
(type $public-child (struct))
;; CHECK: (type $public (struct (field (ref $public-child))))
(type $public (struct (ref $public-child)))
;; CHECK: (type $private (struct (field (ref $public))))
(type $private (struct (ref $public)))

;; CHECK: (global $public (ref null $public) (ref.null none))
(global $public (ref null $public) (ref.null none))
;; CHECK: (global $private (ref null $private) (ref.null none))
(global $private (ref null $private) (ref.null none))

;; CHECK: (export "public" (global $public))
(export "public" (global $public))
)

;; Check that a ref.test inhibits merging (ref.cast is already checked above).
(module
;; CHECK: (rec
Expand Down

0 comments on commit de421db

Please sign in to comment.