[llvm] r322862 - [ADT] Just give up on GCC, I can't fix this.

Benjamin Kramer via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 18 08:23:40 PST 2018


Author: d0k
Date: Thu Jan 18 08:23:40 2018
New Revision: 322862

URL: http://llvm.org/viewvc/llvm-project?rev=322862&view=rev
Log:
[ADT] Just give up on GCC, I can't fix this.

While the memmove workaround fixed it for GCC 6.3. GCC 4.8 and GCC 7.1
are still broken. I have no clue what's going on, just blacklist GCC for
now.

Needless to say this code is ubsan, asan and msan-clean.

Modified:
    llvm/trunk/include/llvm/ADT/Optional.h
    llvm/trunk/unittests/ADT/OptionalTest.cpp

Modified: llvm/trunk/include/llvm/ADT/Optional.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=322862&r1=322861&r2=322862&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/Optional.h (original)
+++ llvm/trunk/include/llvm/ADT/Optional.h Thu Jan 18 08:23:40 2018
@@ -23,7 +23,6 @@
 #include <algorithm>
 #include <cassert>
 #include <new>
-#include <cstring>
 #include <utility>
 
 namespace llvm {
@@ -111,6 +110,7 @@ template <typename T, bool IsPodLike> st
   }
 };
 
+#if !defined(__GNUC__) || defined(__clang__) // GCC up to GCC7 miscompiles this.
 /// Storage for trivially copyable types only.
 template <typename T> struct OptionalStorage<T, true> {
   AlignedCharArrayUnion<T> storage;
@@ -118,21 +118,16 @@ template <typename T> struct OptionalSto
 
   OptionalStorage() = default;
 
-  OptionalStorage(const T &y) : hasVal(true) {
-    // We use memmove here because we know that T is trivially copyable and GCC
-    // up to 7 miscompiles placement new.
-    std::memmove(storage.buffer, &y, sizeof(y));
-  }
+  OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); }
   OptionalStorage &operator=(const T &y) {
+    *reinterpret_cast<T *>(storage.buffer) = y;
     hasVal = true;
-    // We use memmove here because we know that T is trivially copyable and GCC
-    // up to 7 miscompiles placement new.
-    std::memmove(storage.buffer, &y, sizeof(y));
     return *this;
   }
 
   void reset() { hasVal = false; }
 };
+#endif
 } // namespace optional_detail
 
 template <typename T> class Optional {

Modified: llvm/trunk/unittests/ADT/OptionalTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/OptionalTest.cpp?rev=322862&r1=322861&r2=322862&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/OptionalTest.cpp (original)
+++ llvm/trunk/unittests/ADT/OptionalTest.cpp Thu Jan 18 08:23:40 2018
@@ -518,8 +518,7 @@ TEST_F(OptionalTest, OperatorGreaterEqua
   CheckRelation<GreaterEqual>(InequalityLhs, InequalityRhs, !IsLess);
 }
 
-#if (__has_feature(is_trivially_copyable) && defined(_LIBCPP_VERSION)) ||      \
-    (defined(__GNUC__) && __GNUC__ >= 5)
+#if __has_feature(is_trivially_copyable) && defined(_LIBCPP_VERSION)
 static_assert(std::is_trivially_copyable<Optional<int>>::value,
               "Should be trivially copyable");
 static_assert(




More information about the llvm-commits mailing list