[scudo] Make logging more consistent

Summary:
A few changes related to logging:
- prepend `Scudo` to the error messages so that users can identify that we
  reported an error;
- replace a couple of `Report` calls in the RSS check code with
  `dieWithMessage`/`Print`, mark a condition as `UNLIKELY` in the process;
- change some messages so that they all look more or less the same. This
  includes the `CHECK` message;
- adapt a couple of tests with the new strings.

A couple of side notes: this results in a few 1-line-blocks, for which I left
brackets. There doesn't seem to be any style guide for that, I can remove them
if need be. I didn't use `SanitizerToolName` in the strings, but directly
`Scudo` because we are the only users, I could change that too.

Reviewers: alekseyshl, flowerhack

Reviewed By: alekseyshl

Subscribers: mgorny, delcypher, llvm-commits, #sanitizers

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

llvm-svn: 326901

diff --git a/compiler-rt/lib/scudo/scudo_allocator.cpp b/compiler-rt/lib/scudo/scudo_allocator.cpp
index 268f2e7..e238947 100644 (file)
--- a/compiler-rt/lib/scudo/scudo_allocator.cpp
+++ b/compiler-rt/lib/scudo/scudo_allocator.cpp
@@ -135,9 +135,8 @@ namespace Chunk {
         atomic_load_relaxed(getConstAtomicHeader(Ptr));
     *NewUnpackedHeader = bit_cast<UnpackedHeader>(NewPackedHeader);
     if (UNLIKELY(NewUnpackedHeader->Checksum !=
-        computeChecksum(Ptr, NewUnpackedHeader))) {
-      dieWithMessage("ERROR: corrupted chunk header at address %p\n", Ptr);
-    }
+        computeChecksum(Ptr, NewUnpackedHeader)))
+      dieWithMessage("corrupted chunk header at address %p\n", Ptr);
   }
 
   // Packs and stores the header, computing the checksum in the process.
@@ -158,9 +157,8 @@ namespace Chunk {
     PackedHeader OldPackedHeader = bit_cast<PackedHeader>(*OldUnpackedHeader);
     if (UNLIKELY(!atomic_compare_exchange_strong(
             getAtomicHeader(Ptr), &OldPackedHeader, NewPackedHeader,
-            memory_order_relaxed))) {
-      dieWithMessage("ERROR: race on chunk header at address %p\n", Ptr);
-    }
+            memory_order_relaxed)))
+      dieWithMessage("race on chunk header at address %p\n", Ptr);
   }
 }  // namespace Chunk
 
@@ -173,10 +171,8 @@ struct QuarantineCallback {
   void Recycle(void *Ptr) {
     UnpackedHeader Header;
     Chunk::loadHeader(Ptr, &Header);
-    if (UNLIKELY(Header.State != ChunkQuarantine)) {
-      dieWithMessage("ERROR: invalid chunk state when recycling address %p\n",
-                     Ptr);
-    }
+    if (UNLIKELY(Header.State != ChunkQuarantine))
+      dieWithMessage("invalid chunk state when recycling address %p\n", Ptr);
     Chunk::eraseHeader(Ptr);
     void *BackendPtr = Chunk::getBackendPtr(Ptr, &Header);
     if (Header.ClassId)
@@ -236,7 +232,7 @@ struct ScudoAllocator {
   explicit ScudoAllocator(LinkerInitialized)
     : AllocatorQuarantine(LINKER_INITIALIZED) {}
 
-  void performSanityChecks() {
+  NOINLINE void performSanityChecks() {
     // Verify that the header offset field can hold the maximum offset. In the
     // case of the Secondary allocator, it takes care of alignment and the
     // offset will always be 0. In the case of the Primary, the worst case
@@ -251,10 +247,8 @@ struct ScudoAllocator {
     const uptr MaxOffset =
         (MaxPrimaryAlignment - Chunk::getHeaderSize()) >> MinAlignmentLog;
     Header.Offset = MaxOffset;
-    if (Header.Offset != MaxOffset) {
-      dieWithMessage("ERROR: the maximum possible offset doesn't fit in the "
-                     "header\n");
-    }
+    if (Header.Offset != MaxOffset)
+      dieWithMessage("maximum possible offset doesn't fit in header\n");
     // Verify that we can fit the maximum size or amount of unused bytes in the
     // header. Given that the Secondary fits the allocation to a page, the worst
     // case scenario happens in the Primary. It will depend on the second to
@@ -262,16 +256,13 @@ struct ScudoAllocator {
     // The following is an over-approximation that works for our needs.
     const uptr MaxSizeOrUnusedBytes = SizeClassMap::kMaxSize - 1;
     Header.SizeOrUnusedBytes = MaxSizeOrUnusedBytes;
-    if (Header.SizeOrUnusedBytes != MaxSizeOrUnusedBytes) {
-      dieWithMessage("ERROR: the maximum possible unused bytes doesn't fit in "
-                     "the header\n");
-    }
+    if (Header.SizeOrUnusedBytes != MaxSizeOrUnusedBytes)
+      dieWithMessage("maximum possible unused bytes doesn't fit in header\n");
 
     const uptr LargestClassId = SizeClassMap::kLargestClassID;
     Header.ClassId = LargestClassId;
-    if (Header.ClassId != LargestClassId) {
-      dieWithMessage("ERROR: the largest class ID doesn't fit in the header\n");
-    }
+    if (Header.ClassId != LargestClassId)
+      dieWithMessage("largest class ID doesn't fit in header\n");
   }
 
   void init() {
@@ -332,12 +323,9 @@ struct ScudoAllocator {
     //                RSS from /proc/self/statm by default. We might want to
     //                call getrusage directly, even if it's less accurate.
     const uptr CurrentRssMb = GetRSS() >> 20;
-    if (HardRssLimitMb && HardRssLimitMb < CurrentRssMb) {
-      Report("%s: hard RSS limit exhausted (%zdMb vs %zdMb)\n",
-             SanitizerToolName, HardRssLimitMb, CurrentRssMb);
-      DumpProcessMap();
-      Die();
-    }
+    if (HardRssLimitMb && UNLIKELY(HardRssLimitMb < CurrentRssMb))
+      dieWithMessage("hard RSS limit exhausted (%zdMb vs %zdMb)\n",
+                     HardRssLimitMb, CurrentRssMb);
     if (SoftRssLimitMb) {
       if (atomic_load_relaxed(&RssLimitExceeded)) {
         if (CurrentRssMb <= SoftRssLimitMb)
@@ -345,8 +333,8 @@ struct ScudoAllocator {
       } else {
         if (CurrentRssMb > SoftRssLimitMb) {
           atomic_store_relaxed(&RssLimitExceeded, true);
-          Report("%s: soft RSS limit exhausted (%zdMb vs %zdMb)\n",
-                 SanitizerToolName, SoftRssLimitMb, CurrentRssMb);
+          Printf("Scudo INFO: soft RSS limit exhausted (%zdMb vs %zdMb)\n",
+                 SoftRssLimitMb, CurrentRssMb);
         }
       }
     }
@@ -484,33 +472,27 @@ struct ScudoAllocator {
       __sanitizer_free_hook(Ptr);
     if (UNLIKELY(!Ptr))
       return;
-    if (UNLIKELY(!Chunk::isAligned(Ptr))) {
-      dieWithMessage("ERROR: attempted to deallocate a chunk not properly "
-                     "aligned at address %p\n", Ptr);
-    }
+    if (UNLIKELY(!Chunk::isAligned(Ptr)))
+      dieWithMessage("misaligned pointer when deallocating address %p\n", Ptr);
     UnpackedHeader Header;
     Chunk::loadHeader(Ptr, &Header);
-    if (UNLIKELY(Header.State != ChunkAllocated)) {
-      dieWithMessage("ERROR: invalid chunk state when deallocating address "
-                     "%p\n", Ptr);
-    }
+    if (UNLIKELY(Header.State != ChunkAllocated))
+      dieWithMessage("invalid chunk state when deallocating address %p\n", Ptr);
     if (DeallocationTypeMismatch) {
       // The deallocation type has to match the allocation one.
       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 when deallocating "
-                         "address %p\n", Ptr);
-        }
+        if (Header.AllocType != FromMemalign || Type != FromMalloc)
+          dieWithMessage("allocation type mismatch when deallocating address "
+                         "%p\n", Ptr);
       }
     }
     const uptr Size = Header.ClassId ? Header.SizeOrUnusedBytes :
         Chunk::getUsableSize(Ptr, &Header) - Header.SizeOrUnusedBytes;
     if (DeleteSizeMismatch) {
-      if (DeleteSize && DeleteSize != Size) {
-        dieWithMessage("ERROR: invalid sized delete on chunk at address %p\n",
+      if (DeleteSize && DeleteSize != Size)
+        dieWithMessage("invalid sized delete when deallocating address %p\n",
                        Ptr);
-      }
     }
     quarantineOrDeallocateChunk(Ptr, &Header, Size);
   }
@@ -519,21 +501,18 @@ struct ScudoAllocator {
   // size still fits in the chunk.
   void *reallocate(void *OldPtr, uptr NewSize) {
     initThreadMaybe();
-    if (UNLIKELY(!Chunk::isAligned(OldPtr))) {
-      dieWithMessage("ERROR: attempted to reallocate a chunk not properly "
-                     "aligned at address %p\n", OldPtr);
-    }
+    if (UNLIKELY(!Chunk::isAligned(OldPtr)))
+      dieWithMessage("misaligned address when reallocating address %p\n",
+                     OldPtr);
     UnpackedHeader OldHeader;
     Chunk::loadHeader(OldPtr, &OldHeader);
-    if (UNLIKELY(OldHeader.State != ChunkAllocated)) {
-      dieWithMessage("ERROR: invalid chunk state when reallocating address "
-                     "%p\n", OldPtr);
-    }
+    if (UNLIKELY(OldHeader.State != ChunkAllocated))
+      dieWithMessage("invalid chunk state when reallocating address %p\n",
+                     OldPtr);
     if (DeallocationTypeMismatch) {
-      if (UNLIKELY(OldHeader.AllocType != FromMalloc)) {
-        dieWithMessage("ERROR: allocation type mismatch when reallocating "
-                       "address %p\n", OldPtr);
-      }
+      if (UNLIKELY(OldHeader.AllocType != FromMalloc))
+        dieWithMessage("allocation type mismatch when reallocating address "
+                       "%p\n", OldPtr);
     }
     const uptr UsableSize = Chunk::getUsableSize(OldPtr, &OldHeader);
     // The new size still fits in the current chunk, and the size difference
@@ -566,10 +545,8 @@ struct ScudoAllocator {
     UnpackedHeader Header;
     Chunk::loadHeader(Ptr, &Header);
     // Getting the usable size of a chunk only makes sense if it's allocated.
-    if (UNLIKELY(Header.State != ChunkAllocated)) {
-      dieWithMessage("ERROR: invalid chunk state when sizing address %p\n",
-                     Ptr);
-    }
+    if (UNLIKELY(Header.State != ChunkAllocated))
+      dieWithMessage("invalid chunk state when sizing address %p\n", Ptr);
     return Chunk::getUsableSize(Ptr, &Header);
   }
 

diff --git a/compiler-rt/lib/scudo/scudo_termination.cpp b/compiler-rt/lib/scudo/scudo_termination.cpp
index c441ff3..4237d3b 100644 (file)
--- a/compiler-rt/lib/scudo/scudo_termination.cpp
+++ b/compiler-rt/lib/scudo/scudo_termination.cpp
@@ -35,7 +35,7 @@ void SetCheckFailedCallback(CheckFailedCallbackType callback) {}
 
 void NORETURN CheckFailed(const char *File, int Line, const char *Condition,
                           u64 Value1, u64 Value2) {
-  __scudo::dieWithMessage("Scudo CHECK failed: %s:%d %s (%lld, %lld)\n",
+  __scudo::dieWithMessage("CHECK failed at %s:%d %s (%lld, %lld)\n",
                           File, Line, Condition, Value1, Value2);
 }
 

diff --git a/compiler-rt/lib/scudo/scudo_utils.cpp b/compiler-rt/lib/scudo/scudo_utils.cpp
index 2f936bf..05e6321 100644 (file)
--- a/compiler-rt/lib/scudo/scudo_utils.cpp
+++ b/compiler-rt/lib/scudo/scudo_utils.cpp
@@ -38,11 +38,14 @@ extern int VSNPrintf(char *buff, int buff_length, const char *format,
 namespace __scudo {
 
 FORMAT(1, 2) void NORETURN dieWithMessage(const char *Format, ...) {
+  static const char ScudoError[] = "Scudo ERROR: ";
+  static constexpr uptr PrefixSize = sizeof(ScudoError) - 1;
   // Our messages are tiny, 256 characters is more than enough.
   char Message[256];
   va_list Args;
   va_start(Args, Format);
-  VSNPrintf(Message, sizeof(Message), Format, Args);
+  internal_memcpy(Message, ScudoError, PrefixSize);
+  VSNPrintf(Message + PrefixSize, sizeof(Message) - PrefixSize, Format, Args);
   va_end(Args);
   RawWrite(Message);
   Die();

diff --git a/compiler-rt/test/scudo/alignment.c b/compiler-rt/test/scudo/alignment.c
index 6235d50..4e2dc1a 100644 (file)
--- a/compiler-rt/test/scudo/alignment.c
+++ b/compiler-rt/test/scudo/alignment.c
@@ -20,4 +20,4 @@ int main(int argc, char **argv)
   return 0;
 }
 
-// CHECK: ERROR: attempted to deallocate a chunk not properly aligned
+// CHECK: ERROR: misaligned pointer when deallocating address

diff --git a/compiler-rt/test/scudo/sized-delete.cpp b/compiler-rt/test/scudo/sized-delete.cpp
index 85df05e..81151b0 100644 (file)
--- a/compiler-rt/test/scudo/sized-delete.cpp
+++ b/compiler-rt/test/scudo/sized-delete.cpp
@@ -38,4 +38,4 @@ int main(int argc, char **argv)
   return 0;
 }
 
-// CHECK: ERROR: invalid sized delete on chunk at address
+// CHECK: ERROR: invalid sized delete when deallocating address