Skip to content

Commit

Permalink
MurmerHash: avoid undef-behavior in C
Browse files Browse the repository at this point in the history
This pointer is not necessarily properly aligned, making this undefined behavior.
All modern compilers should optimize this to the proper sequence for unaligned access.
(confirmed with objdump that Apple LLVM version 9.0.0 (clang-900.0.38)) gives exactly
the same asm for hashing.o as before)

fix JuliaLang#26512
  • Loading branch information
vtjnash authored and Keno committed Dec 18, 2018
1 parent 92ac90e commit 78159b9
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 52 deletions.
37 changes: 6 additions & 31 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1019,37 +1019,12 @@ void jl_register_fptrs(uint64_t sysimage_base, const struct _jl_sysimg_fptrs_t *

extern arraylist_t partial_inst;

STATIC_INLINE uint64_t jl_load_unaligned_i64(const void *ptr) JL_NOTSAFEPOINT
{
uint64_t val;
memcpy(&val, ptr, 8);
return val;
}
STATIC_INLINE uint32_t jl_load_unaligned_i32(const void *ptr) JL_NOTSAFEPOINT
{
uint32_t val;
memcpy(&val, ptr, 4);
return val;
}
STATIC_INLINE uint16_t jl_load_unaligned_i16(const void *ptr) JL_NOTSAFEPOINT
{
uint16_t val;
memcpy(&val, ptr, 2);
return val;
}

STATIC_INLINE void jl_store_unaligned_i64(void *ptr, uint64_t val) JL_NOTSAFEPOINT
{
memcpy(ptr, &val, 8);
}
STATIC_INLINE void jl_store_unaligned_i32(void *ptr, uint32_t val) JL_NOTSAFEPOINT
{
memcpy(ptr, &val, 4);
}
STATIC_INLINE void jl_store_unaligned_i16(void *ptr, uint16_t val) JL_NOTSAFEPOINT
{
memcpy(ptr, &val, 2);
}
STATIC_INLINE uint64_t jl_load_unaligned_i64(const void *ptr) JL_NOTSAFEPOINT;
STATIC_INLINE uint32_t jl_load_unaligned_i32(const void *ptr) JL_NOTSAFEPOINT;
STATIC_INLINE uint16_t jl_load_unaligned_i16(const void *ptr) JL_NOTSAFEPOINT;
STATIC_INLINE void jl_store_unaligned_i64(void *ptr, uint64_t val) JL_NOTSAFEPOINT;
STATIC_INLINE void jl_store_unaligned_i32(void *ptr, uint32_t val) JL_NOTSAFEPOINT;
STATIC_INLINE void jl_store_unaligned_i16(void *ptr, uint16_t val) JL_NOTSAFEPOINT;

#if jl_has_builtin(__builtin_assume_aligned) || defined(_COMPILER_GCC_)
#define jl_assume_aligned(ptr, align) __builtin_assume_aligned(ptr, align)
Expand Down
28 changes: 7 additions & 21 deletions src/support/MurmurHash3.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,6 @@ static inline uint64_t rotl64 ( uint64_t x, int8_t r )

#endif // !defined(_MSC_VER)

//-----------------------------------------------------------------------------
// Block read - if your platform needs to do endian-swapping or can only
// handle aligned reads, do the conversion here

FORCE_INLINE uint32_t getblock32 ( const uint32_t * p, int i )
{
return p[i];
}

FORCE_INLINE uint64_t getblock64 ( const uint64_t * p, int i )
{
return p[i];
}

//-----------------------------------------------------------------------------
// Finalization mix - force all bits of a hash block to avalanche

Expand Down Expand Up @@ -109,7 +95,7 @@ void MurmurHash3_x86_32 ( const void * key, int len,

for(int i = -nblocks; i; i++)
{
uint32_t k1 = getblock32(blocks,i);
uint32_t k1 = jl_load_unaligned_i32(blocks + i);

k1 *= c1;
k1 = ROTL32(k1,15);
Expand Down Expand Up @@ -170,10 +156,10 @@ void MurmurHash3_x86_128 ( const void * key, const int len,

for(int i = -nblocks; i; i++)
{
uint32_t k1 = getblock32(blocks,i*4+0);
uint32_t k2 = getblock32(blocks,i*4+1);
uint32_t k3 = getblock32(blocks,i*4+2);
uint32_t k4 = getblock32(blocks,i*4+3);
uint32_t k1 = jl_load_unaligned_i32(blocks + i*4 + 0);
uint32_t k2 = jl_load_unaligned_i32(blocks + i*4 + 1);
uint32_t k3 = jl_load_unaligned_i32(blocks + i*4 + 2);
uint32_t k4 = jl_load_unaligned_i32(blocks + i*4 + 3);

k1 *= c1; k1 = ROTL32(k1,15); k1 *= c2; h1 ^= k1;

Expand Down Expand Up @@ -274,8 +260,8 @@ void MurmurHash3_x64_128 ( const void * key, const int len,

for(int i = 0; i < nblocks; i++)
{
uint64_t k1 = getblock64(blocks,i*2+0);
uint64_t k2 = getblock64(blocks,i*2+1);
uint64_t k1 = jl_load_unaligned_i64(blocks + i*2 + 0);
uint64_t k2 = jl_load_unaligned_i64(blocks + i*2 + 1);

k1 *= c1; k1 = ROTL64(k1,31); k1 *= c2; h1 ^= k1;

Expand Down
34 changes: 34 additions & 0 deletions src/support/dtypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <stddef.h>
#include <stddef.h> // double include of stddef.h fixes #3421
#include <stdint.h>
#include <string.h> // memcpy
#if defined(_COMPILER_INTEL_)
#include <mathimf.h>
#else
Expand Down Expand Up @@ -220,4 +221,37 @@ typedef enum { T_INT8, T_UINT8, T_INT16, T_UINT16, T_INT32, T_UINT32,
#define JL_UNUSED
#endif

STATIC_INLINE uint64_t jl_load_unaligned_i64(const void *ptr)
{
uint64_t val;
memcpy(&val, ptr, 8);
return val;
}
STATIC_INLINE uint32_t jl_load_unaligned_i32(const void *ptr)
{
uint32_t val;
memcpy(&val, ptr, 4);
return val;
}
STATIC_INLINE uint16_t jl_load_unaligned_i16(const void *ptr)
{
uint16_t val;
memcpy(&val, ptr, 2);
return val;
}

STATIC_INLINE void jl_store_unaligned_i64(void *ptr, uint64_t val)
{
memcpy(ptr, &val, 8);
}
STATIC_INLINE void jl_store_unaligned_i32(void *ptr, uint32_t val)
{
memcpy(ptr, &val, 4);
}
STATIC_INLINE void jl_store_unaligned_i16(void *ptr, uint16_t val)
{
memcpy(ptr, &val, 2);
}


#endif /* DTYPES_H */

0 comments on commit 78159b9

Please sign in to comment.