From f6f52f3e575f1c7dc75bddcfa39db37e79753836 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Wed, 25 Nov 2015 11:06:24 -0800 Subject: [PATCH] folly/detail/ThreadLocalDetail.h: avoid UBSAN-detected memcpy abuse 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 | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/folly/detail/ThreadLocalDetail.h b/folly/detail/ThreadLocalDetail.h index c97083ff..999d169d 100644 --- a/folly/detail/ThreadLocalDetail.h +++ b/folly/detail/ThreadLocalDetail.h @@ -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; } -- 2.34.1