[llvm] r322859 - [ADT] Add a workaround for GCC miscompiling the trivially copyable Optional

Benjamin Kramer via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 18 07:47:59 PST 2018


Author: d0k
Date: Thu Jan 18 07:47:59 2018
New Revision: 322859

URL: http://llvm.org/viewvc/llvm-project?rev=322859&view=rev
Log:
[ADT] Add a workaround for GCC miscompiling the trivially copyable Optional

I've seen random crashes with GCC 4.8, GCC 6.3 and GCC 7.3, triggered by
my Optional change. All of them affect a different set of targets. This
change fixes the instance of the problem I'm seeing on my local machine,
let's hope it's good enough for the other instances too.

Modified:
    llvm/trunk/include/llvm/ADT/Optional.h

Modified: llvm/trunk/include/llvm/ADT/Optional.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=322859&r1=322858&r2=322859&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/Optional.h (original)
+++ llvm/trunk/include/llvm/ADT/Optional.h Thu Jan 18 07:47:59 2018
@@ -23,6 +23,7 @@
 #include <algorithm>
 #include <cassert>
 #include <new>
+#include <cstring>
 #include <utility>
 
 namespace llvm {
@@ -117,10 +118,16 @@ template <typename T> struct OptionalSto
 
   OptionalStorage() = default;
 
-  OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); }
+  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 &operator=(const T &y) {
-    new (storage.buffer) 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));
     return *this;
   }
 




More information about the llvm-commits mailing list