Skip to content

vector_algorithms.cpp: undefined behavior #5741

@AlexGuteniev

Description

@AlexGuteniev

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 @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));

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