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

[Benjamin Kramer via llvm-commits]

I didn't manage to get this reduced to a version I could send to GCC.
The problem is that the bug manifests differently in every single
version of GCC I tested.

On Wed, Jan 24, 2018 at 8:30 PM, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> 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