Skip to content

Commit 96f2976

Browse files
authored
Fix conflicting generalized types in TypeSSA (#8119)
TypeSSA already had logic to detect and resolve inadvertent conflicts between its newly constructed types and existing types. However, this logic did not take into account the changes that the binary writer can make when writing types, so it was still possible to construct a situation where TypeSSA would produce types that would start conflicting after binary writing. Fix the problem by using UniqueRecGroups to ensure the new types are different from existing types.
1 parent d236068 commit 96f2976

File tree

4 files changed

+76
-79
lines changed

4 files changed

+76
-79
lines changed

src/passes/TypeSSA.cpp

Lines changed: 20 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -54,93 +54,37 @@
5454
#include "ir/utils.h"
5555
#include "pass.h"
5656
#include "support/hash.h"
57+
#include "wasm-type-shape.h"
5758
#include "wasm.h"
5859

5960
namespace wasm {
6061

6162
namespace {
6263

63-
// Given some TypeBuilder items that we want to build new types with, this
64-
// function builds the types in a new rec group.
65-
//
66-
// This is almost the same as just calling build(), but there is a risk of a
67-
// collision with an existing rec group. This function handles that by finding a
68-
// way to ensure that the new types are in fact in a new rec group.
69-
//
70-
// TODO: Move this outside if we find more uses.
71-
std::vector<HeapType> ensureTypesAreInNewRecGroup(RecGroup recGroup,
64+
// Ensure there are no conflicts between the newly built types and any existing
65+
// types.
66+
std::vector<HeapType> ensureTypesAreInNewRecGroup(std::vector<HeapType>&& types,
7267
Module& wasm) {
73-
auto num = recGroup.size();
74-
75-
std::vector<HeapType> types;
76-
types.reserve(num);
77-
for (auto type : recGroup) {
78-
types.push_back(type);
68+
std::unordered_set<RecGroup> existing;
69+
for (auto type : ModuleUtils::collectHeapTypes(wasm)) {
70+
existing.insert(type.getRecGroup());
7971
}
8072

81-
// Find all the heap types present before we create the new ones. The new
82-
// types must not appear in |existingSet|.
83-
std::vector<HeapType> existing = ModuleUtils::collectHeapTypes(wasm);
84-
std::unordered_set<HeapType> existingSet(existing.begin(), existing.end());
85-
86-
// Check for a collision with an existing rec group. Note that it is enough to
87-
// check one of the types: either the entire rec group gets merged, so they
88-
// are all merged, or not.
89-
if (existingSet.count(types[0])) {
90-
// Unfortunately there is a conflict. Handle it by adding a "hash" - a
91-
// "random" extra item in the rec group that is so outlandish it will
92-
// surely (?) never collide with anything. We must loop while doing so,
93-
// until we find a hash that does not collide.
94-
//
95-
// Note that we use uint64_t here, and deterministic_hash_combine below, to
96-
// ensure our output is fully deterministic - the types we add here are
97-
// observable in the output.
98-
uint64_t hashSize = num + 10;
99-
uint64_t random = num;
100-
while (1) {
101-
// Make a builder and add a slot for the hash.
102-
TypeBuilder builder(num + 1);
103-
for (Index i = 0; i < num; i++) {
104-
builder[i].copy(types[i]);
105-
}
106-
107-
// Implement the hash as a struct with "random" fields, and add it.
108-
Struct hashStruct;
109-
for (Index i = 0; i < hashSize; i++) {
110-
// TODO: a denser encoding?
111-
auto type = (random & 1) ? Type::i32 : Type::f64;
112-
deterministic_hash_combine(random, hashSize + i);
113-
hashStruct.fields.push_back(Field(type, Mutable));
114-
}
115-
builder[num] = hashStruct;
116-
117-
// Build and hope for the best.
118-
builder.createRecGroup(0, num + 1);
119-
auto result = builder.build();
120-
assert(!result.getError());
121-
types = *result;
122-
assert(types.size() == num + 1);
123-
124-
if (existingSet.count(types[0])) {
125-
// There is still a collision. Exponentially use larger hashes to
126-
// quickly find one that works. Note that we also use different
127-
// pseudorandom values while doing so in the for-loop above.
128-
hashSize *= 2;
129-
} else {
130-
// Success! Leave the loop.
131-
break;
132-
}
133-
}
73+
UniqueRecGroups unique(wasm.features);
74+
for (auto group : existing) {
75+
std::vector<HeapType> types(group.begin(), group.end());
76+
[[maybe_unused]] auto uniqueTypes = unique.insert(std::move(types));
77+
assert(uniqueTypes.size() == group.size() && "unexpected collision");
13478
}
13579

136-
#ifndef NDEBUG
137-
// Verify the lack of a collision, just to be safe.
138-
for (auto newType : types) {
139-
assert(!existingSet.count(newType));
80+
auto num = types.size();
81+
std::vector<HeapType> uniqueTypes = unique.insert(std::move(types));
82+
if (uniqueTypes.size() != num) {
83+
// Remove the brand type, which we do not need to consider further.
84+
uniqueTypes.pop_back();
14085
}
141-
#endif
142-
143-
return types;
86+
assert(uniqueTypes.size() == num);
87+
return uniqueTypes;
14488
}
14589

14690
// A vector of struct.new or one of the variations on array.new.
@@ -407,8 +351,7 @@ struct TypeSSA : public Pass {
407351
assert(newTypes.size() == num);
408352

409353
// Make sure this is actually a new rec group.
410-
auto recGroup = newTypes[0].getRecGroup();
411-
newTypes = ensureTypesAreInNewRecGroup(recGroup, *module);
354+
newTypes = ensureTypesAreInNewRecGroup(std::move(newTypes), *module);
412355

413356
// Success: we can apply the new types.
414357

test/lit/passes/type-ssa-shared.wast

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@
8686
;; CHECK: (rec
8787
;; CHECK-NEXT: (type $A_1 (sub $A (shared (array (mut i32)))))
8888

89-
;; CHECK: (type $4 (struct (field (mut i32)) (field (mut i32)) (field (mut f64)) (field (mut f64)) (field (mut i32)) (field (mut f64)) (field (mut f64)) (field (mut i32)) (field (mut i32)) (field (mut i32)) (field (mut i32))))
89+
;; CHECK: (type $4 (struct))
9090

9191
;; CHECK: (func $func (type $1)
9292
;; CHECK-NEXT: (local $local (ref $B))

test/lit/passes/type-ssa.wast

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@
460460
;; CHECK: (rec
461461
;; CHECK-NEXT: (type $array_1 (sub $array (array (mut f32))))
462462

463-
;; CHECK: (type $4 (struct (field (mut i32)) (field (mut i32)) (field (mut f64)) (field (mut f64)) (field (mut i32)) (field (mut f64)) (field (mut f64)) (field (mut i32)) (field (mut i32)) (field (mut i32)) (field (mut i32))))
463+
;; CHECK: (type $4 (struct))
464464

465465
;; CHECK: (func $1 (type $2) (param $ref (ref $subarray))
466466
;; CHECK-NEXT: (drop
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
;; RUN: wasm-opt %s -all --disable-custom-descriptors \
3+
;; RUN: --type-ssa --roundtrip --fuzz-exec -S -o - | filecheck %s
4+
5+
(module
6+
;; CHECK: (type $foo (struct))
7+
(type $foo (struct))
8+
;; CHECK: (type $super (sub (struct (field (ref $foo)))))
9+
(type $super (sub (struct (field (ref $foo)))))
10+
;; CHECK: (type $sub (sub $super (struct (field (ref $foo)))))
11+
(type $sub (sub $super (struct (field (ref (exact $foo))))))
12+
13+
;; CHECK: (type $3 (func (result i32)))
14+
15+
;; CHECK: (rec
16+
;; CHECK-NEXT: (type $super_1 (sub $super (struct (field (ref $foo)))))
17+
18+
;; CHECK: (type $5 (struct))
19+
20+
;; CHECK: (export "test" (func $test))
21+
22+
;; CHECK: (func $test (type $3) (result i32)
23+
;; CHECK-NEXT: (local $sub (ref $sub))
24+
;; CHECK-NEXT: (local $any anyref)
25+
;; CHECK-NEXT: (ref.test (ref $sub)
26+
;; CHECK-NEXT: (select (result anyref)
27+
;; CHECK-NEXT: (struct.new $super_1
28+
;; CHECK-NEXT: (struct.new_default $foo)
29+
;; CHECK-NEXT: )
30+
;; CHECK-NEXT: (local.get $any)
31+
;; CHECK-NEXT: (i32.const 1)
32+
;; CHECK-NEXT: )
33+
;; CHECK-NEXT: )
34+
;; CHECK-NEXT: )
35+
(func $test (export "test") (result i32)
36+
(local $sub (ref $sub))
37+
(local $any anyref)
38+
;; TypeSSA will create another subtype of $super, which will differ from
39+
;; $sub because its field will not be exact. However, since exactness will
40+
;; be erased by the round trip, we still need a brand type to distinguish
41+
;; $sub and the new subtype. If we did not insert a brand type, this
42+
;; ref.test would incorrectly return 1 after optimization.
43+
(ref.test (ref $sub)
44+
;; The select stops the ref.test from being optimized by finalization.
45+
(select (result anyref)
46+
(struct.new $super
47+
(struct.new $foo)
48+
)
49+
(local.get $any)
50+
(i32.const 1)
51+
)
52+
)
53+
)
54+
)

0 commit comments

Comments
 (0)