-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Closed
Labels
bugSomething isn't workingSomething isn't workingfixedSomething works now, yay!Something works now, yay!
Description
We have some intentional unaligned access in vector_algorithms.cpp. Some occurrences are marked:
STL/stl/src/vector_algorithms.cpp
Lines 114 to 141 in 48db51b
| #if defined(_M_X64) // NOTE: UNALIGNED MEMORY ACCESSES | |
| constexpr size_t _Mask_8 = ~((static_cast<size_t>(1) << 3) - 1); | |
| if (_Byte_length(_First1, _Last1) >= 8) { | |
| const void* _Stop_at = _First1; | |
| _Advance_bytes(_Stop_at, _Byte_length(_First1, _Last1) & _Mask_8); | |
| do { | |
| const unsigned long long _Left = *static_cast<unsigned long long*>(_First1); | |
| const unsigned long long _Right = *static_cast<unsigned long long*>(_First2); | |
| *static_cast<unsigned long long*>(_First1) = _Right; | |
| *static_cast<unsigned long long*>(_First2) = _Left; | |
| _Advance_bytes(_First1, 8); | |
| _Advance_bytes(_First2, 8); | |
| } while (_First1 != _Stop_at); | |
| } | |
| #elif defined(_M_IX86) // NOTE: UNALIGNED MEMORY ACCESSES | |
| constexpr size_t _Mask_4 = ~((static_cast<size_t>(1) << 2) - 1); | |
| if (_Byte_length(_First1, _Last1) >= 4) { | |
| const void* _Stop_at = _First1; | |
| _Advance_bytes(_Stop_at, _Byte_length(_First1, _Last1) & _Mask_4); | |
| do { | |
| const unsigned long _Left = *static_cast<unsigned long*>(_First1); | |
| const unsigned long _Right = *static_cast<unsigned long*>(_First2); | |
| *static_cast<unsigned long*>(_First1) = _Right; | |
| *static_cast<unsigned long*>(_First2) = _Left; | |
| _Advance_bytes(_First1, 4); | |
| _Advance_bytes(_First2, 4); | |
| } while (_First1 != _Stop_at); | |
| } |
There may be some unmarked occurrences as well.
I can think of a few fixing that
memcpy. The usual way, but it makes debug codegen slightly worse.#pragma intrinsiccan partly mitigate debug codegen worsening.__builtin_bit_cast. Perfect codegen, ugly construct:unsigned long long cast(void* p) { return __builtin_bit_cast(unsigned long long, *reinterpret_cast<unsigned char(*)[8]>(p)); }
There's a bug reported by @frederick-vs-ja as DevCom-10974125, now Fixed - Pending Release. Workaround is to save the reinterpret_cast result to a variable.
__builtin_bit_castwrappersstd::_Bit_castorstd::bit_cast. Same as the previous, plus function call in debug. Avoids the bug.
The memcpy approach has already been used in some more recent parts, so if we prefer other approach, we may want to revisit that as well:
STL/stl/src/vector_algorithms.cpp
Line 6927 in 48db51b
| memcpy(&_Val, _Src, sizeof(_Val)); |
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't workingfixedSomething works now, yay!Something works now, yay!