Concurrency: fix UB in DefaultActor initialization #40260
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CC: @mikeash |
|
@swift-ci please smoke test |
mikeash
approved these changes
Nov 19, 2021
|
@swift-ci please test Windows platform |
This fixes a latent UB instance in the `DefaultActor` implementation that has haunted the Windows target. The shared constructor for the type caused an errant typo that happened to compile which introduced UB but happened to work for the non-Windows cases. This happened to work for the other targets as `swift::atomic` had a `std::atomic` at on most configurations, and the C delegate for the Actor initializer happened to overlap and initialize the memory properly. The Windows case used an inline pointer width value but would be attempted to be initialized as a `std::atomic`. Relying on the overlap is unsafe to assume, and we should use the type's own constructor which delegates appropriately.
compnerd
force-pushed
the
ub-all-the-things
branch
from
0c7cf8a to
306662f
Compare
|
@swift-ci please test Windows platform |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a latent UB instance in the
DefaultActorimplementationthat has haunted the Windows target. The shared constructor for the
type caused an errant typo that happened to compile which introduced
UB but happened to work for the non-Windows cases. This happened to
work for the other targets as
swift::atomichad astd::atomicaton most configurations, and the C delegate for the Actor initializer
happened to overlap and initialize the memory properly. The Windows
case used an inline pointer width value but would be attempted to be
initialized as a
std::atomic. Relying on the overlap is unsafe toassume, and we should use the type's own constructor which delegates
appropriately.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.