martinmoene / variant-lite

variant lite - A C++17-like variant, a type-safe union for C++98, C++11 and later in a single-file header-only library
Boost Software License 1.0
239 stars 26 forks source link

`std::hash<nonstd::variant>` returns random value depends on uninitialized memory area. #32

Closed tekezo closed 5 years ago

tekezo commented 5 years ago

There is a problem that std::hash<nonstd::variant> does not return the same hash value for the same variant value.

Version

Both variant-lite@1051f91afd28fdfc05b2d2b8d1a0359090aabe3e and v1.1.0.

Example code

#define variant_CONFIG_SELECT_VARIANT variant_VARIANT_NONSTD

#include <cstdint>
#include <iostream>
#include <nonstd/variant.hpp>

int main(void) {
  using variant_t = nonstd::variant<uint32_t, uint64_t>;

  variant_t v1(static_cast<uint32_t>(42));

  variant_t v2(9223372036854775808ull);
  v2 = static_cast<uint32_t>(42);

  // std::hash<variant_t>{}(v1) should be equal std::hash<variant_t>{}(v2) since v1 == v2

  std::cout << "v1: " << std::hash<variant_t>{}(v1) << std::endl;
  std::cout << "v2: " << std::hash<variant_t>{}(v2) << std::endl;

  return 0;
}

Actual result

(v1 != v2)

v1: 2269231707
v2: 2820897755

Expected result

(v1 == v2)

v1: 2269231707
v2: 2269231707

Note

I guess the hash is accessing uninitialized memory here.

https://github.com/martinmoene/variant-lite/blob/master/include/nonstd/variant.hpp#L832

tekezo commented 5 years ago

I confirmed the latest revision fixed this issue. Thank you!

martinmoene commented 5 years ago

@tekezo Thanks for letting me know.

martinmoene commented 5 years ago

As @siffiejoe notes, std::hash<nonstd::variant> may well include padding bits in a variant alternative structure for the computation of the hash value. This can have a randomizing effect on the hash value.

The solution seems to be to require std::hash<userdefined-struct> to be available and use that.

martinmoene commented 5 years ago

Thanks @tekezo & s @siffiejoe.