[compiler-rt] r282030 - [scudo] Fix a bug in the new Secondary Allocator

[Kostya Kortchinsky via llvm-commits]

Author: cryptoad
Date: Tue Sep 20 17:17:59 2016
New Revision: 282030

URL: http://llvm.org/viewvc/llvm-project?rev=282030&view=rev
Log:
[scudo] Fix a bug in the new Secondary Allocator


Summary:
GetActuallyAllocatedSize() was not accounting for the last page of the mapping
being a guard page, and was returning the wrong number of actually allocated
bytes, which in turn would mess up with the realloc logic. Current tests didn't
find this as the size exercised was only serviced by the Primary.

Correct the issue by subtracting PageSize, and update the realloc test to
exercise paths in both the Primary and the Secondary.

Reviewers: kcc

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D24787

Modified:
    compiler-rt/trunk/lib/scudo/scudo_allocator_secondary.h
    compiler-rt/trunk/test/scudo/realloc.cpp

Modified: compiler-rt/trunk/lib/scudo/scudo_allocator_secondary.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_allocator_secondary.h?rev=282030&r1=282029&r2=282030&view=diff
==============================================================================
--- compiler-rt/trunk/lib/scudo/scudo_allocator_secondary.h (original)
+++ compiler-rt/trunk/lib/scudo/scudo_allocator_secondary.h Tue Sep 20 17:17:59 2016
@@ -42,7 +42,7 @@ class ScudoLargeMmapAllocator {
     uptr Ptr = MapBeg + sizeof(SecondaryHeader);
     // TODO(kostyak): add a random offset to Ptr.
     CHECK_GT(Ptr + Size, MapBeg);
-    CHECK_LE(Ptr + Size, MapEnd);
+    CHECK_LT(Ptr + Size, MapEnd);
     SecondaryHeader *Header = getHeader(Ptr);
     Header->MapBeg = MapBeg - PageSize;
     Header->MapSize = MapSize + 2 * PageSize;
@@ -78,7 +78,8 @@ class ScudoLargeMmapAllocator {
 
   uptr GetActuallyAllocatedSize(void *Ptr) {
     SecondaryHeader *Header = getHeader(Ptr);
-    uptr MapEnd = Header->MapBeg + Header->MapSize;
+    // Deduct PageSize as MapEnd includes the trailing guard page.
+    uptr MapEnd = Header->MapBeg + Header->MapSize - PageSize;
     return MapEnd - reinterpret_cast<uptr>(Ptr);
   }
 

Modified: compiler-rt/trunk/test/scudo/realloc.cpp
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/scudo/realloc.cpp?rev=282030&r1=282029&r2=282030&view=diff
==============================================================================
--- compiler-rt/trunk/test/scudo/realloc.cpp (original)
+++ compiler-rt/trunk/test/scudo/realloc.cpp Tue Sep 20 17:17:59 2016
@@ -14,54 +14,59 @@
 #include <malloc.h>
 #include <string.h>
 
+#include <vector>
+
 int main(int argc, char **argv)
 {
   void *p, *old_p;
-  size_t size = 32;
+  // Those sizes will exercise both allocators (Primary & Secondary).
+  std::vector<size_t> sizes{1 << 5, 1 << 17};
 
   assert(argc == 2);
-  if (!strcmp(argv[1], "pointers")) {
-    old_p = p = realloc(nullptr, size);
-    if (!p)
-      return 1;
-    size = malloc_usable_size(p);
-    // Our realloc implementation will return the same pointer if the size
-    // requested is lower or equal to the usable size of the associated chunk.
-    p = realloc(p, size - 1);
-    if (p != old_p)
-      return 1;
-    p = realloc(p, size);
-    if (p != old_p)
-      return 1;
-    // And a new one if the size is greater.
-    p = realloc(p, size + 1);
-    if (p == old_p)
-      return 1;
-    // A size of 0 will free the chunk and return nullptr.
-    p = realloc(p, 0);
-    if (p)
-      return 1;
-    old_p = nullptr;
-  }
-  if (!strcmp(argv[1], "contents")) {
-    p = realloc(nullptr, size);
-    if (!p)
-      return 1;
-    for (int i = 0; i < size; i++)
-      reinterpret_cast<char *>(p)[i] = 'A';
-    p = realloc(p, size + 1);
-    // The contents of the reallocated chunk must match the original one.
-    for (int i = 0; i < size; i++)
-      if (reinterpret_cast<char *>(p)[i] != 'A')
+  for (size_t size : sizes) {
+    if (!strcmp(argv[1], "pointers")) {
+      old_p = p = realloc(nullptr, size);
+      if (!p)
         return 1;
-  }
-  if (!strcmp(argv[1], "memalign")) {
-    // A chunk coming from memalign cannot be reallocated.
-    p = memalign(16, size);
-    if (!p)
-      return 1;
-    p = realloc(p, size);
-    free(p);
+      size = malloc_usable_size(p);
+      // Our realloc implementation will return the same pointer if the size
+      // requested is lower or equal to the usable size of the associated chunk.
+      p = realloc(p, size - 1);
+      if (p != old_p)
+        return 1;
+      p = realloc(p, size);
+      if (p != old_p)
+        return 1;
+      // And a new one if the size is greater.
+      p = realloc(p, size + 1);
+      if (p == old_p)
+        return 1;
+      // A size of 0 will free the chunk and return nullptr.
+      p = realloc(p, 0);
+      if (p)
+        return 1;
+      old_p = nullptr;
+    }
+    if (!strcmp(argv[1], "contents")) {
+      p = realloc(nullptr, size);
+      if (!p)
+        return 1;
+      for (int i = 0; i < size; i++)
+        reinterpret_cast<char *>(p)[i] = 'A';
+      p = realloc(p, size + 1);
+      // The contents of the reallocated chunk must match the original one.
+      for (int i = 0; i < size; i++)
+        if (reinterpret_cast<char *>(p)[i] != 'A')
+          return 1;
+    }
+    if (!strcmp(argv[1], "memalign")) {
+      // A chunk coming from memalign cannot be reallocated.
+      p = memalign(16, size);
+      if (!p)
+        return 1;
+      p = realloc(p, size);
+      free(p);
+    }
   }
   return 0;
 }