Fix aliasing bug in optimized static builds with gcc-4.4.
authorPeter D. Barnes, Jr. <barnes26@llnl.gov>
Fri, 30 Aug 2013 12:33:03 -0700
changeset 10189 9b6d0037b360
parent 10188 f2177f4b2cb1
child 10191 fd59d56b782d
Fix aliasing bug in optimized static builds with gcc-4.4.
src/core/model/hash-murmur3.cc
src/core/test/hash-test-suite.cc
--- a/src/core/model/hash-murmur3.cc	Thu Aug 29 23:36:42 2013 -0700
+++ b/src/core/model/hash-murmur3.cc	Fri Aug 30 12:33:03 2013 -0700
@@ -34,33 +34,6 @@
 
 #include <iomanip>
 
-/*
- * \brief Silence erroneous strict alias warning from a gcc 4.4 bug
- *
- * Casting \c (void*) triggers a strict alias warning bug
- * in gcc 4.4 (see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39390).
- *
- * In the murmur3 code, data is returned by
- * \code
- *   void Function (... , void * out)
- *   {
- *     ...
- *     *(uint32_t *)out = ...
- *   }
- * \endcode
- *
- * which triggers the erroneous warning.
- *
- * We suppress strict-alias warnings in this compilation unit.
- * (gcc 4.4 doesn't support the <tt>diagnostic push/pop</tt> pragmas,
- * so we can't narrow down the suppression any further.)
- */
-// Test for gcc 4.4.x
-#define GCC_VERSION (__GNUC__ * 100 + __GNUC_MINOR__)
-#if (GCC_VERSION == 404)
-#  pragma GCC diagnostic ignored "-Wstrict-aliasing"
-#endif
- 
 namespace ns3 {
 
 NS_LOG_COMPONENT_DEFINE ("Hash-Murmur3");
@@ -479,12 +452,28 @@
 {
   using namespace Murmur3Implementation;
   MurmurHash3_x86_128_incr (buffer, size,
-                            (uint32_t *)(void *)m_hash64, (void *)(m_hash64));
+                            (uint32_t *)(void *)m_hash64, m_hash64);
   m_size64 += size;
-  uint64_t hash[2];
+
+  // Simpler would be:
+  //
+  //   uint64_t hash[2];
+  //   MurmurHash3_x86_128_fin (m_size64, m_hash64, hash);
+  //   return hash[0];
+  //
+  // but this triggers an aliasing bug in gcc-4.4 (perhaps related to
+  // http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39390).
+  // In ns-3, this bug produces incorrect results in static optimized
+  // builds only.
+  //
+  // Using uint32_t here avoids the bug, and continues to works with newer gcc.
+  uint32_t hash[4];
+  
   MurmurHash3_x86_128_fin (m_size64,
-                           (uint32_t*)(void *)m_hash64, (void *)hash);
-  return hash[0];
+                           (uint32_t *)(void *)m_hash64, hash);
+  uint64_t result = hash[1];
+  result = (result << 32) | hash[0];
+  return result;
 }
 
 void
@@ -492,8 +481,7 @@
 {
   m_hash32 = (uint32_t)SEED;
   m_size32 = 0;
-  m_hash64[0] = ((uint64_t)(SEED) << 32) + (uint64_t)SEED;
-  m_hash64[1] = ((uint64_t)(SEED) << 32) + (uint64_t)SEED;
+  m_hash64[0] = m_hash64[1] = ((uint64_t)(SEED) << 32) + (uint64_t)SEED;
   m_size64 = 0;
 }
 
--- a/src/core/test/hash-test-suite.cc	Thu Aug 29 23:36:42 2013 -0700
+++ b/src/core/test/hash-test-suite.cc	Fri Aug 30 12:33:03 2013 -0700
@@ -376,22 +376,10 @@
 HashTestSuite::HashTestSuite ()
   : TestSuite ("hash", UNIT)
 {
-// The below tests fail for static optimized builds in gcc-4.4.3 (64-bit)
-// This is likely due to a compiler bug (see also the strict alias
-// warning issue mentioned in hash-murmur3.cc).  It does not impact
-// most users of the code, so we silence the test failure while we
-// continue to use gcc-4.4.3 (python bindings) for other purposes
-//
-// This code can be removed once gcc-4.4.3 is no longer supported
-//
-// Test for gcc 4.4.x
-#define GCC_VERSION (__GNUC__ * 100 + __GNUC_MINOR__)
-#if (GCC_VERSION != 404)
   AddTestCase (new DefaultHashTestCase, QUICK);
   AddTestCase (new Murmur3TestCase, QUICK);
   AddTestCase (new Fnv1aTestCase, QUICK);
   AddTestCase (new IncrementalTestCase, QUICK);
-#endif
   AddTestCase (new Hash32FunctionPtrTestCase, QUICK);
   AddTestCase (new Hash64FunctionPtrTestCase, QUICK);
 }