Skip to content

Commit c9b5f25

Browse files
tlivelytlively
committed
[Custom Descriptors] Fix descriptor subtyping rules
We previously allowed one descriptor type to be a subtype of another descriptor type even if their described types were unrelated, but this was unsound because it let descriptor casts successfully cast the type described by the descriptor subtype to the unrelated type described by the descriptor supertype. Fix the soundness problem by requiring that the type described by the supertype descriptor be the declared supertype of the type described by the subtype descriptor. Update the heap type fuzzer to avoid emitting newly invalid types. Also update tests for various passes to make them valid or delete them if they were testing situations that can no longer happen.
1 parent a6e2620 commit c9b5f25

File tree

7 files changed

+84
-152
lines changed

7 files changed

+84
-152
lines changed

‎src/tools/fuzzing/heap-types.cpp‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -196,24 +196,22 @@ struct HeapTypeGeneratorImpl {
196196
assert(remaining >= numPlannedDescriptors);
197197
size_t remainingUncommitted = remaining - numPlannedDescriptors;
198198

199-
if (!super && i >= numRoots && rand.oneIn(2)) {
200-
// Try to pick a supertype. The supertype must be a descriptor type if and
201-
// only if we are currently generating a descriptor type. Furthermore, we
202-
// must have space left in the current chain if it exists, or else in the
203-
// rec group, to mirror the supertype's descriptor chain, if it has one.
204-
// Finally, if this is a descriptor, the sharedness of the described type
205-
// and supertype must match.
206-
size_t maxChain =
207-
isDescriptor ? descriptorChainLengths[i] : remainingUncommitted;
199+
// Possibly choose a supertype. If the current type is a descriptor type,
200+
// then either we already determined its supertype based on its described
201+
// type, or there is no valid supertype to give it. A valid supertype would
202+
// have to describe the supertype of the current described type, but if the
203+
// supertype chain had such an entry, we would already have found it above.
204+
if (!super && !isDescriptor && i >= numRoots && rand.oneIn(2)) {
205+
// Try to pick a supertype. The supertype must not be a descriptor type,
206+
// and we must have space left in the rec group to mirror the supertype's
207+
// descriptor chain, if it has one.
208208
std::vector<Index> candidates;
209209
candidates.reserve(i);
210210
for (Index candidate = 0; candidate < i; ++candidate) {
211-
bool descMatch = bool(describedIndices[candidate]) == isDescriptor;
212-
bool chainMatch = descriptorChainLengths[candidate] <= maxChain;
213-
bool shareMatch = !isDescriptor ||
214-
HeapType(builder[candidate]).getShared() ==
215-
HeapType(builder[*describedIndices[i]]).getShared();
216-
if (descMatch && chainMatch && shareMatch) {
211+
if (describedIndices[candidate]) {
212+
continue;
213+
}
214+
if (descriptorChainLengths[candidate] <= remainingUncommitted) {
217215
candidates.push_back(candidate);
218216
}
219217
}

‎src/wasm/wasm-type.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2389,10 +2389,12 @@ bool isValidSupertype(const HeapTypeInfo& sub, const HeapTypeInfo& super) {
23892389
return false;
23902390
}
23912391
}
2392-
// A supertype of a type must have a describes clause iff the type has a
2393-
// describes clause.
2394-
if (bool(sub.described) != bool(super.described)) {
2395-
return false;
2392+
// A supertype of a type with a (describes $x) clause must have a (describes
2393+
// $y) clause where $y is the declared supertype of $x.
2394+
if (sub.described) {
2395+
if (!super.described || sub.described->supertype != super.described) {
2396+
return false;
2397+
}
23962398
}
23972399
SubTyper typer;
23982400
switch (sub.kind) {

‎test/lit/fuzz-types.test‎

Lines changed: 44 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,58 +3,62 @@
33
;; CHECK: Running with seed 0
44
;; CHECK-NEXT: Built 20 types:
55
;; CHECK-NEXT: (rec
6-
;; CHECK-NEXT: (type $0 (sub (shared (array (mut i32)))))
7-
;; CHECK-NEXT: (type $1 (sub (shared (array (ref null $7)))))
8-
;; CHECK-NEXT: (type $2 (sub (shared (func (result (ref $0))))))
9-
;; CHECK-NEXT: (type $3 (sub $0 (shared (array (mut i32)))))
10-
;; CHECK-NEXT: (type $4 (sub (array f32)))
11-
;; CHECK-NEXT: (type $5 (sub $0 (shared (array (mut i32)))))
12-
;; CHECK-NEXT: (type $6 (sub $2 (shared (func (result (ref $3))))))
13-
;; CHECK-NEXT: (type $7 (sub (shared (descriptor $9 (struct (field (mut f32)) (field f64) (field (mut i32)))))))
14-
;; CHECK-NEXT: (type $8 (sub $2 (shared (func (result (ref $0))))))
15-
;; CHECK-NEXT: (type $9 (shared (describes $7 (descriptor $12 (struct (field (ref $5)) (field i64) (field (mut f32)) (field (mut (ref null $0))) (field (mut f64)) (field i64))))))
16-
;; CHECK-NEXT: (type $10 (sub (struct (field i8) (field f32))))
17-
;; CHECK-NEXT: (type $11 (sub (shared (struct (field i64) (field v128) (field (mut i16)) (field i32) (field (mut (ref (shared array))))))))
18-
;; CHECK-NEXT: (type $12 (shared (describes $9 (struct))))
6+
;; CHECK-NEXT: (type $0 (sub (shared (array i8))))
7+
;; CHECK-NEXT: (type $1 (shared (array (ref $1))))
8+
;; CHECK-NEXT: (type $2 (sub (shared (func (param i64 (ref null $5) i64) (result (ref $2))))))
9+
;; CHECK-NEXT: (type $3 (sub $0 (shared (array i8))))
10+
;; CHECK-NEXT: (type $4 (sub (array (ref null $9))))
11+
;; CHECK-NEXT: (type $5 (sub final $0 (shared (array i8))))
12+
;; CHECK-NEXT: (type $6 (sub $2 (shared (func (param i64 (ref null (shared eq)) i64) (result (ref $8))))))
13+
;; CHECK-NEXT: (type $7 (sub (shared (descriptor $9 (struct (field (ref null $7)) (field (mut i64)) (field (mut i8)) (field i32) (field (mut i32)))))))
14+
;; CHECK-NEXT: (type $8 (sub $2 (shared (func (param i64 (ref null $0) i64) (result (ref $8))))))
15+
;; CHECK-NEXT: (type $9 (shared (describes $7 (descriptor $12 (struct (field i32) (field (mut (ref $8))) (field f64))))))
16+
;; CHECK-NEXT: (type $10 (sub (array (mut externref))))
17+
;; CHECK-NEXT: (type $11 (sub $4 (array (ref $9))))
18+
;; CHECK-NEXT: (type $12 (sub (shared (describes $9 (struct (field (mut f64)) (field (mut i32)) (field (mut f64)))))))
1919
;; CHECK-NEXT: )
20-
;; CHECK-NEXT: (type $13 (array (ref $11)))
2120
;; CHECK-NEXT: (rec
22-
;; CHECK-NEXT: (type $14 (sub final $10 (struct (field i8) (field f32) (field (ref null (shared any))))))
23-
;; CHECK-NEXT: (type $15 (sub (shared (struct (field i64) (field (ref (shared array))) (field (ref $0)) (field (mut i32)) (field i64)))))
21+
;; CHECK-NEXT: (type $13 (sub (struct (field (mut i16)) (field i64) (field (mut (ref (shared func)))))))
22+
;; CHECK-NEXT: (type $14 (sub (shared (array (ref null $14)))))
2423
;; CHECK-NEXT: )
2524
;; CHECK-NEXT: (rec
26-
;; CHECK-NEXT: (type $16 (sub (shared (struct (field (ref $1))))))
27-
;; CHECK-NEXT: (type $17 (sub $5 (shared (array (mut i32)))))
28-
;; CHECK-NEXT: (type $18 (sub final $3 (shared (array (mut i32)))))
25+
;; CHECK-NEXT: (type $15 (shared (array (ref null (shared extern)))))
26+
;; CHECK-NEXT: (type $16 (sub final $14 (shared (array (ref null $16)))))
27+
;; CHECK-NEXT: )
28+
;; CHECK-NEXT: (rec
29+
;; CHECK-NEXT: (type $17 (shared (struct (field (mut (ref null $2))) (field i32) (field (mut (ref $14))))))
30+
;; CHECK-NEXT: (type $18 (sub $11 (array (ref $9))))
31+
;; CHECK-NEXT: (type $19 (sub $2 (shared (func (param i64 (ref null $0) i64) (result (ref $6))))))
2932
;; CHECK-NEXT: )
30-
;; CHECK-NEXT: (type $19 (sub final $16 (shared (struct (field (ref $1)) (field (mut v128)) (field (mut (ref $17))) (field v128)))))
3133
;; CHECK-NEXT:
3234
;; CHECK-NEXT: Inhabitable types:
3335
;; CHECK-NEXT:
3436
;; CHECK-NEXT: Built 20 types:
3537
;; CHECK-NEXT: (rec
36-
;; CHECK-NEXT: (type $0 (sub (shared (array (mut i32)))))
37-
;; CHECK-NEXT: (type $1 (sub (shared (array (ref null $7)))))
38-
;; CHECK-NEXT: (type $2 (sub (shared (func (result (ref $0))))))
39-
;; CHECK-NEXT: (type $3 (sub $0 (shared (array (mut i32)))))
40-
;; CHECK-NEXT: (type $4 (sub (array f32)))
41-
;; CHECK-NEXT: (type $5 (sub $0 (shared (array (mut i32)))))
42-
;; CHECK-NEXT: (type $6 (sub $2 (shared (func (result (ref $3))))))
43-
;; CHECK-NEXT: (type $7 (sub (shared (descriptor $9 (struct (field (mut f32)) (field f64) (field (mut i32)))))))
44-
;; CHECK-NEXT: (type $8 (sub $2 (shared (func (result (ref $0))))))
45-
;; CHECK-NEXT: (type $9 (shared (describes $7 (descriptor $12 (struct (field (ref $5)) (field i64) (field (mut f32)) (field (mut (ref null $0))) (field (mut f64)) (field i64))))))
46-
;; CHECK-NEXT: (type $10 (sub (struct (field i8) (field f32))))
47-
;; CHECK-NEXT: (type $11 (sub (shared (struct (field i64) (field v128) (field (mut i16)) (field i32) (field (mut (ref (shared array))))))))
48-
;; CHECK-NEXT: (type $12 (shared (describes $9 (struct))))
38+
;; CHECK-NEXT: (type $0 (sub (shared (array i8))))
39+
;; CHECK-NEXT: (type $1 (shared (array (ref null $1))))
40+
;; CHECK-NEXT: (type $2 (sub (shared (func (param i64 (ref null $5) i64) (result (ref $2))))))
41+
;; CHECK-NEXT: (type $3 (sub $0 (shared (array i8))))
42+
;; CHECK-NEXT: (type $4 (sub (array (ref null $9))))
43+
;; CHECK-NEXT: (type $5 (sub final $0 (shared (array i8))))
44+
;; CHECK-NEXT: (type $6 (sub $2 (shared (func (param i64 (ref null (shared eq)) i64) (result (ref $8))))))
45+
;; CHECK-NEXT: (type $7 (sub (shared (descriptor $9 (struct (field (ref null $7)) (field (mut i64)) (field (mut i8)) (field i32) (field (mut i32)))))))
46+
;; CHECK-NEXT: (type $8 (sub $2 (shared (func (param i64 (ref null $0) i64) (result (ref $8))))))
47+
;; CHECK-NEXT: (type $9 (shared (describes $7 (descriptor $12 (struct (field i32) (field (mut (ref $8))) (field f64))))))
48+
;; CHECK-NEXT: (type $10 (sub (array (mut externref))))
49+
;; CHECK-NEXT: (type $11 (sub $4 (array (ref $9))))
50+
;; CHECK-NEXT: (type $12 (sub (shared (describes $9 (struct (field (mut f64)) (field (mut i32)) (field (mut f64)))))))
4951
;; CHECK-NEXT: )
50-
;; CHECK-NEXT: (type $13 (array (ref $11)))
5152
;; CHECK-NEXT: (rec
52-
;; CHECK-NEXT: (type $14 (sub final $10 (struct (field i8) (field f32) (field (ref null (shared any))))))
53-
;; CHECK-NEXT: (type $15 (sub (shared (struct (field i64) (field (ref (shared array))) (field (ref $0)) (field (mut i32)) (field i64)))))
53+
;; CHECK-NEXT: (type $13 (sub (struct (field (mut i16)) (field i64) (field (mut (ref (shared func)))))))
54+
;; CHECK-NEXT: (type $14 (sub (shared (array (ref null $14)))))
5455
;; CHECK-NEXT: )
5556
;; CHECK-NEXT: (rec
56-
;; CHECK-NEXT: (type $16 (sub (shared (struct (field (ref $1))))))
57-
;; CHECK-NEXT: (type $17 (sub $5 (shared (array (mut i32)))))
58-
;; CHECK-NEXT: (type $18 (sub final $3 (shared (array (mut i32)))))
57+
;; CHECK-NEXT: (type $15 (shared (array (ref null (shared extern)))))
58+
;; CHECK-NEXT: (type $16 (sub final $14 (shared (array (ref null $16)))))
5959
;; CHECK-NEXT: )
60-
;; CHECK-NEXT: (type $19 (sub final $16 (shared (struct (field (ref $1)) (field (mut v128)) (field (mut (ref $17))) (field v128)))))
60+
;; CHECK-NEXT: (rec
61+
;; CHECK-NEXT: (type $17 (shared (struct (field (mut (ref null $2))) (field i32) (field (mut (ref $14))))))
62+
;; CHECK-NEXT: (type $18 (sub $11 (array (ref $9))))
63+
;; CHECK-NEXT: (type $19 (sub $2 (shared (func (param i64 (ref null $0) i64) (result (ref $6))))))
64+
;; CHECK-NEXT: )

‎test/lit/passes/abstract-type-refining-desc.wast‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -850,19 +850,19 @@
850850
;; invalid allocations of its described type.
851851
(rec
852852
;; YESTNH: (rec
853-
;; YESTNH-NEXT: (type $struct (descriptor $uninstantiated (struct)))
853+
;; YESTNH-NEXT: (type $struct (sub (descriptor $uninstantiated (struct))))
854854
;; NO_TNH: (rec
855-
;; NO_TNH-NEXT: (type $struct (descriptor $uninstantiated (struct)))
856-
(type $struct (descriptor $uninstantiated (struct)))
855+
;; NO_TNH-NEXT: (type $struct (sub (descriptor $uninstantiated (struct))))
856+
(type $struct (sub (descriptor $uninstantiated (struct))))
857857
;; YESTNH: (type $uninstantiated (sub (describes $struct (struct))))
858858
;; NO_TNH: (type $uninstantiated (sub (describes $struct (struct))))
859859
(type $uninstantiated (sub (describes $struct (struct))))
860-
;; YESTNH: (type $other (descriptor $instantiated (struct)))
861-
;; NO_TNH: (type $other (descriptor $instantiated (struct)))
862-
(type $other (descriptor $instantiated (struct)))
863-
;; YESTNH: (type $instantiated (sub $uninstantiated (describes $other (struct))))
864-
;; NO_TNH: (type $instantiated (sub $uninstantiated (describes $other (struct))))
865-
(type $instantiated (sub $uninstantiated (describes $other (struct))))
860+
;; YESTNH: (type $sub (sub $struct (descriptor $instantiated (struct))))
861+
;; NO_TNH: (type $sub (sub $struct (descriptor $instantiated (struct))))
862+
(type $sub (sub $struct (descriptor $instantiated (struct))))
863+
;; YESTNH: (type $instantiated (sub $uninstantiated (describes $sub (struct))))
864+
;; NO_TNH: (type $instantiated (sub $uninstantiated (describes $sub (struct))))
865+
(type $instantiated (sub $uninstantiated (describes $sub (struct))))
866866
)
867867

868868
;; YESTNH: (type $4 (func (result (ref $struct))))

‎test/lit/passes/type-merging-desc.wast‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -434,59 +434,3 @@
434434
;; CHECK: (export "public" (global $public))
435435
(export "public" (global $public))
436436
)
437-
438-
(module
439-
;; In principle we could merge the $B chain into the $A chain, because $A and
440-
;; $B are mergeable as siblings and $B.desc is mergeable into its supertype
441-
;; A.desc. However we do not currently do this kind of merge in either phase.
442-
(rec
443-
;; CHECK: (rec
444-
;; CHECK-NEXT: (type $A (sub (descriptor $A.desc (struct))))
445-
(type $A (sub (descriptor $A.desc (struct))))
446-
;; CHECK: (type $A.desc (sub (describes $A (struct))))
447-
(type $A.desc (sub (describes $A (struct))))
448-
;; CHECK: (type $B (sub (descriptor $B.desc (struct))))
449-
(type $B (sub (descriptor $B.desc (struct))))
450-
;; CHECK: (type $B.desc (sub $A.desc (describes $B (struct))))
451-
(type $B.desc (sub $A.desc (describes $B (struct))))
452-
)
453-
454-
;; CHECK: (global $B.desc (ref null $B.desc) (ref.null none))
455-
(global $B.desc (ref null $B.desc) (ref.null none))
456-
)
457-
458-
(module
459-
(rec
460-
;; Like above, but now we have an unrelated C chain. We have these chains:
461-
;;
462-
;; A -> A.desc
463-
;; ^
464-
;; B -> B.desc
465-
;;
466-
;; C -> C.desc
467-
;;
468-
;; The C chain should not be considered a sibling of the B chain because
469-
;; C.desc is not a subtype of A.desc, even though they would look like
470-
;; siblings if you only considered the base type in the chain.
471-
;; CHECK: (rec
472-
;; CHECK-NEXT: (type $A (descriptor $A.desc (struct)))
473-
(type $A (descriptor $A.desc (struct)))
474-
;; CHECK: (type $A.desc (sub (describes $A (struct))))
475-
(type $A.desc (sub (describes $A (struct))))
476-
;; CHECK: (type $B (descriptor $B.desc (struct)))
477-
(type $B (descriptor $B.desc (struct)))
478-
;; CHECK: (type $B.desc (sub $A.desc (describes $B (struct))))
479-
(type $B.desc (sub $A.desc (describes $B (struct))))
480-
;; CHECK: (type $C (descriptor $C.desc (struct)))
481-
(type $C (descriptor $C.desc (struct)))
482-
;; CHECK: (type $C.desc (describes $C (struct)))
483-
(type $C.desc (describes $C (struct)))
484-
)
485-
486-
;; CHECK: (global $C (ref null $C) (ref.null none))
487-
(global $C (ref null $C) (ref.null none))
488-
489-
;; This would become invalid if $B.desc were merged into $C.desc.
490-
;; CHECK: (global $A.desc (ref null $A.desc) (struct.new_default $B.desc))
491-
(global $A.desc (ref null $A.desc) (struct.new_default $B.desc))
492-
)

‎test/lit/passes/unsubtyping-desc.wast‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,22 +1242,3 @@
12421242
)
12431243
)
12441244
)
1245-
1246-
(module
1247-
;; This will be invalid soon, but in the meantime we should not be confused when
1248-
;; the types described by two related descriptors are unrelated.
1249-
(rec
1250-
;; CHECK: (rec
1251-
;; CHECK-NEXT: (type $A (descriptor $super (struct)))
1252-
(type $A (descriptor $super (struct)))
1253-
;; CHECK: (type $B (descriptor $sub (struct)))
1254-
(type $B (descriptor $sub (struct)))
1255-
;; CHECK: (type $super (sub (describes $A (struct))))
1256-
(type $super (sub (describes $A (struct))))
1257-
;; CHECK: (type $sub (sub $super (describes $B (struct))))
1258-
(type $sub (sub $super (describes $B (struct))))
1259-
)
1260-
;; CHECK: (global $public (ref null $B) (ref.null none))
1261-
(global $public (export "public") (ref null $B) (ref.null none))
1262-
)
1263-
;; CHECK: (export "public" (global $public))

‎test/spec/descriptors.wast‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,16 +95,19 @@
9595
)
9696
)
9797

98-
(module
99-
(type $super (sub (struct)))
100-
(rec
101-
(type $other (sub (descriptor $super-desc (struct))))
102-
(type $super-desc (sub (describes $other (struct))))
103-
)
104-
(rec
105-
(type $sub (sub $super (descriptor $sub-desc (struct))))
106-
(type $sub-desc (sub $super-desc (describes $sub (struct))))
98+
(assert_invalid
99+
(module
100+
(type $super (sub (struct)))
101+
(rec
102+
(type $other (sub (descriptor $super-desc (struct))))
103+
(type $super-desc (sub (describes $other (struct))))
104+
)
105+
(rec
106+
(type $sub (sub $super (descriptor $sub-desc (struct))))
107+
(type $sub-desc (sub $super-desc (describes $sub (struct))))
108+
)
107109
)
110+
"supertype of descriptor type must describe supertype of described type"
108111
)
109112

110113
(assert_invalid

0 commit comments

Comments
 (0)