Skip to content

Commit 53ab5f5

Browse files
tlivelytlively
authored
[Custom Descriptors] Fix allocations with optimized descs in AbstractTypeRefining (#7769)
Previously When AbstractTypeRefining optimized a descriptor type, it left any allocation of its described type with an invalid, mismatched descriptor type. Avoid this problem by preoptimizing these allocations, since we know they cannot possibly succeed. In function contexts, we can drop their children and replace them with unreachables. Outside function contexts, we can replace the descriptors will null values.
1 parent 981cf69 commit 53ab5f5

File tree

2 files changed

+237
-31
lines changed

2 files changed

+237
-31
lines changed

‎src/passes/AbstractTypeRefining.cpp

Lines changed: 61 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535

3636
#include <memory>
3737

38+
#include "ir/drop.h"
3839
#include "ir/localize.h"
3940
#include "ir/module-utils.h"
4041
#include "ir/subtypes.h"
@@ -269,7 +270,9 @@ struct AbstractTypeRefining : public Pass {
269270
return;
270271
}
271272

272-
fixupCasts(*module, mapping);
273+
// We may need to apply some preliminary optimizations to ensure we maintain
274+
// validity and correctness when we rewrite the types.
275+
preoptimize(*module, mapping);
273276

274277
// Rewriting types can usually rewrite subtype relationships. For example,
275278
// if we have this:
@@ -302,44 +305,22 @@ struct AbstractTypeRefining : public Pass {
302305
ReFinalize().run(getPassRunner(), module);
303306
}
304307

305-
void fixupCasts(Module& module, const TypeMapper::TypeUpdates& mapping) {
308+
void preoptimize(Module& module, const TypeMapper::TypeUpdates& mapping) {
306309
if (!module.features.hasCustomDescriptors()) {
307-
// No descriptor or exact casts to fix up.
310+
// No descriptor casts, exact casts, or allocations with descriptors to
311+
// fix up.
308312
return;
309313
}
310314

311-
// We may have casts like this:
312-
//
313-
// (ref.cast_desc (ref null $optimized-to-bottom)
314-
// (some struct...)
315-
// (some desc...)
316-
// )
317-
//
318-
// We will optimize the cast target to nullref, but then ReFinalize would
319-
// fix up the cast target back to $optimized-to-bottom. Optimize out the
320-
// cast (which we know must either be a null check or unconditional trap)
321-
// to avoid this reintroduction of the optimized type.
322-
//
323-
// Separately, we may have exact casts like this:
324-
//
325-
// (br_on_cast anyref $l (ref (exact $uninstantiated)) ... )
326-
//
327-
// We know such casts will fail (or will pass only for null values), but
328-
// with traps-never-happen, we might optimize them to this:
329-
//
330-
// (br_on_cast anyref $l (ref (exact $instantiated-subtype)) ... )
331-
//
332-
// This might cause the casts to incorrectly start succeeding. To avoid
333-
// that, optimize them out first.
334-
struct CastFixer : WalkerPass<PostWalker<CastFixer>> {
315+
struct Preoptimizer : WalkerPass<PostWalker<Preoptimizer>> {
335316
const TypeMapper::TypeUpdates& mapping;
336317

337-
CastFixer(const TypeMapper::TypeUpdates& mapping) : mapping(mapping) {}
318+
Preoptimizer(const TypeMapper::TypeUpdates& mapping) : mapping(mapping) {}
338319

339320
bool isFunctionParallel() override { return true; }
340321

341322
std::unique_ptr<Pass> create() override {
342-
return std::make_unique<CastFixer>(mapping);
323+
return std::make_unique<Preoptimizer>(mapping);
343324
}
344325

345326
Block* localizeChildren(Expression* curr) {
@@ -361,6 +342,29 @@ struct AbstractTypeRefining : public Pass {
361342
return it->second;
362343
}
363344

345+
// We may have casts like this:
346+
//
347+
// (ref.cast_desc (ref null $optimized-to-bottom)
348+
// (some struct...)
349+
// (some desc...)
350+
// )
351+
//
352+
// We will optimize the cast target to nullref, but then ReFinalize would
353+
// fix up the cast target back to $optimized-to-bottom. Optimize out the
354+
// cast (which we know must either be a null check or unconditional trap)
355+
// to avoid this reintroduction of the optimized type.
356+
//
357+
// Separately, we may have exact casts like this:
358+
//
359+
// (br_on_cast anyref $l (ref (exact $uninstantiated)) ... )
360+
//
361+
// We know such casts will fail (or will pass only for null values), but
362+
// with traps-never-happen, we might optimize them to this:
363+
//
364+
// (br_on_cast anyref $l (ref (exact $instantiated-subtype)) ... )
365+
//
366+
// This might cause the casts to incorrectly start succeeding. To avoid
367+
// that, optimize them out first.
364368
void visitRefCast(RefCast* curr) {
365369
auto optimized = getOptimized(curr->type);
366370
if (!optimized) {
@@ -426,8 +430,34 @@ struct AbstractTypeRefining : public Pass {
426430
}
427431
}
428432
}
429-
} fixer(mapping);
430-
fixer.run(getPassRunner(), &module);
433+
434+
void visitStructNew(StructNew* curr) {
435+
if (!curr->desc) {
436+
return;
437+
}
438+
auto optimized = getOptimized(curr->desc->type);
439+
if (!optimized) {
440+
return;
441+
}
442+
// The descriptor type is not instantiated, so there is no way this
443+
// allocation can succeed. We need to remove it to avoid leaving it with
444+
// a mismatched descriptor type after type rewriting. If we are in a
445+
// function context, replace it with unreachable, taking care to
446+
// preserve any side effects. If we're not in a function context, then
447+
// we cannot use things like blocks or drops, but there are also no
448+
// effects besides traps, so we can just replace the descriptor with a
449+
// null.
450+
Builder builder(*getModule());
451+
if (getFunction()) {
452+
replaceCurrent(getDroppedChildrenAndAppend(
453+
curr, *getModule(), getPassOptions(), builder.makeUnreachable()));
454+
} else {
455+
curr->desc = builder.makeRefNull(HeapType::none);
456+
}
457+
}
458+
} preoptimizer(mapping);
459+
preoptimizer.run(getPassRunner(), &module);
460+
preoptimizer.runOnModuleCode(getPassRunner(), &module);
431461
}
432462
};
433463

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

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,3 +844,179 @@
844844
(unreachable)
845845
)
846846
)
847+
848+
(module
849+
;; We will optimize the descriptor type, so we must take care not to leave
850+
;; invalid allocations of its described type.
851+
(rec
852+
;; YESTNH: (rec
853+
;; YESTNH-NEXT: (type $struct (descriptor $uninstantiated (struct)))
854+
;; NO_TNH: (rec
855+
;; NO_TNH-NEXT: (type $struct (descriptor $uninstantiated (struct)))
856+
(type $struct (descriptor $uninstantiated (struct)))
857+
;; YESTNH: (type $uninstantiated (sub (describes $struct (struct))))
858+
;; NO_TNH: (type $uninstantiated (sub (describes $struct (struct))))
859+
(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))))
866+
)
867+
868+
;; YESTNH: (type $4 (func (result (ref $struct))))
869+
870+
;; YESTNH: (type $5 (func))
871+
872+
;; YESTNH: (import "" "" (func $effect (type $5)))
873+
;; NO_TNH: (type $4 (func (result (ref $struct))))
874+
875+
;; NO_TNH: (type $5 (func))
876+
877+
;; NO_TNH: (import "" "" (func $effect (type $5)))
878+
(import "" "" (func $effect))
879+
880+
;; YESTNH: (global $instantiated (ref $instantiated) (struct.new_default $instantiated))
881+
;; NO_TNH: (global $instantiated (ref $instantiated) (struct.new_default $instantiated))
882+
(global $instantiated (ref $instantiated) (struct.new $instantiated))
883+
884+
;; YESTNH: (global $fake-desc (ref null (exact $instantiated)) (ref.null none))
885+
;; NO_TNH: (global $fake-desc (ref null (exact $uninstantiated)) (ref.null none))
886+
(global $fake-desc (ref null (exact $uninstantiated)) (ref.null none))
887+
888+
;; YESTNH: (global $impossible (ref $struct) (struct.new_default $struct
889+
;; YESTNH-NEXT: (ref.null none)
890+
;; YESTNH-NEXT: ))
891+
;; NO_TNH: (global $impossible (ref $struct) (struct.new_default $struct
892+
;; NO_TNH-NEXT: (global.get $fake-desc)
893+
;; NO_TNH-NEXT: ))
894+
(global $impossible (ref $struct)
895+
(struct.new $struct
896+
(global.get $fake-desc)
897+
)
898+
)
899+
900+
;; YESTNH: (func $impossible (type $4) (result (ref $struct))
901+
;; YESTNH-NEXT: (unreachable)
902+
;; YESTNH-NEXT: )
903+
;; NO_TNH: (func $impossible (type $4) (result (ref $struct))
904+
;; NO_TNH-NEXT: (struct.new_default $struct
905+
;; NO_TNH-NEXT: (global.get $fake-desc)
906+
;; NO_TNH-NEXT: )
907+
;; NO_TNH-NEXT: )
908+
(func $impossible (result (ref $struct))
909+
(struct.new $struct
910+
(global.get $fake-desc)
911+
)
912+
)
913+
914+
;; YESTNH: (func $impossible-effect (type $4) (result (ref $struct))
915+
;; YESTNH-NEXT: (drop
916+
;; YESTNH-NEXT: (block (result (ref null (exact $instantiated)))
917+
;; YESTNH-NEXT: (call $effect)
918+
;; YESTNH-NEXT: (global.get $fake-desc)
919+
;; YESTNH-NEXT: )
920+
;; YESTNH-NEXT: )
921+
;; YESTNH-NEXT: (unreachable)
922+
;; YESTNH-NEXT: )
923+
;; NO_TNH: (func $impossible-effect (type $4) (result (ref $struct))
924+
;; NO_TNH-NEXT: (struct.new_default $struct
925+
;; NO_TNH-NEXT: (block (result (ref null (exact $uninstantiated)))
926+
;; NO_TNH-NEXT: (call $effect)
927+
;; NO_TNH-NEXT: (global.get $fake-desc)
928+
;; NO_TNH-NEXT: )
929+
;; NO_TNH-NEXT: )
930+
;; NO_TNH-NEXT: )
931+
(func $impossible-effect (result (ref $struct))
932+
(struct.new $struct
933+
(block (result (ref null (exact $uninstantiated)))
934+
(call $effect)
935+
(global.get $fake-desc)
936+
)
937+
)
938+
)
939+
)
940+
941+
(module
942+
;; Same, but now we're optimizing the descriptor to bottom, so we don't need
943+
;; to do any preoptimization to ensure validity. We optimize anyway because
944+
;; we can.
945+
(rec
946+
;; YESTNH: (rec
947+
;; YESTNH-NEXT: (type $struct (descriptor $uninstantiated (struct)))
948+
;; NO_TNH: (rec
949+
;; NO_TNH-NEXT: (type $struct (descriptor $uninstantiated (struct)))
950+
(type $struct (descriptor $uninstantiated (struct)))
951+
;; YESTNH: (type $uninstantiated (describes $struct (struct)))
952+
;; NO_TNH: (type $uninstantiated (describes $struct (struct)))
953+
(type $uninstantiated (describes $struct (struct)))
954+
)
955+
956+
;; YESTNH: (type $2 (func (result (ref $struct))))
957+
958+
;; YESTNH: (type $3 (func))
959+
960+
;; YESTNH: (import "" "" (func $effect (type $3)))
961+
;; NO_TNH: (type $2 (func (result (ref $struct))))
962+
963+
;; NO_TNH: (type $3 (func))
964+
965+
;; NO_TNH: (import "" "" (func $effect (type $3)))
966+
(import "" "" (func $effect))
967+
968+
;; YESTNH: (global $fake-desc nullref (ref.null none))
969+
;; NO_TNH: (global $fake-desc nullref (ref.null none))
970+
(global $fake-desc (ref null (exact $uninstantiated)) (ref.null none))
971+
972+
;; YESTNH: (global $impossible (ref $struct) (struct.new_default $struct
973+
;; YESTNH-NEXT: (ref.null none)
974+
;; YESTNH-NEXT: ))
975+
;; NO_TNH: (global $impossible (ref $struct) (struct.new_default $struct
976+
;; NO_TNH-NEXT: (ref.null none)
977+
;; NO_TNH-NEXT: ))
978+
(global $impossible (ref $struct)
979+
(struct.new $struct
980+
(global.get $fake-desc)
981+
)
982+
)
983+
984+
;; YESTNH: (func $impossible (type $2) (result (ref $struct))
985+
;; YESTNH-NEXT: (unreachable)
986+
;; YESTNH-NEXT: )
987+
;; NO_TNH: (func $impossible (type $2) (result (ref $struct))
988+
;; NO_TNH-NEXT: (unreachable)
989+
;; NO_TNH-NEXT: )
990+
(func $impossible (result (ref $struct))
991+
(struct.new $struct
992+
(global.get $fake-desc)
993+
)
994+
)
995+
996+
;; YESTNH: (func $impossible-effect (type $2) (result (ref $struct))
997+
;; YESTNH-NEXT: (drop
998+
;; YESTNH-NEXT: (block (result nullref)
999+
;; YESTNH-NEXT: (call $effect)
1000+
;; YESTNH-NEXT: (global.get $fake-desc)
1001+
;; YESTNH-NEXT: )
1002+
;; YESTNH-NEXT: )
1003+
;; YESTNH-NEXT: (unreachable)
1004+
;; YESTNH-NEXT: )
1005+
;; NO_TNH: (func $impossible-effect (type $2) (result (ref $struct))
1006+
;; NO_TNH-NEXT: (drop
1007+
;; NO_TNH-NEXT: (block (result nullref)
1008+
;; NO_TNH-NEXT: (call $effect)
1009+
;; NO_TNH-NEXT: (global.get $fake-desc)
1010+
;; NO_TNH-NEXT: )
1011+
;; NO_TNH-NEXT: )
1012+
;; NO_TNH-NEXT: (unreachable)
1013+
;; NO_TNH-NEXT: )
1014+
(func $impossible-effect (result (ref $struct))
1015+
(struct.new $struct
1016+
(block (result (ref null (exact $uninstantiated)))
1017+
(call $effect)
1018+
(global.get $fake-desc)
1019+
)
1020+
)
1021+
)
1022+
)

0 commit comments

Comments
 (0)