-
Notifications
You must be signed in to change notification settings - Fork 790
Avoid invalid exact casts in RemoveUnusedBrs #7522
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.
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
Conversation
We previously allowed trivial exact casts (i.e. those where the input and target types are the same) to validate even without custom descriptors enabled to ensure that finalization always produces valid casts. But validation can also produce casts where the cast target is a non-nullable exact reference and the input type is a nullable exact reference to the same heap type. Update validation to allow these casts. This is safe because erasing the exactness of the input and cast types does not change the results of these casts. They succeed iff the input is a non-null value.
Do not merge types that differ in exactness.
RemoveUnusedBrs can introduce new casts when optimizing out br_on_cast and br_on_cast_fail instructions. This happens when the pass finds a more precise type for the fallthrough value reaching the casting branch, allowing it to improve the cast target type. When the branch is subsequently optimized out, this precise type information is recovered with an inserted cast. Update the pass to avoid creating invalid casts to exact types when custom descriptors is not enabled.
!curr->castType.isExact()) { | ||
// When custom descriptors is not enabled, nontrivial exact casts are | ||
// not allowed. | ||
glb = glb.with(Inexact); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could have a utility getMaybeWithoutExact(type, module)
which would check if the module's features support exactness and returns the type without, if not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Can I add that in a follow-up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
// When custom descriptors is not enabled, nontrivial exact casts are | ||
// not allowed. | ||
glb = glb.with(Inexact); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, is the check here enough? castType might have been exact, which is valid in the rare cases we do allow exact casts (trivial or nullability) , but the glb might not be similarly allowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GLB of an exact type and any other type in the same hierarchy is either the same exact type (if the other type is a supertype of it), or the bottom type otherwise. If the original exact type was valid, then both of these possible results are also valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right, because nothing (edit: nontrivial) is below exact types, thanks.
RemoveUnusedBrs can introduce new casts when optimizing out br_on_cast
and br_on_cast_fail instructions. This happens when the pass finds a
more precise type for the fallthrough value reaching the casting branch,
allowing it to improve the cast target type. When the branch is
subsequently optimized out, this precise type information is recovered
with an inserted cast.
Update the pass to avoid creating invalid casts to exact types when
custom descriptors is not enabled.