-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[temp-rvalue-opt] Disable handling of load [take] in temp-rvalue-elim. #34327
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
[temp-rvalue-opt] Disable handling of load [take] in temp-rvalue-elim. #34327
Conversation
@swift-ci test |
Build failed |
Build failed |
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.
You have to update the temp_rvalue_opt_ossa.sil test
This pass was attempting to handle cases where we use load [take] + destroy_value instead of a destroy_addr since we have a loadable value. There are a few issues around how this was done, so for 5.3, we are just going to turn off the optimization. rdar://70356725
275dc42
to
d0b0600
Compare
@swift-ci test |
1 similar comment
@swift-ci test |
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.
LGTM!
@swift-ci nominate |
This pass was attempting to handle cases where we use load [take] +
destroy_value instead of a destroy_addr since we have a loadable value. There
are a few issues around how this was done, so for 5.3, we are just going to turn
off the optimization.
rdar://70356725
CCC
Explanation: This patch turns off an optimization in
TempRValueOpt that could cause miscompiles such as
leaks/use-after-frees. Note that we haven't seen this in practice
from actual Swift programs, we have only seen it in SIL code
snippets. So we are disabling this out of an abundance of
caution.
Scope: This miscompile could result in memory unsafety.
SR Issue: rdar://70356725
Risk: None. This is just turning off the optimization using a 2
line fix. We are going to do a fuller fix on main.
Testing: Added compiler tests that validate the optimization has
been disabled.
Reviewer: @eeckstein