Skip to content

Heap2Local: Properly handle failing array casts #6772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kripken merged 6 commits into from
Jul 18, 2024
Merged

Heap2Local: Properly handle failing array casts #6772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ba8b69c
yikes
kripken Jul 18, 2024
572c455
fix
kripken Jul 18, 2024
cf70314
work
kripken Jul 18, 2024
59c5cdb
test
kripken Jul 18, 2024
1698ccd
Update test/lit/passes/heap2local.wast
kripken Jul 18, 2024
125cbe5
fix bad comment
kripken Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 39 additions & 5 deletions src/passes/Heap2Local.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,11 @@ struct Array2Struct : PostWalker<Array2Struct> {
// The original type of the allocation, before we turn it into a struct.
Type originalType;

// The type of the struct we are changing to (nullable and non-nullable
// variations).
Type nullStruct;
Type nonNullStruct;

Array2Struct(Expression* allocation,
EscapeAnalyzer& analyzer,
Function* func,
Expand Down Expand Up @@ -928,9 +933,15 @@ struct Array2Struct : PostWalker<Array2Struct> {
// lowered away to locals anyhow.
auto nullArray = Type(arrayType, Nullable);
auto nonNullArray = Type(arrayType, NonNullable);
auto nullStruct = Type(structType, Nullable);
auto nonNullStruct = Type(structType, NonNullable);
nullStruct = Type(structType, Nullable);
nonNullStruct = Type(structType, NonNullable);
for (auto* reached : analyzer.reached) {
if (reached->is<RefCast>()) {
// Casts must be handled later: We need to see the old type, and to
// potentially replace the cast based on that, see below.
continue;
}

// We must check subtyping here because the allocation may be upcast as it
// flows around. If we do see such upcasting then we are refining here and
// must refinalize.
Expand Down Expand Up @@ -1032,15 +1043,14 @@ struct Array2Struct : PostWalker<Array2Struct> {
}

// Some additional operations need special handling

void visitRefTest(RefTest* curr) {
if (!analyzer.reached.count(curr)) {
return;
}

// When we ref.test an array allocation, we cannot simply turn the array
// into a struct, as then the test will behave different. (Note that this is
// not a problem for ref.*cast*, as the cast simply goes away when the value
// flows through, and we verify it will do so in the escape analysis.) To
// into a struct, as then the test will behave differently. To properly
// handle this, check if the test succeeds or not, and write out the outcome
// here (similar to Struct2Local::visitRefTest). Note that we test on
// |originalType| here and not |allocation->type|, as the allocation has
Expand All @@ -1050,6 +1060,30 @@ struct Array2Struct : PostWalker<Array2Struct> {
builder.makeConst(Literal(result))));
}

void visitRefCast(RefCast* curr) {
Copy link
Member

@tlively tlively Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the comment saying "this is not a problem for ref.cast" above need to be edited?

Copy link
Member Author

@kripken kripken Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed.

The cast does go away when we optimize, but now that we allow casts to fail, we do need to know if it fails or not...

if (!analyzer.reached.count(curr)) {
return;
}

// As with RefTest, we need to check if the cast succeeds with the array
// type before we turn it into a struct type (as after that change, the
// outcome of the cast will look different).
if (!Type::isSubType(originalType, curr->type)) {
// The cast fails, ensure we trap with an unreachable.
replaceCurrent(builder.makeSequence(builder.makeDrop(curr),
builder.makeUnreachable()));
} else {
// The cast succeeds. Update the type. (It is ok to use the non-nullable
// type here unconditionally, since we know the allocation flows through
// here, and anyhow we will be removing the reference during Struct2Local,
// later.)
curr->type = nonNullStruct;
}

// Regardless of how we altered the type here, refinalize.
refinalize = true;
}

// Get the value in an expression we know must contain a constant index.
Index getIndex(Expression* curr) {
return curr->cast<Const>()->value.getUnsigned();
Expand Down
125 changes: 125 additions & 0 deletions test/lit/passes/heap2local.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4303,3 +4303,128 @@
)
)
)

;; Array casts to structs and arrays.
(module
;; CHECK: (type $1 (func))

;; CHECK: (type $0 (func (result (ref struct))))
(type $0 (func (result (ref struct))))
;; CHECK: (type $3 (func (result structref)))

;; CHECK: (type $4 (func (result (ref array))))

;; CHECK: (type $array (array i8))
(type $array (array i8))

;; CHECK: (func $array.cast.struct (type $0) (result (ref struct))
;; CHECK-NEXT: (local $eq (ref eq))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
(func $array.cast.struct (result (ref struct))
(local $eq (ref eq))
;; This cast will fail: we cast an array to struct. That we go through
;; (ref eq) in the middle, which seems like it could cast to struct, should
;; not confuse us. And, as the cast fails, the reference does not escape.
;; We can optimize here and will emit an unreachable for the failing cast.
(ref.cast (ref struct)
(local.tee $eq
(array.new_fixed $array 0)
)
)
)

;; CHECK: (func $array.cast.struct.null (type $3) (result structref)
;; CHECK-NEXT: (local $eq (ref eq))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
(func $array.cast.struct.null (result (ref null struct))
(local $eq (ref eq))
;; As above but the cast is to a nullable type, which changes nothing.
(ref.cast (ref null struct)
(local.tee $eq
(array.new_fixed $array 0)
)
)
)

;; CHECK: (func $array.cast.array (type $4) (result (ref array))
;; CHECK-NEXT: (local $eq (ref eq))
;; CHECK-NEXT: (ref.cast (ref array)
;; CHECK-NEXT: (local.tee $eq
;; CHECK-NEXT: (array.new_fixed $array 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $array.cast.array (result (ref array))
(local $eq (ref eq))
;; Now we cast to array, and the cast succeeds, so we escape, and do
;; nothing to optimize.
(ref.cast (ref array)
(local.tee $eq
(array.new_fixed $array 0)
)
)
)

;; CHECK: (func $array.cast.array.set (type $1)
;; CHECK-NEXT: (local $eq (ref eq))
;; CHECK-NEXT: (local $array (ref array))
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $array.cast.array.set
(local $eq (ref eq))
(local $array (ref array))
;; As above, but now we store the result in a local rather than return it
;; out, so it does not escape.
(local.set $array
(ref.cast (ref array)
(local.tee $eq
(array.new_fixed $array 0)
)
)
)
)

;; CHECK: (func $array.cast.struct.set (type $1)
;; CHECK-NEXT: (local $eq (ref eq))
;; CHECK-NEXT: (local $struct (ref struct))
;; CHECK-NEXT: (local.tee $struct
;; CHECK-NEXT: (block
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (block (result nullref)
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (unreachable)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
(func $array.cast.struct.set
(local $eq (ref eq))
(local $struct (ref struct))
;; As above, but now the cast fails and is stored to a struct local. We do not
;; escape and we emit an unreachable for the cast.
(local.set $struct
(ref.cast (ref struct)
(local.tee $eq
(array.new_fixed $array 0)
)
)
)
)
)