Skip to content

Fix @effects of finalizeUninitializedArray when assertions are enabled #34101

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
Sep 28, 2020

Conversation

meg-gupta
Copy link
Contributor

When assertions are enabled _endCOWMutation writes to _native.isImmutable.
But finalizeUninitializedArray is marked as @effects(readnone). This
mismatch caused miscompile in the stdlib due to CodeSinking optimization
which relies on SILInstruction::mayReadOrWriteMemory which looks at
@effects.

@meg-gupta
Copy link
Contributor Author

Yet to create a repro

@meg-gupta meg-gupta requested a review from eeckstein September 28, 2020 18:21
When assertions are enabled _endCOWMutation writes to _native.isImmutable.
But finalizeUninitializedArray is marked as @effects(readnone). This
mismatch caused miscompile in the stdlib due to CodeSinking optimization
which relies on SILInstruction::mayReadOrWriteMemory which looks at
@effects.
@meg-gupta meg-gupta marked this pull request as ready for review September 28, 2020 19:25
@meg-gupta
Copy link
Contributor Author

@swift-ci test

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

@meg-gupta meg-gupta merged commit 410b88e into swiftlang:main Sep 28, 2020
ainu-bot added a commit to google/swift that referenced this pull request Sep 28, 2020
* 'master' of github.com:apple/swift:
  Fix @effects of finalizeUninitializedArray when assertions are enabled (swiftlang#34101)
@meg-gupta meg-gupta deleted the fixeffects branch July 9, 2021 00:26
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.

2 participants