Skip to content

Commit

Permalink
Fix technical UB + compiler warnings
Browse files Browse the repository at this point in the history
Summary:
* XXH*_update functions technically have UB on pointer arithmetic
beyond the bounds of the input object, which can trigger a compiler
warning in the case of statically sized input buffer. This can be
solved with cast to uintptr_t where available. (In theory if your
buffer is close to an extreme of your address space, it could still
over/underflow, but eh.)

* typedef XXH_endianess in implementation is unused, so removed.
(When it was in xxhash.c it would cause compiler warning.)

Test Plan: existing tests
  • Loading branch information
pdillinger committed Aug 11, 2021
1 parent 2c611a7 commit 4d6083b
Showing 1 changed file with 17 additions and 11 deletions.
28 changes: 17 additions & 11 deletions xxhash.h
Original file line number Diff line number Diff line change
Expand Up @@ -1429,6 +1429,14 @@ static void* XXH_memcpy(void* dest, const void* src, size_t size)
#endif
typedef XXH32_hash_t xxh_u32;

#ifdef UINTPTR_MAX
/* Pointer arithmetic slightly beyond bounds can save some cycles but is
* technically UB on pointer type. Use uintptr_t when available. */
typedef uintptr_t xxh_u8_ptr_arith_t;
#else
typedef const xxh_u8* xxh_u8_ptr_arith_t;
#endif

#ifdef XXH_OLD_NAMES
# define BYTE xxh_u8
# define U8 xxh_u8
Expand Down Expand Up @@ -1534,8 +1542,6 @@ static xxh_u32 XXH_read32(const void* memPtr)


/* *** Endianness *** */
typedef enum { XXH_bigEndian=0, XXH_littleEndian=1 } XXH_endianess;

/*!
* @ingroup tuning
* @def XXH_CPU_LITTLE_ENDIAN
Expand Down Expand Up @@ -2047,8 +2053,8 @@ XXH32_update(XXH32_state_t* state, const void* input, size_t len)
state->memsize = 0;
}

if (p <= bEnd-16) {
const xxh_u8* const limit = bEnd - 16;
if ((xxh_u8_ptr_arith_t)p <= (xxh_u8_ptr_arith_t)bEnd - 16) {
const xxh_u8_ptr_arith_t limit = (xxh_u8_ptr_arith_t)bEnd - 16;
xxh_u32 v1 = state->v1;
xxh_u32 v2 = state->v2;
xxh_u32 v3 = state->v3;
Expand All @@ -2059,7 +2065,7 @@ XXH32_update(XXH32_state_t* state, const void* input, size_t len)
v2 = XXH32_round(v2, XXH_readLE32(p)); p+=4;
v3 = XXH32_round(v3, XXH_readLE32(p)); p+=4;
v4 = XXH32_round(v4, XXH_readLE32(p)); p+=4;
} while (p<=limit);
} while ((xxh_u8_ptr_arith_t)p<=limit);

state->v1 = v1;
state->v2 = v2;
Expand Down Expand Up @@ -2474,8 +2480,8 @@ XXH64_update (XXH64_state_t* state, const void* input, size_t len)
state->memsize = 0;
}

if (p+32 <= bEnd) {
const xxh_u8* const limit = bEnd - 32;
if ((xxh_u8_ptr_arith_t)p + 32 <= (xxh_u8_ptr_arith_t)bEnd) {
const xxh_u8_ptr_arith_t limit = (xxh_u8_ptr_arith_t)bEnd - 32;
xxh_u64 v1 = state->v1;
xxh_u64 v2 = state->v2;
xxh_u64 v3 = state->v3;
Expand All @@ -2486,7 +2492,7 @@ XXH64_update (XXH64_state_t* state, const void* input, size_t len)
v2 = XXH64_round(v2, XXH_readLE64(p)); p+=8;
v3 = XXH64_round(v3, XXH_readLE64(p)); p+=8;
v4 = XXH64_round(v4, XXH_readLE64(p)); p+=8;
} while (p<=limit);
} while ((xxh_u8_ptr_arith_t)p<=limit);

state->v1 = v1;
state->v2 = v2;
Expand Down Expand Up @@ -4645,16 +4651,16 @@ XXH3_update(XXH3_state_t* state,
XXH_ASSERT(input < bEnd);

/* Consume input by a multiple of internal buffer size */
if (input+XXH3_INTERNALBUFFER_SIZE < bEnd) {
const xxh_u8* const limit = bEnd - XXH3_INTERNALBUFFER_SIZE;
if ((xxh_u8_ptr_arith_t)input + XXH3_INTERNALBUFFER_SIZE < (xxh_u8_ptr_arith_t)bEnd) {
const xxh_u8_ptr_arith_t limit = (xxh_u8_ptr_arith_t)bEnd - XXH3_INTERNALBUFFER_SIZE;
do {
XXH3_consumeStripes(state->acc,
&state->nbStripesSoFar, state->nbStripesPerBlock,
input, XXH3_INTERNALBUFFER_STRIPES,
secret, state->secretLimit,
f_acc512, f_scramble);
input += XXH3_INTERNALBUFFER_SIZE;
} while (input<limit);
} while ((xxh_u8_ptr_arith_t)input<limit);
/* for last partial stripe */
memcpy(state->buffer + sizeof(state->buffer) - XXH_STRIPE_LEN, input - XXH_STRIPE_LEN, XXH_STRIPE_LEN);
}
Expand Down

0 comments on commit 4d6083b

Please sign in to comment.