folly/detail/ThreadLocalDetail.h: avoid UBSAN-detected memcpy abuse
authorJim Meyering <meyering@fb.com>
Wed, 25 Nov 2015 19:06:24 +0000 (11:06 -0800)
committerfacebook-github-bot-4 <folly-bot@fb.com>
Wed, 25 Nov 2015 19:20:23 +0000 (11:20 -0800)
Summary: [technically, the existing code is probably a no-op on
all systems we care about, but since it is officially UB, switching
to a more strict platform could cause trouble, so it's worth fixing]

Calling memcpy with "nullptr" as 2nd argument is undefined, even when
the third argument is zero, and causes a failure when testing with an
UBSAN-enabled binary (-fsanitize=undefined).
Before this change, the buck-run test below would evoke this failure:

  [ RUN      ] ThreadLocalPtr.BasicDestructor
  folly/detail/ThreadLocalDetail.h:533:29: runtime error: null pointer passed as argument 2, which is declared to never be null
  third-party-buck/build/glibc/include/string.h:47:45: note: nonnull attribute specified here

Ironically, the failure of the target-determinator-buck_push_blocking test (due to an unrelated proxygen dep problem) would block me from landing this, so I am adding this line to override it.

Reviewed By: luciang, alexshap

Differential Revision: D2692625

fb-gh-sync-id: 8bdc5cd2899705f39c9565d640921de1f363807d

folly/detail/ThreadLocalDetail.h

index c97083ffecff20aac572213a7863d0e4ef78cbf0..999d169d7bdcc8752d059d7242a499e06ff87f15 100644 (file)
@@ -530,10 +530,11 @@ struct StaticMeta {
         * destructing a ThreadLocal and writing to the elements vector
         * of this thread.
         */
-        memcpy(reallocated, threadEntry->elements,
-               sizeof(ElementWrapper) * prevCapacity);
-        using std::swap;
-        swap(reallocated, threadEntry->elements);
+        if (prevCapacity != 0) {
+          memcpy(reallocated, threadEntry->elements,
+                 sizeof(*reallocated) * prevCapacity);
+        }
+        std::swap(reallocated, threadEntry->elements);
       }
       threadEntry->elementsCapacity = newCapacity;
     }