Always take BLOSC_STRICT_ALIGN path

thesamesam · thesamesam · commit 18c48d5efe49 · 2023-08-21T08:12:50.000+01:00
Unaligned access is UB, even on x86. UBsan will also detect this (-fsanitize=undefined or -fsanitize=alignment). On arm, I hit SIGBUSes in pandas's test suite via pandas->pytables->c-blosc2 because of unaligned loads and stores. Modern compilers are capable of optimising the "slow" path bitshifts and memcpy into faster alternatives where it is legal. Bug: https://bugs.gentoo.org/911660 Bug: pandas-dev/pandas#54391 Bug: pandas-dev/pandas#54396 Signed-off-by: Sam James <sam@gentoo.org>
diff --git a/blosc/blosc2.c b/blosc/blosc2.c
@@ -1045,11 +1045,7 @@ static bool get_run(const uint8_t* ip, const uint8_t* ip_bound) {
   /* Broadcast the value for every byte in a 64-bit register */
   memset(&value, x, 8);
   while (ip < (ip_bound - 8)) {
-#if defined(BLOSC_STRICT_ALIGN)
     memcpy(&value2, ip, 8);
-#else
-    value2 = *(int64_t*)ip;
-#endif
     if (value != value2) {
       // Values differ.  We don't have a run.
       return false;
diff --git a/blosc/blosclz.c b/blosc/blosclz.c
@@ -46,13 +46,8 @@
 #define MAX_DISTANCE 8191
 #define MAX_FARDISTANCE (65535 + MAX_DISTANCE - 1)
 
-#ifdef BLOSC_STRICT_ALIGN
-  #define BLOSCLZ_READU16(p) ((p)[0] | (p)[1]<<8)
-  #define BLOSCLZ_READU32(p) ((p)[0] | (p)[1]<<8 | (p)[2]<<16 | (p)[3]<<24)
-#else
-  #define BLOSCLZ_READU16(p) *((const uint16_t*)(p))
-  #define BLOSCLZ_READU32(p) *((const uint32_t*)(p))
-#endif
+#define BLOSCLZ_READU16(p) ((p)[0] | (p)[1]<<8)
+#define BLOSCLZ_READU32(p) ((p)[0] | (p)[1]<<8 | (p)[2]<<16 | (p)[3]<<24)
 
 #define HASH_LOG (14U)
 
@@ -123,11 +118,7 @@ static uint8_t *get_run(uint8_t *ip, const uint8_t *ip_bound, const uint8_t *ref
   memset(&value, x, 8);
   /* safe because the outer check against ip limit */
   while (ip < (ip_bound - sizeof(int64_t))) {
-#if defined(BLOSC_STRICT_ALIGN)
     memcpy(&value2, ref, 8);
-#else
-    value2 = ((int64_t*)ref)[0];
-#endif
     if (value != value2) {
       /* Return the byte that starts to differ */
       while (*ref++ == x) ip++;
@@ -146,19 +137,6 @@ static uint8_t *get_run(uint8_t *ip, const uint8_t *ip_bound, const uint8_t *ref
 
 /* Return the byte that starts to differ */
 uint8_t *get_match(uint8_t *ip, const uint8_t *ip_bound, const uint8_t *ref) {
-#if !defined(BLOSC_STRICT_ALIGN)
-  while (ip < (ip_bound - sizeof(int64_t))) {
-    if (*(int64_t*)ref != *(int64_t*)ip) {
-      /* Return the byte that starts to differ */
-      while (*ref++ == *ip++) {}
-      return ip;
-    }
-    else {
-      ip += sizeof(int64_t);
-      ref += sizeof(int64_t);
-    }
-  }
-#endif
   /* Look into the remainder */
   while ((ip < ip_bound) && (*ref++ == *ip++)) {}
   return ip;
diff --git a/blosc/fastcopy.c b/blosc/fastcopy.c
@@ -22,9 +22,7 @@
 
 #include <assert.h>
 #include <stdint.h>
-#if defined(BLOSC_STRICT_ALIGN)
 #include <string.h>
-#endif
 
 /*
  * Use inlined functions for supported systems.
@@ -40,13 +38,9 @@ static inline unsigned char *copy_1_bytes(unsigned char *out, const unsigned cha
 }
 
 static inline unsigned char *copy_2_bytes(unsigned char *out, const unsigned char *from) {
-#if defined(BLOSC_STRICT_ALIGN)
   uint16_t chunk;
   memcpy(&chunk, from, 2);
   memcpy(out, &chunk, 2);
-#else
-  *(uint16_t *) out = *(uint16_t *) from;
-#endif
   return out + 2;
 }
 
@@ -56,13 +50,9 @@ static inline unsigned char *copy_3_bytes(unsigned char *out, const unsigned cha
 }
 
 static inline unsigned char *copy_4_bytes(unsigned char *out, const unsigned char *from) {
-#if defined(BLOSC_STRICT_ALIGN)
   uint32_t chunk;
   memcpy(&chunk, from, 4);
   memcpy(out, &chunk, 4);
-#else
-  *(uint32_t *) out = *(uint32_t *) from;
-#endif
   return out + 4;
 }
 
@@ -82,13 +72,9 @@ static inline unsigned char *copy_7_bytes(unsigned char *out, const unsigned cha
 }
 
 static inline unsigned char *copy_8_bytes(unsigned char *out, const unsigned char *from) {
-#if defined(BLOSC_STRICT_ALIGN)
   uint64_t chunk;
   memcpy(&chunk, from, 8);
   memcpy(out, &chunk, 8);
-#else
-  *(uint64_t *) out = *(uint64_t *) from;
-#endif
   return out + 8;
 }
 
@@ -99,17 +85,10 @@ static inline unsigned char *copy_16_bytes(unsigned char *out, const unsigned ch
   chunk = _mm_loadu_si128((__m128i*)from);
   _mm_storeu_si128((__m128i*)out, chunk);
   out += 16;
-#elif !defined(BLOSC_STRICT_ALIGN)
-  *(uint64_t*)out = *(uint64_t*)from;
-   from += 8; out += 8;
-   *(uint64_t*)out = *(uint64_t*)from;
-   from += 8; out += 8;
-#else
-   int i;
-   for (i = 0; i < 16; i++) {
-     *out++ = *from++;
-   }
-#endif
+  int i;
+  for (i = 0; i < 16; i++) {
+    *out++ = *from++;
+  }
   return out;
 }
 
@@ -127,15 +106,6 @@ static inline unsigned char *copy_32_bytes(unsigned char *out, const unsigned ch
   chunk = _mm_loadu_si128((__m128i*)from);
   _mm_storeu_si128((__m128i*)out, chunk);
   out += 16;
-#elif !defined(BLOSC_STRICT_ALIGN)
-  *(uint64_t*)out = *(uint64_t*)from;
-  from += 8; out += 8;
-  *(uint64_t*)out = *(uint64_t*)from;
-  from += 8; out += 8;
-  *(uint64_t*)out = *(uint64_t*)from;
-  from += 8; out += 8;
-  *(uint64_t*)out = *(uint64_t*)from;
-  from += 8; out += 8;
 #else
   int i;
   for (i = 0; i < 32; i++) {
@@ -159,32 +129,9 @@ static inline unsigned char *copy_32_bytes(unsigned char *out, const unsigned ch
 static inline unsigned char *copy_bytes(unsigned char *out, const unsigned char *from, unsigned len) {
   assert(len < 8);
 
-#ifdef BLOSC_STRICT_ALIGN
   while (len--) {
     *out++ = *from++;
   }
-#else
-  switch (len) {
-    case 7:
-      return copy_7_bytes(out, from);
-    case 6:
-      return copy_6_bytes(out, from);
-    case 5:
-      return copy_5_bytes(out, from);
-    case 4:
-      return copy_4_bytes(out, from);
-    case 3:
-      return copy_3_bytes(out, from);
-    case 2:
-      return copy_2_bytes(out, from);
-    case 1:
-      return copy_1_bytes(out, from);
-    case 0:
-      return out;
-    default:
-      assert(0);
-  }
-#endif /* BLOSC_STRICT_ALIGN */
   return out;
 }
 
diff --git a/include/blosc2/blosc2-common.h b/include/blosc2/blosc2-common.h
@@ -42,34 +42,6 @@
   #define __SSE2__
 #endif
 
-/*
- * Detect if the architecture is fine with unaligned access.
- */
-#if !defined(BLOSC_STRICT_ALIGN)
-#define BLOSC_STRICT_ALIGN
-#if defined(__i386__) || defined(__386) || defined (__amd64)  /* GNU C, Sun Studio */
-#undef BLOSC_STRICT_ALIGN
-#elif defined(__i486__) || defined(__i586__) || defined(__i686__)  /* GNU C */
-#undef BLOSC_STRICT_ALIGN
-#elif defined(_M_IX86) || defined(_M_X64)   /* Intel, MSVC */
-#undef BLOSC_STRICT_ALIGN
-#elif defined(__386)
-#undef BLOSC_STRICT_ALIGN
-#elif defined(_X86_) /* MinGW */
-#undef BLOSC_STRICT_ALIGN
-#elif defined(__I86__) /* Digital Mars */
-#undef BLOSC_STRICT_ALIGN
-/* Modern ARM systems (like ARM64) should support unaligned access
-   quite efficiently. */
-#elif defined(__ARM_FEATURE_UNALIGNED)   /* ARM, GNU C */
-#undef BLOSC_STRICT_ALIGN
-#elif defined(_ARCH_PPC) || defined(__PPC__)
-/* Modern PowerPC systems (like POWER8) should support unaligned access
-   quite efficiently. */
-#undef BLOSC_STRICT_ALIGN
-#endif
-#endif
-
 #if defined(__SSE2__)
   #include <emmintrin.h>
 #endif
diff --git a/plugins/codecs/ndlz/ndlz4x4.c b/plugins/codecs/ndlz/ndlz4x4.c
@@ -45,13 +45,8 @@
 #define MAX_DISTANCE 65535
 
 
-#ifdef BLOSC_STRICT_ALIGN
 #define NDLZ_READU16(p) ((p)[0] | (p)[1]<<8)
 #define NDLZ_READU32(p) ((p)[0] | (p)[1]<<8 | (p)[2]<<16 | (p)[3]<<24)
-#else
-#define NDLZ_READU16(p) *((const uint16_t*)(p))
-#define NDLZ_READU32(p) *((const uint32_t*)(p))
-#endif
 
 #define HASH_LOG (12)
 
diff --git a/plugins/codecs/ndlz/ndlz8x8.c b/plugins/codecs/ndlz/ndlz8x8.c
@@ -45,13 +45,8 @@
 #define MAX_DISTANCE 65535
 
 
-#ifdef BLOSC_STRICT_ALIGN
 #define NDLZ_READU16(p) ((p)[0] | (p)[1]<<8)
 #define NDLZ_READU32(p) ((p)[0] | (p)[1]<<8 | (p)[2]<<16 | (p)[3]<<24)
-#else
-#define NDLZ_READU16(p) *((const uint16_t*)(p))
-#define NDLZ_READU32(p) *((const uint32_t*)(p))
-#endif
 
 #define HASH_LOG (12)
 
