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

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 24 11:30:22 PST 2018


Have you reported the bug? If so it might be a good idea to note the gcc
bug number in a comment.

Thanks,
Rafael

Benjamin Kramer via llvm-commits <llvm-commits at lists.llvm.org> writes:

> 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(
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list