reinterpret_cast bug or UB? [duplicate]

Question

This question already has answers here:

Observing weird behavior with 'auto' and std::minmax (1 answer)

structured bindings with std::minmax and rvalues (2 answers)

Closed 2 years ago.

Consider following code:

#include <cstdint>
#include <algorithm>

std::uintptr_t minPointer(void *first, void *second) {
    const auto pair = std::minmax(
        reinterpret_cast<std::uintptr_t>(first),
        reinterpret_cast<std::uintptr_t>(second)
    );
    return pair.first;
}

and the assembly generated by GCC8 with -O3 on https://godbolt.org/z/qWJuV_ for minPointer:

minPointer(void*, void*):
  mov rax, QWORD PTR [rsp-8]
  ret

which clearly does not do what is intended by the code creator. Is this code causing some UB or is it GCC(8) bug?

Answer 1

This is UB, but not for the reason you might think.

The relevant signature of std::minmax() is:

template< class T > 
std::pair<const T&,const T&> minmax( const T& a, const T& b );

In this case, your pair is a pair of references to uintptr_t const. Where are the actual objects we're referencing? That's right, they were temporaries created on the last line that have already gone out of scope! We have dangling references.

If you wrote:

return std::minmax(
    reinterpret_cast<std::uintptr_t>(first),
    reinterpret_cast<std::uintptr_t>(second)
).first;

then we don't have any dangling references and you can see that gcc generates appropriate code:

minPointer(void*, void*):
  cmp rsi, rdi
  mov rax, rdi
  cmovbe rax, rsi
  ret

Alternatively, you could explicitly specify the type of pair as std::pair<std::uintptr_t, std::uintptr_t>. Or just sidestep the pair entirely and return std::min(...);.

---

As far as language specifics, you are allowed to convert a pointer to a large enough integral type due to [expr.reinterpret.cast]/4, and std::uintptr_t is guaranteed to be large enough. So once you fix the dangling reference issue, you're fine.

Answer 2

The reinterpret_cast is well defined. The problem is that the type of const auto pair is const std::pair<const std::uintptr_t&, const std::uintptr_t&> as that's what std::minmax returns, so you have dangling references.

You just need to get rid of the dangling references for it to work:

std::uintptr_t minPointer(void *first, void *second) {
    const std::pair<std::uintptr_t, std::uintptr_t> pair = std::minmax(
        reinterpret_cast<std::uintptr_t>(first),
        reinterpret_cast<std::uintptr_t>(second)
    );
    return pair.first;
}

Godbolt link