folly/Traits.h: fix a deep problem with FBVector
authorJim Meyering <meyering@fb.com>
Fri, 13 Jan 2017 21:01:25 +0000 (13:01 -0800)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Fri, 13 Jan 2017 21:03:02 +0000 (13:03 -0800)
Summary:
This started when many fbvector-related tests began failing with reports of
ASAN-detected heap abuse (usually invalid free of an address 16 bytes
into an already-freed buffer).

It was very specific, though. The bug struck only when:
  - gcc-5-based platform
  - compiling with clang (~3.8) and GCC5's libstdc++, not gcc
  - strings were short enough to reside within an in-situ std::string

Why? because FBVector.h:1588's conditional:

  if (folly::IsRelocatable<T>::value && usingStdAllocator::value) {

was true for clang (erroneously), but false for gcc. The difference
was in `IsRelocatable<std::string>::value`.  However, manual tests showed that
each component from the definition of `IsRelocatable` were the same for both
gcc and clang. Then Jay Feldblum realized that we'd probably specialized
that trait for `std::string`, and sure enough, I found the culprit:

```
FOLLY_ASSUME_FBVECTOR_COMPATIBLE_3(std::basic_string)
```

Reviewed By: pixelb

Differential Revision: D4414159

fbshipit-source-id: 60e3fb4b096ec77cbd2b48c561e5c6ec8f128fff

folly/Traits.h

index 88d46a581648a8207e764a8cb908a7ab9058d59a..ed46cba91ce9778d5741b353a224959c99960fac 100644 (file)
@@ -691,8 +691,8 @@ constexpr initlist_construct_t initlist_construct{};
 
 // Assume nothing when compiling with MSVC.
 #ifndef _MSC_VER
-// gcc-5.0 changed string's implementation in libgcc to be non-relocatable
-#if __GNUC__ < 5
+// gcc-5.0 changed string's implementation in libstdc++ to be non-relocatable
+#if !_GLIBCXX_USE_CXX11_ABI
 FOLLY_ASSUME_FBVECTOR_COMPATIBLE_3(std::basic_string)
 #endif
 FOLLY_ASSUME_FBVECTOR_COMPATIBLE_2(std::vector)