Skip to content

5.7 fix cast ownership #42378

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 6 commits into from
Apr 19, 2022
Merged

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Apr 14, 2022

Fixes rdar://91768775 (crash when specializing a cast for AnyObject)

atrick added 6 commits April 14, 2022 15:35
Added an AST helper in Types.h:

- isPotentiallyAnyObject()

This formalizes logic for when cast operations forward
ownership. Various OSSA optimization rely on this for
correctness. This fixes latent bugs throughout the optimizer.

I was compelled to fix this now because we want to make OSSA
optimizations across dynamic casts more aggressive. For example, we
want to optimize retain/release across enum formation.

(cherry picked from commit 1a9c2ad)
Allow borrowed class-to-AnyObject scalar casts.

(cherry picked from commit 16e5fe9)
Use the new API that determines whether a cast preserves
ownership. Remove an old hack.

(cherry picked from commit a7c6a94)
@atrick atrick requested a review from eeckstein April 14, 2022 22:42
@atrick atrick requested a review from a team as a code owner April 14, 2022 22:42
@atrick
Copy link
Contributor Author

atrick commented Apr 14, 2022

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Apr 15, 2022

Linux error:
FAIL: swift-package-tests :: test-swift-docc/test-swift-docc.txt

@atrick atrick added the r5.7 label Apr 15, 2022
@atrick
Copy link
Contributor Author

atrick commented Apr 15, 2022

@swift-ci test linux

@atrick
Copy link
Contributor Author

atrick commented Apr 15, 2022

@eeckstein @tbkka these changes, taken together, should prevent the generation of ownership-guaranteed casts from T : AnyObject to X where X is a class type.

This SIL becomes illegal, as it should be:

class X {}
sil [ossa] @test : $@convention(thin) <T where T : AnyObject> (@guaranteed T) -> Builtin.Int1 {
bb0(%0 : @guaranteed $T):
  checked_cast_br %0 : $T to X, bb1, bb2

I'm cherry-picking this back to 5.7 because @eeckstein found this case occurring in a real project. Without these changes, the compiler ignores the SIL above, potentially miscompiling, until the function is specialized. Only when T is specialized as AnyObject will the compiler assert on the SIL above.

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

lgtm

@atrick
Copy link
Contributor Author

atrick commented Apr 15, 2022

@tbkka hello branch manager

@atrick
Copy link
Contributor Author

atrick commented Apr 18, 2022

@tbkka waiting for branch manager review

@atrick atrick merged commit 2e81826 into swiftlang:release/5.7 Apr 19, 2022
@atrick atrick deleted the 5.7-fix-cast-ownership branch April 19, 2022 00:26
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.7 labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants