Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions src/passes/AbstractTypeRefining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,13 @@ struct AbstractTypeRefining : public Pass {
// This might cause the casts to incorrectly start succeeding. To avoid
// that, optimize them out first.
void visitRefCast(RefCast* curr) {
if (curr->ref->type == Type::unreachable ||
(curr->desc && curr->desc->type == Type::unreachable)) {
// A child may have become unreachable earlier in Preoptimizer, before
// ReFinalize has run. Propagate unreachability and skip optimizing.
curr->type = Type::unreachable;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this needed? Did we modify the code at this point but not call refinalize yet? If so it seems worth a comment as in general this should not be necessary.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added comments.

return;
}
auto optimized = getOptimized(curr->type);
if (!optimized) {
return;
Expand Down Expand Up @@ -402,6 +409,11 @@ struct AbstractTypeRefining : public Pass {
// If we optimize away a descriptor type, then we must fix up any
// ref.get_desc of it, as ReFinalize would fix us up to return it.
void visitRefGetDesc(RefGetDesc* curr) {
if (curr->ref->type == Type::unreachable) {
// As in visitRefCast, propagate unreachability and skip optimizing.
curr->type = Type::unreachable;
return;
}
auto optimized = getOptimized(curr->type);
if (!optimized) {
return;
Expand Down Expand Up @@ -430,6 +442,12 @@ struct AbstractTypeRefining : public Pass {
}

void visitBrOn(BrOn* curr) {
if (curr->ref->type == Type::unreachable ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[HIGH | Confidence: 5/5 | BUG]

The unreachable check in visitBrOn is placed after the early return for BrOnNull and BrOnNonNull:

Suggested change
if (curr->ref->type == Type::unreachable ||
if (curr->op == BrOnNull || curr->op == BrOnNonNull) {
return;
}
if (curr->ref->type == Type::unreachable ||
(curr->desc && curr->desc->type == Type::unreachable)) {
curr->type = Type::unreachable;
return;
}

If the ref operand of a BrOnNull or BrOnNonNull became unreachable during the post-order traversal (e.g., from an uninstantiated RefGetDesc replaced with an unreachable sequence), line 442 returns early without updating curr->type to Type::unreachable.

If this BrOn expression is subsequently visited as the ref child of an enclosing RefCast, RefGetDesc, or another BrOn, the enclosing expression will observe curr->ref->type as a reachable concrete type instead of unreachable. This bypasses the enclosing expression's unreachable guard and causes it to perform invalid transformations (e.g., localizeChildren extracting the branch to a local, or makeRefCast), leading to invalid IR or assertion failures during finalization.

To ensure consistent type propagation matching BrOn::finalize(), move the unreachable check before the operation check:

Suggested change
if (curr->ref->type == Type::unreachable ||
void visitBrOn(BrOn* curr) {
if (curr->ref->type == Type::unreachable ||
(curr->desc && curr->desc->type == Type::unreachable)) {
curr->type = Type::unreachable;
return;
}
if (curr->op == BrOnNull || curr->op == BrOnNonNull) {
return;
}
auto optimized = getOptimized(curr->castType);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Didn't quite get the comment here, but it at least makes sense to me to move the check up like it suggests? Although maybe refinalize would do this anyway

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, seems reasonable.

(curr->desc && curr->desc->type == Type::unreachable)) {
// As in visitRefCast, propagate unreachability and skip optimizing.
curr->type = Type::unreachable;
return;
}
if (curr->op == BrOnNull || curr->op == BrOnNonNull) {
return;
}
Expand Down
184 changes: 184 additions & 0 deletions test/lit/passes/abstract-type-refining-desc.wast
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,107 @@
)
(unreachable)
)

;; YESTNH: (func $branch-fail-unreachable (type $2) (param $ref anyref) (param $desc (ref null $desc)) (result nullref)
;; YESTNH-NEXT: (block $block
;; YESTNH-NEXT: (ref.cast_desc_eq (ref $struct)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (block
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.null none)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: (unreachable)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; YESTNH-NEXT: (local.get $desc)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; NO_TNH: (func $branch-fail-unreachable (type $2) (param $ref anyref) (param $desc (ref null $desc)) (result nullref)
;; NO_TNH-NEXT: (block $block
;; NO_TNH-NEXT: (ref.cast_desc_eq (ref $struct)
;; NO_TNH-NEXT: (drop
;; NO_TNH-NEXT: (block
;; NO_TNH-NEXT: (drop
;; NO_TNH-NEXT: (ref.null none)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: (unreachable)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: (local.get $desc)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
(func $branch-fail-unreachable (param $ref anyref) (param $desc (ref null $desc)) (result (ref null $struct))
(block $block (result (ref null $struct))
;; This cast is unreachable because its ref input is an unreachable drop,
;; so it is skipped. During ReFinalize it requires its ref input to remain
;; unreachable.
(ref.cast_desc_eq (ref $struct)
;; This drop is only valid as the ref input to ref.cast_desc_eq because
;; its child is unreachable.
(drop
;; The descriptor operand is unreachable, so this branch is skipped
;; and replaced by an unreachable block in ReFinalize.
(br_on_cast_desc_eq_fail $block (ref null $struct) (ref $struct)
(ref.null none)
(unreachable)
)
)
(local.get $desc)
)
)
)

;; YESTNH: (func $cast-unreachable-desc (type $2) (param $ref anyref) (param $desc (ref null $desc)) (result nullref)
;; YESTNH-NEXT: (block $block
;; YESTNH-NEXT: (block ;; (replaces unreachable RefCast we can't emit)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (local.get $ref)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (block
;; YESTNH-NEXT: (unreachable)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (local.get $desc)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; YESTNH-NEXT: (unreachable)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; NO_TNH: (func $cast-unreachable-desc (type $2) (param $ref anyref) (param $desc (ref null $desc)) (result nullref)
;; NO_TNH-NEXT: (block $block
;; NO_TNH-NEXT: (block ;; (replaces unreachable RefCast we can't emit)
;; NO_TNH-NEXT: (drop
;; NO_TNH-NEXT: (local.get $ref)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: (drop
;; NO_TNH-NEXT: (block
;; NO_TNH-NEXT: (unreachable)
;; NO_TNH-NEXT: (drop
;; NO_TNH-NEXT: (local.get $desc)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: (unreachable)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
(func $cast-unreachable-desc (param $ref anyref) (param $desc (ref null $desc)) (result (ref null $struct))
(block $block (result (ref null $struct))
;; Same as above, but with an unreachable ref operand in the branch and an
;; unreachable descriptor operand in the cast.
(ref.cast_desc_eq (ref $struct)
(local.get $ref)
(br_on_cast_desc_eq_fail $block (ref null $struct) (ref $struct)
(unreachable)
(local.get $desc)
)
)
)
)
)

(module
Expand Down Expand Up @@ -1115,6 +1216,8 @@

;; YESTNH: (type $6 (func (param (ref $A)) (result (ref $B.sub))))

;; YESTNH: (type $7 (func (param (ref (exact $B.sub)) (ref $B.sub)) (result (ref $B.sub))))

;; YESTNH: (func $create (type $4)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (struct.new_default_desc $A.sub
Expand All @@ -1128,6 +1231,8 @@

;; NO_TNH: (type $6 (func (param (ref $A)) (result (ref $B))))

;; NO_TNH: (type $7 (func (param (ref (exact $B)) (ref $B)) (result (ref $B))))

;; NO_TNH: (func $create (type $4)
;; NO_TNH-NEXT: (drop
;; NO_TNH-NEXT: (struct.new_default_desc $A.sub
Expand Down Expand Up @@ -1187,6 +1292,41 @@
(local.get $inexact)
)
)

;; YESTNH: (func $inexact-unreachable (type $7) (param $exact (ref (exact $B.sub))) (param $inexact (ref $B.sub)) (result (ref $B.sub))
;; YESTNH-NEXT: (block ;; (replaces unreachable RefGetDesc we can't emit)
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.cast_desc_eq (ref $A.sub)
;; YESTNH-NEXT: (unreachable)
;; YESTNH-NEXT: (local.get $inexact)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; YESTNH-NEXT: (unreachable)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; NO_TNH: (func $inexact-unreachable (type $7) (param $exact (ref (exact $B))) (param $inexact (ref $B)) (result (ref $B))
;; NO_TNH-NEXT: (ref.get_desc $A
;; NO_TNH-NEXT: (ref.cast_desc_eq (ref $A)
;; NO_TNH-NEXT: (struct.new_default_desc $A
;; NO_TNH-NEXT: (local.get $exact)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: (local.get $inexact)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
(func $inexact-unreachable (param $exact (ref (exact $B))) (param $inexact (ref $B)) (result (ref $B))
;; The struct.new_default_desc is replaced with unreachable, which makes the
;; inexact ref.cast_desc_eq unreachable, which in turn makes the
;; ref.get_desc unreachable.
(ref.get_desc $A
(ref.cast_desc_eq (ref $A)
(struct.new_default_desc $A
(local.get $exact)
)
(local.get $inexact)
)
)
)
)

;; Shared abstract described types.
Expand Down Expand Up @@ -1272,6 +1412,8 @@

;; YESTNH: (type $8 (func (param (ref null $desc2))))

;; YESTNH: (type $9 (func (param (ref $struct) (ref null $desc2))))

;; YESTNH: (global $desc2 (ref $desc2) (struct.new_default $desc2))
;; NO_TNH: (type $4 (func (param (ref none))))

Expand All @@ -1283,6 +1425,8 @@

;; NO_TNH: (type $8 (func (param (ref null $desc2))))

;; NO_TNH: (type $9 (func (param (ref $struct) (ref null $desc2))))

;; NO_TNH: (global $desc2 (ref $desc2) (struct.new_default $desc2))
(global $desc2 (ref $desc2) (struct.new_default $desc2))

Expand Down Expand Up @@ -1452,6 +1596,46 @@
)
)

;; YESTNH: (func $ref-cast-desc-unreachable (type $9) (param $s (ref $struct)) (param $desc2 (ref null $desc2))
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (ref.cast_desc_eq (ref $struct2)
;; YESTNH-NEXT: (block
;; YESTNH-NEXT: (drop
;; YESTNH-NEXT: (local.get $s)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: (unreachable)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: (local.get $desc2)
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; YESTNH-NEXT: )
;; NO_TNH: (func $ref-cast-desc-unreachable (type $9) (param $s (ref $struct)) (param $desc2 (ref null $desc2))
;; NO_TNH-NEXT: (drop
;; NO_TNH-NEXT: (ref.cast_desc_eq (ref $struct2)
;; NO_TNH-NEXT: (block
;; NO_TNH-NEXT: (drop
;; NO_TNH-NEXT: (local.get $s)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: (unreachable)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: (local.get $desc2)
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
;; NO_TNH-NEXT: )
(func $ref-cast-desc-unreachable (param $s (ref $struct)) (param $desc2 (ref null $desc2))
;; The inner ref.get_desc is replaced with an unreachable block before the
;; enclosing ref.cast_desc_eq is visited, so the enclosing cast becomes
;; unreachable and is skipped.
(drop
(ref.cast_desc_eq (ref $struct2)
(ref.get_desc $struct
(local.get $s)
)
(local.get $desc2)
)
)
)

;; YESTNH: (func $br-on-cast-desc-pop (type $8) (param $desc (ref null $desc2))
;; YESTNH-NEXT: (local $1 anyref)
;; YESTNH-NEXT: (local $2 (ref null $desc2))
Expand Down
Loading