runtime: identify potential miscompiles #42130
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
`operator new` up until C++17 was alignment unaware. We use C++ types decorated with `alignas` to enforce 16-byte alignment. This is fine on the current platforms that we support as they are all enforcing 16-byte alignment on allocations. However, this is not a guarantee that C++ makes. It only provides the guarantee that `operator new` will align the memory to `__STDCPP_DEFAULT_NEW_ALIGNMENT__`. On 32-bit platforms such as Windows i686, this value is actually 8. However, due to the class(es) being attributed as `alignas(16)`, the default constructor which is emitted by the compiler assumes the proper alignment will be provided for externally and will zero the memory using the following sequence: ~~~ xorps xmm0, xmm0 movaps xmm0, [eax] movaps xmm0, [eax+16] ~~~ This assumes that the returned pointer is suitably aligned for XMM operations - 16-bytes - as the attribution indicates as such. This misalignment would cause a bus error on Linux, and more confusingly triggers an invalid access (the equivalent of a segmentation fault) on Windows. Add a static assertion to identify this unintended misalignment on allocation. This check will be meaningless post C++17 as that will use a two-phase overload resolution for `operator new`, preferring the newly introduced `operator new(std::size_t, std::align_val_t)` which would suitably align the type and as such is guarded by the feature macro `__cpp_aligned_new`.
|
@swift-ci please test |
mikeash
approved these changes
Apr 1, 2022
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.
operator newup until C++17 was alignment unaware. We use C++ typesdecorated with
alignasto enforce 16-byte alignment. This is fine onthe current platforms that we support as they are all enforcing 16-byte
alignment on allocations. However, this is not a guarantee that C++
makes. It only provides the guarantee that
operator newwill alignthe memory to
__STDCPP_DEFAULT_NEW_ALIGNMENT__. On 32-bit platformssuch as Windows i686, this value is actually 8. However, due to the
class(es) being attributed as
alignas(16), the default constructorwhich is emitted by the compiler assumes the proper alignment will be
provided for externally and will zero the memory using the following
sequence:
This assumes that the returned pointer is suitably aligned for XMM
operations - 16-bytes - as the attribution indicates as such. This
misalignment would cause a bus error on Linux, and more confusingly
triggers an invalid access (the equivalent of a segmentation fault)
on Windows.
Add a static assertion to identify this unintended misalignment on
allocation. This check will be meaningless post C++17 as that will use
a two-phase overload resolution for
operator new, preferring the newlyintroduced
operator new(std::size_t, std::align_val_t)which wouldsuitably align the type and as such is guarded by the feature macro
__cpp_aligned_new.Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.