[PATCH] D70276: [scudo][standalone] Enabled SCUDO_DEBUG for tests + fixes

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad created this revision.
cryptoad added reviewers: hctim, pcc, cferris, eugenis, vitalybuka.
Herald added subscribers: Sanitizers, mgorny.
Herald added projects: Sanitizers, LLVM.

`SCUDO_DEBUG` was not enabled for unit tests, meaning the `DCHECK`s
were never tripped. While turning this on, I discovered that a few
of those not-exercised checks were actually wrong. This CL addresses
those incorrect checks.

Not that to work in tests `CHECK_IMPL` has to explicitely use the
`scudo` namespace. Also changes a C cast to a C++ cast.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70276

Files:
  compiler-rt/lib/scudo/standalone/combined.h
  compiler-rt/lib/scudo/standalone/internal_defs.h
  compiler-rt/lib/scudo/standalone/secondary.h
  compiler-rt/lib/scudo/standalone/size_class_map.h
  compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
  compiler-rt/lib/scudo/standalone/vector.h


Index: compiler-rt/lib/scudo/standalone/vector.h
===================================================================
--- compiler-rt/lib/scudo/standalone/vector.h
+++ compiler-rt/lib/scudo/standalone/vector.h
@@ -84,7 +84,8 @@
     DCHECK_LE(Size, NewCapacity);
     const uptr NewCapacityBytes =
         roundUpTo(NewCapacity * sizeof(T), getPageSizeCached());
-    T *NewData = (T *)map(nullptr, NewCapacityBytes, "scudo:vector");
+    T *NewData =
+        reinterpret_cast<T *>(map(nullptr, NewCapacityBytes, "scudo:vector"));
     if (Data) {
       memcpy(NewData, Data, Size * sizeof(T));
       unmap(Data, CapacityBytes);
Index: compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
===================================================================
--- compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
+++ compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
@@ -11,6 +11,7 @@
   -I${COMPILER_RT_SOURCE_DIR}/lib
   -I${COMPILER_RT_SOURCE_DIR}/lib/scudo/standalone
   -DGTEST_HAS_RTTI=0
+  -DSCUDO_DEBUG=1
   # Extra flags for the C++ tests
   # TODO(kostyak): find a way to make -fsized-deallocation work
   -Wno-mismatched-new-delete)
Index: compiler-rt/lib/scudo/standalone/size_class_map.h
===================================================================
--- compiler-rt/lib/scudo/standalone/size_class_map.h
+++ compiler-rt/lib/scudo/standalone/size_class_map.h
@@ -120,7 +120,8 @@
       if (C < LargestClassId)
         CHECK_EQ(getClassIdBySize(S + 1), C + 1);
       CHECK_EQ(getClassIdBySize(S - 1), C);
-      CHECK_GT(getSizeByClassId(C), getSizeByClassId(C - 1));
+      if (C - 1 != BatchClassId)
+        CHECK_GT(getSizeByClassId(C), getSizeByClassId(C - 1));
     }
     // Do not perform the loop if the maximum size is too large.
     if (MaxSizeLog > 19)
@@ -129,7 +130,7 @@
       const uptr C = getClassIdBySize(S);
       CHECK_LT(C, NumClasses);
       CHECK_GE(getSizeByClassId(C), S);
-      if (C > 0)
+      if (C - 1 != BatchClassId)
         CHECK_LT(getSizeByClassId(C - 1), S);
     }
   }
Index: compiler-rt/lib/scudo/standalone/secondary.h
===================================================================
--- compiler-rt/lib/scudo/standalone/secondary.h
+++ compiler-rt/lib/scudo/standalone/secondary.h
@@ -114,7 +114,7 @@
 void *MapAllocator<MaxFreeListSize>::allocate(uptr Size, uptr AlignmentHint,
                                               uptr *BlockEnd,
                                               bool ZeroContents) {
-  DCHECK_GT(Size, AlignmentHint);
+  DCHECK_GE(Size, AlignmentHint);
   const uptr PageSize = getPageSizeCached();
   const uptr RoundedSize =
       roundUpTo(Size + LargeBlock::getHeaderSize(), PageSize);
Index: compiler-rt/lib/scudo/standalone/internal_defs.h
===================================================================
--- compiler-rt/lib/scudo/standalone/internal_defs.h
+++ compiler-rt/lib/scudo/standalone/internal_defs.h
@@ -84,12 +84,12 @@
 
 #define CHECK_IMPL(C1, Op, C2)                                                 \
   do {                                                                         \
-    u64 V1 = (u64)(C1);                                                        \
-    u64 V2 = (u64)(C2);                                                        \
+    scudo::u64 V1 = (scudo::u64)(C1);                                          \
+    scudo::u64 V2 = (scudo::u64)(C2);                                          \
     if (UNLIKELY(!(V1 Op V2))) {                                               \
-      reportCheckFailed(__FILE__, __LINE__, "(" #C1 ") " #Op " (" #C2 ")", V1, \
-                        V2);                                                   \
-      die();                                                                   \
+      scudo::reportCheckFailed(__FILE__, __LINE__,                             \
+                               "(" #C1 ") " #Op " (" #C2 ")", V1, V2);         \
+      scudo::die();                                                            \
     }                                                                          \
   } while (false)
 
Index: compiler-rt/lib/scudo/standalone/combined.h
===================================================================
--- compiler-rt/lib/scudo/standalone/combined.h
+++ compiler-rt/lib/scudo/standalone/combined.h
@@ -222,7 +222,7 @@
     if (UNLIKELY(!isAligned(UserPtr, Alignment))) {
       const uptr AlignedUserPtr = roundUpTo(UserPtr, Alignment);
       const uptr Offset = AlignedUserPtr - UserPtr;
-      DCHECK_GT(Offset, 2 * sizeof(u32));
+      DCHECK_GE(Offset, 2 * sizeof(u32));
       // The BlockMarker has no security purpose, but is specifically meant for
       // the chunk iteration function that can be used in debugging situations.
       // It is the only situation where we have to locate the start of a chunk


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70276.229411.patch
Type: text/x-patch
Size: 4853 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191114/1fec03dd/attachment-0001.bin>