Skip to content

vector_algorithms.cpp: undefined behavior #5741

Description

We have some intentional unaligned access in vector_algorithms.cpp. Some occurrences are marked:

#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

  1. memcpy. The usual way, but it makes debug codegen slightly worse. #pragma intrinsic can partly mitigate debug codegen worsening.
  2. __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 A. Jiang (@frederick-vs-ja) as DevCom-10974125, now Fixed - Pending Release. Workaround is to save the reinterpret_cast result to a variable.

  1. __builtin_bit_cast wrappers std::_Bit_cast or std::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:

memcpy(&_Val, _Src, sizeof(_Val));

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingfixedSomething works now, yay!

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions