Skip to content

Commit f8e164d

Browse files
tlivelytlively
authored
[Custom Descriptors] Edge case in Heap2Local (#7755)
When Heap2Local optimized an allocation that flowed into a ref.cast_desc that admitted nulls as the cast descriptor, it reasoned that the only way the original cast could have succeeded was if the cast reference was null, so Heap2Local optimized the cast to a drop of the descriptor operand followed by a cast to null of the reference operand. This missed the edge case where the same allocation flows in as both the reference and descriptor operands. In that case, the optimized allocation passed the cast to null even though the original cast would have failed. Fix the bug by only using the code path that produces the null cast when the optimized allocation does not also flow into the cast as the reference operand. Casts where the same allocations flows in as both operands are now optimized to a sequence ending in unreachable. We had test coverage for the same allocation flowing in as both the reference and descriptor operands, but only when the cast was to a non-nullable type. Add coverage for nullable casts, and also add new versions of the tests that exercise a slightly different combination of conditions leading to the same code path.
1 parent 4ead624 commit f8e164d

File tree

2 files changed

+108
-3
lines changed

2 files changed

+108
-3
lines changed

‎src/passes/Heap2Local.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,8 @@ struct Struct2Local : PostWalker<Struct2Local> {
859859
// if the optimized allocation flows in as the descriptor, since it cannot
860860
// possibly have been used in the allocation of the cast value without
861861
// having been considered to escape.
862+
bool allocIsCastRef =
863+
analyzer.getInteraction(curr->ref) == ParentChildInteraction::Flows;
862864
bool allocIsCastDesc =
863865
analyzer.getInteraction(curr->desc) == ParentChildInteraction::Flows;
864866
if (!allocation->desc || allocIsCastDesc) {
@@ -867,7 +869,7 @@ struct Struct2Local : PostWalker<Struct2Local> {
867869
// side effects, but that local.set would not be reflected in the parent
868870
// map, so it would not be updated if the allocation flowing through
869871
// that desc operand were later optimized.
870-
if (allocIsCastDesc && curr->type.isNullable()) {
872+
if (allocIsCastDesc && !allocIsCastRef && curr->type.isNullable()) {
871873
// There might be a null value to let through. Reuse curr as a cast to
872874
// null. Use a scratch local to move the reference value past the desc
873875
// value.
@@ -887,8 +889,7 @@ struct Struct2Local : PostWalker<Struct2Local> {
887889
builder.makeUnreachable()));
888890
}
889891
} else {
890-
assert(analyzer.getInteraction(curr->ref) ==
891-
ParentChildInteraction::Flows);
892+
assert(allocIsCastRef);
892893
// The cast succeeds iff the optimized allocation's descriptor is the
893894
// same as the given descriptor and traps otherwise.
894895
auto type = allocation->desc->type;

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

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,110 @@
685685
)
686686
)
687687

688+
;; CHECK: (func $cast-desc-and-ref-nullable (type $14) (param $desc (ref null (exact $chain-descriptor)))
689+
;; CHECK-NEXT: (local $middle (ref null (exact $chain-middle)))
690+
;; CHECK-NEXT: (local $2 (ref null (exact $chain-descriptor)))
691+
;; CHECK-NEXT: (local $3 (ref null (exact $chain-descriptor)))
692+
;; CHECK-NEXT: (drop
693+
;; CHECK-NEXT: (block (result nullref)
694+
;; CHECK-NEXT: (local.set $3
695+
;; CHECK-NEXT: (ref.as_non_null
696+
;; CHECK-NEXT: (local.get $desc)
697+
;; CHECK-NEXT: )
698+
;; CHECK-NEXT: )
699+
;; CHECK-NEXT: (local.set $2
700+
;; CHECK-NEXT: (local.get $3)
701+
;; CHECK-NEXT: )
702+
;; CHECK-NEXT: (ref.null none)
703+
;; CHECK-NEXT: )
704+
;; CHECK-NEXT: )
705+
;; CHECK-NEXT: (drop
706+
;; CHECK-NEXT: (block
707+
;; CHECK-NEXT: (drop
708+
;; CHECK-NEXT: (ref.null none)
709+
;; CHECK-NEXT: )
710+
;; CHECK-NEXT: (drop
711+
;; CHECK-NEXT: (ref.null none)
712+
;; CHECK-NEXT: )
713+
;; CHECK-NEXT: (unreachable)
714+
;; CHECK-NEXT: )
715+
;; CHECK-NEXT: )
716+
;; CHECK-NEXT: )
717+
(func $cast-desc-and-ref-nullable (param $desc (ref null (exact $chain-descriptor)))
718+
;; Same, but now the cast allows nulls. It should still trap.
719+
(local $middle (ref null (exact $chain-middle)))
720+
(local.set $middle
721+
(struct.new $chain-middle
722+
(local.get $desc)
723+
)
724+
)
725+
(drop
726+
(ref.cast_desc (ref null (exact $chain-described))
727+
(local.get $middle)
728+
(local.get $middle)
729+
)
730+
)
731+
)
732+
733+
;; CHECK: (func $cast-desc-and-ref-tee (type $10)
734+
;; CHECK-NEXT: (local $desc (ref null (exact $super.desc)))
735+
;; CHECK-NEXT: (drop
736+
;; CHECK-NEXT: (block
737+
;; CHECK-NEXT: (drop
738+
;; CHECK-NEXT: (block (result nullref)
739+
;; CHECK-NEXT: (ref.null none)
740+
;; CHECK-NEXT: )
741+
;; CHECK-NEXT: )
742+
;; CHECK-NEXT: (drop
743+
;; CHECK-NEXT: (ref.null none)
744+
;; CHECK-NEXT: )
745+
;; CHECK-NEXT: (unreachable)
746+
;; CHECK-NEXT: )
747+
;; CHECK-NEXT: )
748+
;; CHECK-NEXT: )
749+
(func $cast-desc-and-ref-tee
750+
;; The same allocation flows into both the descriptor and the reference
751+
;; again, but now it uses a tee. The allocation does not have a descriptor.
752+
(local $desc (ref null (exact $super.desc)))
753+
(drop
754+
(ref.cast_desc (ref (exact $super))
755+
(local.tee $desc
756+
(struct.new $super.desc)
757+
)
758+
(local.get $desc)
759+
)
760+
)
761+
)
762+
763+
;; CHECK: (func $cast-desc-and-ref-tee-nullable (type $10)
764+
;; CHECK-NEXT: (local $desc (ref null (exact $super.desc)))
765+
;; CHECK-NEXT: (drop
766+
;; CHECK-NEXT: (block
767+
;; CHECK-NEXT: (drop
768+
;; CHECK-NEXT: (block (result nullref)
769+
;; CHECK-NEXT: (ref.null none)
770+
;; CHECK-NEXT: )
771+
;; CHECK-NEXT: )
772+
;; CHECK-NEXT: (drop
773+
;; CHECK-NEXT: (ref.null none)
774+
;; CHECK-NEXT: )
775+
;; CHECK-NEXT: (unreachable)
776+
;; CHECK-NEXT: )
777+
;; CHECK-NEXT: )
778+
;; CHECK-NEXT: )
779+
(func $cast-desc-and-ref-tee-nullable
780+
;; Same, but the cast allows nulls. It should still trap.
781+
(local $desc (ref null (exact $super.desc)))
782+
(drop
783+
(ref.cast_desc (ref null (exact $super))
784+
(local.tee $desc
785+
(struct.new $super.desc)
786+
)
787+
(local.get $desc)
788+
)
789+
)
790+
)
791+
688792
;; CHECK: (func $cast-desc-stale-parent (type $15) (result (ref (exact $super)))
689793
;; CHECK-NEXT: (drop
690794
;; CHECK-NEXT: (block (result nullref)

0 commit comments

Comments
 (0)