Search code examples
c++language-lawyerundefined-behaviorreinterpret-cast

reinterpret_cast bug or UB?


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?


Solution

  • 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.