[PATCH] [libc++] Remove unaligned type punning from murmurhash/cityhash
Matthew Dempsky
matthew at dempsky.org
Tue Jul 2 14:52:31 PDT 2013
I believe the code below is a conforming C++11 program. However, I'm
pretty sure with libc++, this code causes an unaligned word access
(problem for strict alignment architectures) and also involves type
punning (problem for -fstrict-aliasing).
This is because libc++'s hash<basic_string<...>> directly calls the
murmurhash/cityhash functions on its string data, which in turn
dereference the memory via size_t pointers. The allocator below
ensures the array is oddly aligned; but also because the string data
is a char array, it's unsafe to dereference it as a size_t or uint32_t
array.
Attached patch replaces the type punning with memcpy(), which on
x86/x86-64 clang optimizes to direct word accesses anyway.
#include <string>
#include <iostream>
#include <cstdlib>
namespace {
template <typename T>
class SillyAlloc
{
public:
typedef T value_type;
SillyAlloc() = default;
template <typename U> SillyAlloc(const SillyAlloc<U>&) noexcept {}
T* allocate(std::size_t n)
{
T* p = static_cast<T*>(std::malloc((n + 1) * sizeof(T)));
return p ? p + 1 : nullptr;
}
void deallocate(T* p, std::size_t) noexcept
{
if (p) std::free(p - 1);
}
};
typedef std::basic_string<char, std::char_traits<char>, SillyAlloc<char>> SillyString;
}
int
main()
{
SillyString s = "hello";
std::hash<SillyString> h;
std::cout << h(s) << std::endl;
}
-------------- next part --------------
Index: libcxx/include/memory
===================================================================
--- libcxx/include/memory (revision 185441)
+++ libcxx/include/memory (working copy)
@@ -3126,6 +3126,17 @@
template <class _Tp> struct hash;
+namespace {
+template <class _Size>
+inline _Size
+__loadword(const void* __p)
+{
+ _Size __r;
+ std::memcpy(&__r, __p, sizeof(__r));
+ return __r;
+}
+}
+
// We use murmur2 when size_t is 32 bits, and cityhash64 when size_t
// is 64 bits. This is because cityhash64 uses 64bit x 64bit
// multiplication, which can be very slow on 32-bit systems.
@@ -3149,7 +3160,7 @@
const unsigned char* __data = static_cast<const unsigned char*>(__key);
for (; __len >= 4; __data += 4, __len -= 4)
{
- _Size __k = *(const _Size*)__data;
+ _Size __k = __loadword<_Size>(__data);
__k *= __m;
__k ^= __k >> __r;
__k *= __m;
@@ -3208,13 +3219,13 @@
static _Size __hash_len_0_to_16(const char* __s, _Size __len) {
if (__len > 8) {
- const _Size __a = *(const _Size*)__s;
- const _Size __b = *(const _Size*)(__s + __len - 8);
+ const _Size __a = __loadword<_Size>(__s);
+ const _Size __b = __loadword<_Size>(__s + __len - 8);
return __hash_len_16(__a, __rotate_by_at_least_1(__b + __len, __len)) ^ __b;
}
if (__len >= 4) {
- const uint32_t __a = *(const uint32_t*)(__s);
- const uint32_t __b = *(const uint32_t*)(__s + __len - 4);
+ const uint32_t __a = __loadword<uint32_t>(__s);
+ const uint32_t __b = __loadword<uint32_t>(__s + __len - 4);
return __hash_len_16(__len + (__a << 3), __b);
}
if (__len > 0) {
@@ -3230,10 +3241,10 @@
}
static _Size __hash_len_17_to_32(const char *__s, _Size __len) {
- const _Size __a = *(const _Size*)(__s) * __k1;
- const _Size __b = *(const _Size*)(__s + 8);
- const _Size __c = *(const _Size*)(__s + __len - 8) * __k2;
- const _Size __d = *(const _Size*)(__s + __len - 16) * __k0;
+ const _Size __a = __loadword<_Size>(__s) * __k1;
+ const _Size __b = __loadword<_Size>(__s + 8);
+ const _Size __c = __loadword<_Size>(__s + __len - 8) * __k2;
+ const _Size __d = __loadword<_Size>(__s + __len - 16) * __k0;
return __hash_len_16(__rotate(__a - __b, 43) + __rotate(__c, 30) + __d,
__a + __rotate(__b ^ __k3, 20) - __c + __len);
}
@@ -3254,33 +3265,33 @@
// Return a 16-byte hash for s[0] ... s[31], a, and b. Quick and dirty.
static pair<_Size, _Size> __weak_hash_len_32_with_seeds(
const char* __s, _Size __a, _Size __b) {
- return __weak_hash_len_32_with_seeds(*(const _Size*)(__s),
- *(const _Size*)(__s + 8),
- *(const _Size*)(__s + 16),
- *(const _Size*)(__s + 24),
+ return __weak_hash_len_32_with_seeds(__loadword<_Size>(__s),
+ __loadword<_Size>(__s + 8),
+ __loadword<_Size>(__s + 16),
+ __loadword<_Size>(__s + 24),
__a,
__b);
}
// Return an 8-byte hash for 33 to 64 bytes.
static _Size __hash_len_33_to_64(const char *__s, size_t __len) {
- _Size __z = *(const _Size*)(__s + 24);
- _Size __a = *(const _Size*)(__s) +
- (__len + *(const _Size*)(__s + __len - 16)) * __k0;
+ _Size __z = __loadword<_Size>(__s + 24);
+ _Size __a = __loadword<_Size>(__s) +
+ (__len + __loadword<_Size>(__s + __len - 16)) * __k0;
_Size __b = __rotate(__a + __z, 52);
_Size __c = __rotate(__a, 37);
- __a += *(const _Size*)(__s + 8);
+ __a += __loadword<_Size>(__s + 8);
__c += __rotate(__a, 7);
- __a += *(const _Size*)(__s + 16);
+ __a += __loadword<_Size>(__s + 16);
_Size __vf = __a + __z;
_Size __vs = __b + __rotate(__a, 31) + __c;
- __a = *(const _Size*)(__s + 16) + *(const _Size*)(__s + __len - 32);
- __z += *(const _Size*)(__s + __len - 8);
+ __a = __loadword<_Size>(__s + 16) + __loadword<_Size>(__s + __len - 32);
+ __z += __loadword<_Size>(__s + __len - 8);
__b = __rotate(__a + __z, 52);
__c = __rotate(__a, 37);
- __a += *(const _Size*)(__s + __len - 24);
+ __a += __loadword<_Size>(__s + __len - 24);
__c += __rotate(__a, 7);
- __a += *(const _Size*)(__s + __len - 16);
+ __a += __loadword<_Size>(__s + __len - 16);
_Size __wf = __a + __z;
_Size __ws = __b + __rotate(__a, 31) + __c;
_Size __r = __shift_mix((__vf + __ws) * __k2 + (__wf + __vs) * __k0);
@@ -3306,26 +3317,26 @@
// For strings over 64 bytes we hash the end first, and then as we
// loop we keep 56 bytes of state: v, w, x, y, and z.
- _Size __x = *(const _Size*)(__s + __len - 40);
- _Size __y = *(const _Size*)(__s + __len - 16) +
- *(const _Size*)(__s + __len - 56);
- _Size __z = __hash_len_16(*(const _Size*)(__s + __len - 48) + __len,
- *(const _Size*)(__s + __len - 24));
+ _Size __x = __loadword<_Size>(__s + __len - 40);
+ _Size __y = __loadword<_Size>(__s + __len - 16) +
+ __loadword<_Size>(__s + __len - 56);
+ _Size __z = __hash_len_16(__loadword<_Size>(__s + __len - 48) + __len,
+ __loadword<_Size>(__s + __len - 24));
pair<_Size, _Size> __v = __weak_hash_len_32_with_seeds(__s + __len - 64, __len, __z);
pair<_Size, _Size> __w = __weak_hash_len_32_with_seeds(__s + __len - 32, __y + __k1, __x);
- __x = __x * __k1 + *(const _Size*)(__s);
+ __x = __x * __k1 + __loadword<_Size>(__s);
// Decrease len to the nearest multiple of 64, and operate on 64-byte chunks.
__len = (__len - 1) & ~static_cast<_Size>(63);
do {
- __x = __rotate(__x + __y + __v.first + *(const _Size*)(__s + 8), 37) * __k1;
- __y = __rotate(__y + __v.second + *(const _Size*)(__s + 48), 42) * __k1;
+ __x = __rotate(__x + __y + __v.first + __loadword<_Size>(__s + 8), 37) * __k1;
+ __y = __rotate(__y + __v.second + __loadword<_Size>(__s + 48), 42) * __k1;
__x ^= __w.second;
- __y += __v.first + *(const _Size*)(__s + 40);
+ __y += __v.first + __loadword<_Size>(__s + 40);
__z = __rotate(__z + __w.first, 33) * __k1;
__v = __weak_hash_len_32_with_seeds(__s, __v.second * __k1, __x + __w.first);
__w = __weak_hash_len_32_with_seeds(__s + 32, __z + __w.second,
- __y + *(const _Size*)(__s + 16));
+ __y + __loadword<_Size>(__s + 16));
std::swap(__z, __x);
__s += 64;
__len -= 64;
More information about the cfe-commits
mailing list