The implementation of GeneralHash::addBits broke C++ aliasing rules; fix
authorJay Foad <jay.foad@gmail.com>
Thu, 23 Feb 2012 09:16:04 +0000 (09:16 +0000)
committerJay Foad <jay.foad@gmail.com>
Thu, 23 Feb 2012 09:16:04 +0000 (09:16 +0000)
it with memcpy. This also fixes a problem on big-endian hosts, where
addUnaligned would return different results depending on the alignment
of the data.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@151247 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/ADT/Hashing.h
lib/Support/Hashing.cpp [deleted file]

index 07c62efe77939fd7c0fd4080b3715196100e2454..27c411e3223d65467e2978a721603332d96b2939 100644 (file)
@@ -19,6 +19,7 @@
 #include "llvm/Support/AlignOf.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/DataTypes.h"
+#include <cstring>
 
 namespace llvm {
 
@@ -140,39 +141,32 @@ private:
     mix(uint32_t(Val));
   }
 
-  template<typename T, bool isAligned>
-  struct addBitsImpl {
-    static void add(GeneralHash &Hash, const T *I, const T *E) {
-      Hash.addUnaligned(
-        reinterpret_cast<const uint8_t *>(I),
-        reinterpret_cast<const uint8_t *>(E));
+  // Add a range of bytes from I to E.
+  void addBytes(const char *I, const char *E) {
+    uint32_t Data;
+    // Note that aliasing rules forbid us from dereferencing
+    // reinterpret_cast<uint32_t *>(I) even if I happens to be suitably
+    // aligned, so we use memcpy instead.
+    for (; E - I >= ptrdiff_t(sizeof Data); I += sizeof Data) {
+      // A clever compiler should be able to turn this memcpy into a single
+      // aligned or unaligned load (depending on the alignment of the type T
+      // that was used in the call to addBits).
+      std::memcpy(&Data, I, sizeof Data);
+      mix(Data);
     }
-  };
-
-  template<typename T>
-  struct addBitsImpl<T, true> {
-    static void add(GeneralHash &Hash, const T *I, const T *E) {
-      Hash.addAligned(
-        reinterpret_cast<const uint32_t *>(I),
-        reinterpret_cast<const uint32_t *>(E));
+    if (I != E) {
+      Data = 0;
+      std::memcpy(&Data, I, E - I);
+      mix(Data);
     }
-  };
+  }
 
   // Add a range of bits from I to E.
   template<typename T>
   void addBits(const T *I, const T *E) {
-    addBitsImpl<T, AlignOf<T>::Alignment_GreaterEqual_4Bytes>::add(*this, I, E);
+    addBytes(reinterpret_cast<const char *>(I),
+             reinterpret_cast<const char *>(E));
   }
-
-  // Add a range of uint32s
-  void addAligned(const uint32_t *I, const uint32_t *E) {
-    while (I < E) {
-      mix(*I++);
-    }
-  }
-
-  // Add a possibly unaligned sequence of bytes.
-  void addUnaligned(const uint8_t *I, const uint8_t *E);
 };
 
 } // end namespace llvm
diff --git a/lib/Support/Hashing.cpp b/lib/Support/Hashing.cpp
deleted file mode 100644 (file)
index 21fcdd7..0000000
+++ /dev/null
@@ -1,46 +0,0 @@
-//===-- llvm/ADT/Hashing.cpp - Utilities for hashing ------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "llvm/ADT/Hashing.h"
-
-namespace llvm {
-
-// Add a possibly unaligned sequence of bytes.
-void GeneralHash::addUnaligned(const uint8_t *I, const uint8_t *E) {
-  ptrdiff_t Length = E - I;
-  if ((uintptr_t(I) & 3) == 0) {
-    while (Length > 3) {
-      mix(*reinterpret_cast<const uint32_t *>(I));
-      I += 4;
-      Length -= 4;
-    }
-  } else {
-    while (Length > 3) {
-      mix(
-        uint32_t(I[0]) +
-        (uint32_t(I[1]) << 8) +
-        (uint32_t(I[2]) << 16) +
-        (uint32_t(I[3]) << 24));
-      I += 4;
-      Length -= 4;
-    }
-  }
-
-  if (Length & 3) {
-    uint32_t Data = 0;
-    switch (Length & 3) {
-      case 3: Data |= uint32_t(I[2]) << 16;   // fall through
-      case 2: Data |= uint32_t(I[1]) << 8;    // fall through
-      case 1: Data |= uint32_t(I[0]); break;
-    }
-    mix(Data);
-  }
-}
-
-}