[scudo] Application & platform compatibility changes

Summary:
This patch changes a few (small) things around for compatibility purposes for
the current Android & Fuchsia work:
- `realloc`'ing some memory that was not allocated with `malloc`, `calloc` or
  `realloc`, while UB according to http://pubs.opengroup.org/onlinepubs/009695399/functions/realloc.html
  is more common that one would think. We now only check this if
  `DeallocationTypeMismatch` is set; change the "mismatch" error
  messages to be more homogeneous;
- some sketchily written but widely used libraries expect a call to `realloc`
  to copy the usable size of the old chunk to the new one instead of the
  requested size. We have to begrundingly abide by this de-facto standard.
  This doesn't seem to impact security either way, unless someone comes up with
  something we didn't think about;
- the CRC32 intrinsics for 64-bit take a 64-bit first argument. This is
  misleading as the upper 32 bits end up being ignored. This was also raising
  `-Wconversion` errors. Change things to take a `u32` as first argument.
  This also means we were (and are) only using 32 bits of the Cookie - not a
  big thing, but worth mentioning.
- Includes-wise: prefer `stddef.h` to `cstddef`, move `scudo_flags.h` where it
  is actually needed.
- Add tests for the memalign-realloc case, and the realloc-usable-size one.

(Edited typos)

Reviewers: alekseyshl

Reviewed By: alekseyshl

Subscribers: llvm-commits

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

llvm-svn: 311018

6 files changed, 92 insertions, 59 deletions

diff --git a/compiler-rt/lib/scudo/scudo_allocator.cpp b/compiler-rt/lib/scudo/scudo_allocator.cpp
index e1758568b53..4f87d35232d 100644
--- a/compiler-rt/lib/scudo/scudo_allocator.cpp
+++ b/compiler-rt/lib/scudo/scudo_allocator.cpp
@@ -16,6 +16,7 @@
 
 #include "scudo_allocator.h"
 #include "scudo_crc32.h"
+#include "scudo_flags.h"
 #include "scudo_tls.h"
 #include "scudo_utils.h"
 
@@ -35,7 +36,7 @@ static uptr Cookie;
 // at compilation or at runtime.
 static atomic_uint8_t HashAlgorithm = { CRC32Software };
 
-INLINE u32 computeCRC32(uptr Crc, uptr Value, uptr *Array, uptr ArraySize) {
+INLINE u32 computeCRC32(u32 Crc, uptr Value, uptr *Array, uptr ArraySize) {
   // If the hardware CRC32 feature is defined here, it was enabled everywhere,
   // as opposed to only for scudo_crc32.cpp. This means that other hardware
   // specific instructions were likely emitted at other places, and as a
@@ -87,7 +88,8 @@ struct ScudoChunk : UnpackedHeader {
     ZeroChecksumHeader.Checksum = 0;
     uptr HeaderHolder[sizeof(UnpackedHeader) / sizeof(uptr)];
     memcpy(&HeaderHolder, &ZeroChecksumHeader, sizeof(HeaderHolder));
-    u32 Crc = computeCRC32(Cookie, reinterpret_cast<uptr>(this), HeaderHolder,
+    u32 Crc = computeCRC32(static_cast<u32>(Cookie),
+                           reinterpret_cast<uptr>(this), HeaderHolder,
                            ARRAY_SIZE(HeaderHolder));
     return static_cast<u16>(Crc);
   }
@@ -514,8 +516,8 @@ struct ScudoAllocator {
       if (Header.AllocType != Type) {
         // With the exception of memalign'd Chunks, that can be still be free'd.
         if (Header.AllocType != FromMemalign || Type != FromMalloc) {
-          dieWithMessage("ERROR: allocation type mismatch on address %p\n",
-                         UserPtr);
+          dieWithMessage("ERROR: allocation type mismatch when deallocating "
+                         "address %p\n", UserPtr);
         }
       }
     }
@@ -546,9 +548,11 @@ struct ScudoAllocator {
       dieWithMessage("ERROR: invalid chunk state when reallocating address "
                      "%p\n", OldPtr);
     }
-    if (UNLIKELY(OldHeader.AllocType != FromMalloc)) {
-      dieWithMessage("ERROR: invalid chunk type when reallocating address %p\n",
-                     OldPtr);
+    if (DeallocationTypeMismatch) {
+      if (UNLIKELY(OldHeader.AllocType != FromMalloc)) {
+        dieWithMessage("ERROR: allocation type mismatch when reallocating "
+                       "address %p\n", OldPtr);
+      }
     }
     uptr UsableSize = Chunk->getUsableSize(&OldHeader);
     // The new size still fits in the current chunk, and the size difference
@@ -567,7 +571,7 @@ struct ScudoAllocator {
     if (NewPtr) {
       uptr OldSize = OldHeader.FromPrimary ? OldHeader.SizeOrUnusedBytes :
           UsableSize - OldHeader.SizeOrUnusedBytes;
-      memcpy(NewPtr, OldPtr, Min(NewSize, OldSize));
+      memcpy(NewPtr, OldPtr, Min(NewSize, UsableSize));
       quarantineOrDeallocateChunk(Chunk, &OldHeader, OldSize);
     }
     return NewPtr;
diff --git a/compiler-rt/lib/scudo/scudo_allocator.h b/compiler-rt/lib/scudo/scudo_allocator.h
index 29d85995a3e..8ecc8cde3e3 100644
--- a/compiler-rt/lib/scudo/scudo_allocator.h
+++ b/compiler-rt/lib/scudo/scudo_allocator.h
@@ -14,8 +14,6 @@
 #ifndef SCUDO_ALLOCATOR_H_
 #define SCUDO_ALLOCATOR_H_
 
-#include "scudo_flags.h"
-
 #include "sanitizer_common/sanitizer_allocator.h"
 
 #if !SANITIZER_LINUX
diff --git a/compiler-rt/lib/scudo/scudo_new_delete.cpp b/compiler-rt/lib/scudo/scudo_new_delete.cpp
index cdefb127b96..c5a1abbed82 100644
--- a/compiler-rt/lib/scudo/scudo_new_delete.cpp
+++ b/compiler-rt/lib/scudo/scudo_new_delete.cpp
@@ -15,7 +15,7 @@
 
 #include "interception/interception.h"
 
-#include <cstddef>
+#include <stddef.h>
 
 using namespace __scudo;
 
diff --git a/compiler-rt/test/scudo/mismatch.cpp b/compiler-rt/test/scudo/mismatch.cpp
index 15dce83ce18..0e51da4a5e7 100644
--- a/compiler-rt/test/scudo/mismatch.cpp
+++ b/compiler-rt/test/scudo/mismatch.cpp
@@ -1,10 +1,12 @@
 // RUN: %clang_scudo %s -o %t
-// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=1 not %run %t mallocdel   2>&1 | FileCheck %s
-// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=0     %run %t mallocdel   2>&1
-// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=1 not %run %t newfree     2>&1 | FileCheck %s
-// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=0     %run %t newfree     2>&1
-// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=1 not %run %t memaligndel 2>&1 | FileCheck %s
-// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=0     %run %t memaligndel 2>&1
+// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=1 not %run %t mallocdel       2>&1 | FileCheck --check-prefix=CHECK-dealloc %s
+// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=0     %run %t mallocdel       2>&1
+// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=1 not %run %t newfree         2>&1 | FileCheck --check-prefix=CHECK-dealloc %s
+// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=0     %run %t newfree         2>&1
+// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=1 not %run %t memaligndel     2>&1 | FileCheck --check-prefix=CHECK-dealloc %s
+// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=0     %run %t memaligndel     2>&1
+// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=1 not %run %t memalignrealloc 2>&1 | FileCheck --check-prefix=CHECK-realloc %s
+// RUN: SCUDO_OPTIONS=DeallocationTypeMismatch=0     %run %t memalignrealloc 2>&1
 
 // Tests that type mismatches between allocation and deallocation functions are
 // caught when the related option is set.
@@ -32,7 +34,14 @@ int main(int argc, char **argv)
     assert(p);
     delete p;
   }
+  if (!strcmp(argv[1], "memalignrealloc")) {
+    void *p = memalign(16, 16);
+    assert(p);
+    p = realloc(p, 32);
+    free(p);
+  }
   return 0;
 }
 
-// CHECK: ERROR: allocation type mismatch on address
+// CHECK-dealloc: ERROR: allocation type mismatch when deallocating address
+// CHECK-realloc: ERROR: allocation type mismatch when reallocating address
diff --git a/compiler-rt/test/scudo/options.cpp b/compiler-rt/test/scudo/options.cpp
index f4afe7d79bf..bb0ed296350 100644
--- a/compiler-rt/test/scudo/options.cpp
+++ b/compiler-rt/test/scudo/options.cpp
@@ -22,4 +22,4 @@ int main(int argc, char **argv)
   return 0;
 }
 
-// CHECK: ERROR: allocation type mismatch on address
+// CHECK: ERROR: allocation type mismatch when deallocating address
diff --git a/compiler-rt/test/scudo/realloc.cpp b/compiler-rt/test/scudo/realloc.cpp
index da377205f10..01149670e15 100644
--- a/compiler-rt/test/scudo/realloc.cpp
+++ b/compiler-rt/test/scudo/realloc.cpp
@@ -1,14 +1,13 @@
 // RUN: %clang_scudo %s -lstdc++ -o %t
-// RUN:     %run %t pointers 2>&1
-// RUN:     %run %t contents 2>&1
-// RUN: not %run %t memalign 2>&1 | FileCheck %s
+// RUN: %run %t pointers 2>&1
+// RUN: %run %t contents 2>&1
+// RUN: %run %t usablesize 2>&1
 
 // Tests that our reallocation function returns the same pointer when the
 // requested size can fit into the previously allocated chunk. Also tests that
 // a new chunk is returned if the size is greater, and that the contents of the
-// chunk are left unchanged.
-// As a final test, make sure that a chunk allocated by memalign cannot be
-// reallocated.
+// chunk are left unchanged. Finally, checks that realloc copies the usable
+// size of the old chunk to the new one (as opposed to the requested size).
 
 #include <assert.h>
 #include <malloc.h>
@@ -24,42 +23,65 @@ int main(int argc, char **argv)
 
   assert(argc == 2);
 
-  for (size_t size : sizes) {
-    if (!strcmp(argv[1], "pointers")) {
-      old_p = p = realloc(nullptr, size);
-      assert(p);
-      size = malloc_usable_size(p);
-      // Our realloc implementation will return the same pointer if the size
-      // requested is lower than or equal to the usable size of the associated
-      // chunk.
-      p = realloc(p, size - 1);
-      assert(p == old_p);
+  if (!strcmp(argv[1], "usablesize")) {
+    // This tests a sketchy behavior inherited from poorly written libraries
+    // that have become somewhat standard. When realloc'ing a chunk, the
+    // copied contents should span the usable size of the chunk, not the
+    // requested size.
+    size_t size = 496, usable_size;
+    p = nullptr;
+    // Make sure we get a chunk with a usable size actually larger than size.
+    do {
+      if (p) free(p);
+      size += 16;
+      p = malloc(size);
+      usable_size = malloc_usable_size(p);
+      assert(usable_size >= size);
+    } while (usable_size == size);
+    for (int i = 0; i < usable_size; i++)
+      reinterpret_cast<char *>(p)[i] = 'A';
+    old_p = p;
+    // Make sure we get a different chunk so that the data is actually copied.
+    do {
+      size *= 2;
       p = realloc(p, size);
-      assert(p == old_p);
-      // And a new one if the size is greater.
-      p = realloc(p, size + 1);
-      assert(p != old_p);
-      // A size of 0 will free the chunk and return nullptr.
-      p = realloc(p, 0);
-      assert(!p);
-      old_p = nullptr;
-    }
-    if (!strcmp(argv[1], "contents")) {
-      p = realloc(nullptr, size);
       assert(p);
-      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++)
-        assert(reinterpret_cast<char *>(p)[i] == 'A');
-    }
-    if (!strcmp(argv[1], "memalign")) {
-      // A chunk coming from memalign cannot be reallocated.
-      p = memalign(16, size);
-      assert(p);
-      p = realloc(p, size);
-      free(p);
+    } while (p == old_p);
+    // The contents of the new chunk must match the old one up to usable_size.
+    for (int i = 0; i < usable_size; i++)
+      assert(reinterpret_cast<char *>(p)[i] == 'A');
+    free(p);
+  } else {
+    for (size_t size : sizes) {
+      if (!strcmp(argv[1], "pointers")) {
+        old_p = p = realloc(nullptr, size);
+        assert(p);
+        size = malloc_usable_size(p);
+        // Our realloc implementation will return the same pointer if the size
+        // requested is lower than or equal to the usable size of the associated
+        // chunk.
+        p = realloc(p, size - 1);
+        assert(p == old_p);
+        p = realloc(p, size);
+        assert(p == old_p);
+        // And a new one if the size is greater.
+        p = realloc(p, size + 1);
+        assert(p != old_p);
+        // A size of 0 will free the chunk and return nullptr.
+        p = realloc(p, 0);
+        assert(!p);
+        old_p = nullptr;
+      }
+      if (!strcmp(argv[1], "contents")) {
+        p = realloc(nullptr, size);
+        assert(p);
+        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++)
+          assert(reinterpret_cast<char *>(p)[i] == 'A');
+      }
     }
   }
   return 0;