Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

reinterpret_cast bug or UB? [duplicate]

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?

like image 533
bartop Avatar asked Sep 17 '18 18:09

bartop


People also ask

Should you use Reinterpret_cast?

Purpose for using reinterpret_cast It is used when we want to work with bits. If we use this type of cast then it becomes a non-portable product. So, it is suggested not to use this concept unless required. It is only used to typecast any pointer to its original type.

What does Reinterpret_cast mean in C++?

The reinterpret_cast allows the pointer to be treated as an integral type. The result is then bit-shifted and XORed with itself to produce a unique index (unique to a high degree of probability). The index is then truncated by a standard C-style cast to the return type of the function.

Can Reinterpret_cast throw?

No. It is a purely compile-time construct. It is very dangerous, because it lets you get away with very wrong conversions.


2 Answers

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.

like image 139
Barry Avatar answered Oct 06 '22 14:10

Barry


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

like image 13
Justin Avatar answered Oct 06 '22 16:10

Justin