From ec4065af4d777b1d48a6c3bf4e0b8c39c818d0bb Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Wed, 15 May 2019 12:46:10 +0800 Subject: [PATCH] blobhash: do not use cast for unaligned access * remove the uncorrect comment. as std::hash<> does not apply to a customized type. see https://en.cppreference.com/w/cpp/utility/hash * do not use cast for accessing an uint32_t by dereferencing (void *) or (char *) pointer. because the alignment requirement of `uint32_t` is stricter than that of `void*` or `char *`. we need to do an explicit memcpy() for accessing the uint32_t pointed by the pointer. see also https://www.kernel.org/doc/Documentation/unaligned-memory-access.txt * instead of using a loop for mixing the last few bytes. use a switch- case. so GCC-9 won't complain with ``` ../src/include/blobhash.h:38:10: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations] 38 | sh += 8; | ~~~^~~~ ../src/include/blobhash.h:36:12: note: within this loop 36 | while (len) { | ^~~ ``` at seeing `sh += 8;` * instead of mixing the last bits repeatly by derefencing `p` using `uint32_t`, while move it forwards with step size of `1`, mix the bits byte-wise. * include . this header file is supposed to be self-contained. * use `std::uint32_t` instead of using `uint32_t`. we cannot assume somebody is `using namespace std` or `using std::uint32_t` for us. * mark the operator() `const noexcept`. see https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c89-make-a-hash-noexcept for the rationale behind this Signed-off-by: Kefu Chai --- src/include/blobhash.h | 43 ++++++++++++++++++++++++------------------ src/msg/msg_types.h | 2 +- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/include/blobhash.h b/src/include/blobhash.h index 597884e4c9d..97f2065ce3a 100644 --- a/src/include/blobhash.h +++ b/src/include/blobhash.h @@ -14,33 +14,40 @@ #ifndef CEPH_BLOBHASH_H #define CEPH_BLOBHASH_H +#include #include "hash.h" -/* -- this is to make some of the STL types work with 64 bit values, string hash keys, etc. -- added when i was using an old STL.. maybe try taking these out and see if things - compile now? -*/ - class blobhash { public: - uint32_t operator()(const char *p, unsigned len) { - static rjhash H; - uint32_t acc = 0; + uint32_t operator()(const void* p, size_t len) { + static rjhash H; + std::uint32_t acc = 0; + auto buf = static_cast(p); while (len >= sizeof(acc)) { - acc ^= *(uint32_t*)p; - p += sizeof(uint32_t); - len -= sizeof(uint32_t); + acc ^= unaligned_load(buf); + buf += sizeof(std::uint32_t); + len -= sizeof(std::uint32_t); } - int sh = 0; - while (len) { - acc ^= (uint32_t)*p << sh; - sh += 8; - len--; - p++; + // handle the last few bytes of p[-(len % 4):] + switch (len) { + case 3: + acc ^= buf[2] << 16; + [[fallthrough]]; + case 2: + acc ^= buf[1] << 8; + [[fallthrough]]; + case 1: + acc ^= buf[0]; + [[fallthrough]]; } return H(acc); } +private: + static inline std::uint32_t unaligned_load(const unsigned char* p) { + std::uint32_t result; + __builtin_memcpy(&result, p, sizeof(result)); + return result; + } }; diff --git a/src/msg/msg_types.h b/src/msg/msg_types.h index d9ca27c3aec..7557c51a82f 100644 --- a/src/msg/msg_types.h +++ b/src/msg/msg_types.h @@ -565,7 +565,7 @@ namespace std { template<> struct hash { size_t operator()( const entity_addr_t& x ) const { static blobhash H; - return H((const char*)&x, sizeof(x)); + return H(&x, sizeof(x)); } }; } // namespace std -- 2.39.5