Skip to content

SILOptimizer: fix a crash in ArrayElementValuePropagation. #22993

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

Merged
merged 1 commit into from
Mar 1, 2019

Conversation

eeckstein
Copy link
Contributor

This optimization is doing 2 things: replacing getElement calls and replacing append(contentOf:) calls.
Now if the argument to a append(contentOf:) is a previously replaced getElement call, we ended up in a use-after-free crash.

The main change here is to do the transformations immediately after gathering the data (and that means: separately) and not collecting all the data and do both transformation afterwards
The pass does not use any Analysis (where invalidation could be a problem). Also iterator invalidation is not a problem here.

SR-10003
rdar://problem/48445856

@eeckstein
Copy link
Contributor Author

@swift-ci test

Copy link
Contributor

@aschwaighofer aschwaighofer left a comment

Choose a reason for hiding this comment

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

LGTM

/// \endcode
/// The source_array and its initialization code can then be deleted (if not
/// used otherwise).
bool ArrayAllocation::replacemeAppendContentOf() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling: replaceme

This optimization is doing 2 things: replacing getElement calls and replacing append(contentOf:) calls.
Now if the argument to a append(contentOf:) is a previously replaced getElement call, we ended up in a use-after-free crash.

The main change here is to do the transformations immediately after gathering the data (and that means: separately) and not collecting all the data and do both transformation afterwards
The pass does not use any Analysis (where invalidation could be a problem). Also iterator invalidation is not a problem here.

SR-10003
rdar://problem/48445856
@eeckstein eeckstein force-pushed the fix-array-element-propagation branch from 9144dc1 to 4acffab Compare March 1, 2019 00:15
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test and merge

1 similar comment
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test and merge

@swift-ci swift-ci merged commit 22bcbe5 into swiftlang:master Mar 1, 2019
@eeckstein eeckstein deleted the fix-array-element-propagation branch March 1, 2019 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants