Skip to content

[5.2]Fix propagation of guaranteed phi args during DiagnoseUnreachable. #29537

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

Jump to bottom
Merged
merged 1 commit into from
Jan 30, 2020
Merged

[5.2]Fix propagation of guaranteed phi args during DiagnoseUnreachable. #29537

merged 1 commit into from
Jan 30, 2020

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Jan 29, 2020

Fixes rdar://58929109 assertion failure - UNREACHABLE executed at
swift/include/swift/SIL/OwnershipUtils.h:127!

(cherry picked from commit b0786e7)

@atrick
Fix propagation of guaranteed phi args during DiagnoseUnreachable.
d977b4e 
Fixes <rdar://58929109> assertion failure - UNREACHABLE executed at
swift/include/swift/SIL/OwnershipUtils.h:127!

(cherry picked from commit b0786e7)
@atrick atrick requested a review from a team as a code owner January 29, 2020 23:11
@atrick atrick requested a review from gottesmm January 29, 2020 23:11
@atrick
Copy link
Contributor Author

atrick commented Jan 29, 2020

@swift-ci smoke test

@atrick
Copy link
Contributor Author

atrick commented Jan 29, 2020

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Jan 29, 2020

-- CCC ---

Explanation: Migration of the DiagnoseUnreachable pass to OSSA was incomplete. SIL speak: This caused an assertion in the case if nested "borrow scopes" rooted in a "@guaranteed phi". Support for phis with @guaranteed ownership was already added, but it was only narrowly applied to SILFunctionArguments. The fix is simply to remove that condition because the underlying logic is valid for all block arguments.

Scope: This is caught immediately by the SIL OSSA verifier, which may throw an assertion in rare cases.

Risk: Zero

Radar: rdar://58929109

Testing: The PR adds a reduced unit test. Besides that, SIL verifier will catch problems.

Reviewer: Michael Gottesman

@gottesmm
Copy link
Contributor

LGTM

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d977b4e

@atrick
Copy link
Contributor Author

atrick commented Jan 30, 2020

@swift-ci test linux

@tkremenek tkremenek merged commit ad5bae4 into swiftlang:swift-5.2-branch Jan 30, 2020
@atrick atrick deleted the 5.2-fix-guaranteed-phi branch February 26, 2020 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkremenek tkremenek tkremenek approved these changes

@gottesmm gottesmm Awaiting requested review from gottesmm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@atrick @gottesmm @swift-ci @tkremenek