rallocm(ALLOCM_ZERO) only zeroes *new* memory

Summary:
If you allocate N bytes and then try to grow that in place using rallocm
(ALLOCM_ZERO | ALLOCM_NO_MOVE) to M > N, our code assumed that all
memory from N to M would be zeroed.  In fact, rallocm only zeroes *new*
memory, so if jemalloc actually allocated N1 > N bytes initially,
rallocm will only zero memory from N1 to M.

Fixed by using calloc.

Test Plan: thread_local_test, tested in production

Reviewed By: delong.j@fb.com

FB internal diff: D519781

diff --git a/folly/detail/ThreadLocalDetail.h b/folly/detail/ThreadLocalDetail.h
index de3ebb8a5f957d7efbbf8f8e2133c0be1b1a8ff0..505682f28d1856f6d1fba108431bafd2bcea8f47 100644 (file)
--- a/folly/detail/ThreadLocalDetail.h
+++ b/folly/detail/ThreadLocalDetail.h
@@ -277,12 +277,18 @@ struct StaticMeta {
         // still linked in meta, so another thread might access invalid memory
         // after realloc succeeds.  We'll copy by hand and update threadEntry_
         // under the lock.
+        //
+        // Note that we're using calloc instead of malloc in order to zero
+        // the entire region.  rallocm (ALLOCM_ZERO) will only zero newly
+        // allocated memory, so if a previous allocation allocated more than
+        // we requested, it's our responsibility to guarantee that the tail
+        // is zeroed.  calloc() is simpler than malloc() followed by memset(),
+        // and potentially faster when dealing with a lot of memory, as
+        // it can get already-zeroed pages from the kernel.
         if ((ptr = static_cast<ElementWrapper*>(
-              malloc(sizeof(ElementWrapper) * newSize))) != NULL) {
+              calloc(newSize, sizeof(ElementWrapper)))) != NULL) {
           memcpy(ptr, threadEntry_.elements,
                  sizeof(ElementWrapper) * prevSize);
-          memset(ptr + prevSize, 0,
-                 (newSize - prevSize) * sizeof(ElementWrapper));
         } else {
           throw std::bad_alloc();
         }