0

I'm swapping bytes in a char buffer:

char* data; // some char buffer
uint16_t* data16 = reinterpret_cast<uint16_t*>(data);

// For debugging:
int align = reinterpret_cast<uintptr_t>(data) % alignof(uint16_t);
std::cout << "aligned? " << align << "\n";

for (size_t i = 0; i < length_of_data16; i++) {
#if defined(__GNUC__) || defined(__clang__)
  data16[i] = __builtin_bswap16(data16[i]);
#elif defined(_MSC_VER)
  data16[i] = _byteswap_ushort(data16[i]);
#endif
}

I'm casting from char* to uint16_t*, which raises a flag because it's casting to a more strictly aligned type.

However, the code runs correctly (on x86), even when the debugging code prints 1 (as in, not aligned). In the assembly I see MOVDQU, which I take to mean that the compiler recognizes that this might not be aligned.

This looks similar to this question, where the answer was "this is not safe." Does the above code only work on certain architectures and with certain compilers, or is there a subtle difference between these two questions that makes the above code valid?

(Less important: consistent with what I've read online, there's also no noticeable perf difference between aligned and unaligned execution of this code.)

Community
  • 1
  • 1
ZachB
  • 13,051
  • 4
  • 61
  • 89
  • 1
    Since you're calling implementation-defined functions, why do you care about portability? – Barmar Jun 08 '16 at 23:30
  • @Barmar Not sure I follow -- you mean that if these compile, then they will be safe? In the real code I do have a [fall-through function that uses bitshifting](https://github.com/zbjornson/node-bswap/blob/master/src/bswap.cc#L18-L32), not sure if that changes the discussion. – ZachB Jun 08 '16 at 23:33
  • The line `uint16_t* data16 = reinterpret_cast(data);` may be an alignment violation; you can't close the gate after the horse has bolted – M.M Jun 08 '16 at 23:47
  • The compiler uses MOVDQU because it doesn't know at compile-time whether or not the data will be aligned... you might have a better shot if you do the alignment check first, and use an `if` to only erform the cast and do the swap if the alignment check succeeded. – M.M Jun 08 '16 at 23:51
  • @M.M Thanks. I'm okay with it using MOVDQU because the perf difference isn't noticeable. Just trying to figure out if any of this is illegal, in which case I could add an `if` to use another swap method if it's not aligned. – ZachB Jun 08 '16 at 23:55
  • 1
    @ZachB Barmar's point is: why do you care if it is safe on all compilers, when you're not compiling this with all compilers? – user253751 Jun 09 '16 at 00:06
  • This is not safe, it's the same deal as the other question. What's supposed to happen if you perform pointer arithmetic between an aligned an unaligned pointer? – QuestionC Jun 09 '16 at 00:17
  • @QuestionC undefined behaviour (pointer subtraction is only defined when both pointers point to elements of the same array object, or the "past-the-end" element) – M.M Jun 09 '16 at 22:15

1 Answers1

1

If alignof(unit16_t) != 1 then this line may cause undefined behaviour due to alignment:

uint16_t* data16 = reinterpret_cast<uint16_t*>(data);

Putting an alignment check after this is no good; for a compiler could hardcode the check to say 1 because it knows that correct code couldn't reach that point otherwise.

In Standard C++ , for this check to be meaningful it must occur before the cast, and then the cast must not be performed if the check fails. (UB can time travel).

Of course, individual compilers may choose to define behaviour that is not defined by the Standard, e.g. perhaps g++ targeting x86 or x64 includes a definition that you're allowed to form unaligned pointers and dereference them.

There is no strict aliasing violation, as __builtin_bswap16 is not covered by the standard and we presume g++ implements it in such a way that is consistent with itself. MSVC doesn't do strict aliasing optimizations anyway.

M.M
  • 138,810
  • 21
  • 208
  • 365
  • Thanks. Is this right: The cast can cause undefined behavior, and even though the three compilers I've tested handle it correctly, I still shouldn't do it because it's brittle to rely on non-standard behaviors? – ZachB Jun 09 '16 at 00:28
  • @ZachB I'd agree with that – M.M Jun 09 '16 at 01:40