[scudo] Make logging more consistent

[cryptoad]

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.

[cryptoad]

Rebased.

[alekseyshl]

lib/scudo/scudo_allocator.cpp

174–175

I'd remove brackets in one liners to be consistent with the rest of the code.

[cryptoad]

Remove brackets from 1-liners (modulo the 80 character wrap).

lib/scudo/scudo_allocator.cpp

namespace Chunk {
 
   // Loads and unpacks the header, verifying the checksum in the process.
   static INLINE
   void loadHeader(const void *Ptr, UnpackedHeader *NewUnpackedHeader) {
     PackedHeader NewPackedHeader =
         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.
   static INLINE void storeHeader(void *Ptr, UnpackedHeader *NewUnpackedHeader) {
     NewUnpackedHeader->Checksum = computeChecksum(Ptr, NewUnpackedHeader);
     PackedHeader NewPackedHeader = bit_cast<PackedHeader>(*NewUnpackedHeader);
     atomic_store_relaxed(getAtomicHeader(Ptr), NewPackedHeader);
   }
 
   // Packs and stores the header, computing the checksum in the process. We
   // compare the current header with the expected provided one to ensure that
   // we are not being raced by a corruption occurring in another thread.
   static INLINE void compareExchangeHeader(void *Ptr,
                                            UnpackedHeader *NewUnpackedHeader,
                                            UnpackedHeader *OldUnpackedHeader) {
     NewUnpackedHeader->Checksum = computeChecksum(Ptr, NewUnpackedHeader);
     PackedHeader NewPackedHeader = bit_cast<PackedHeader>(*NewUnpackedHeader);
     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
 
 struct QuarantineCallback {
   explicit QuarantineCallback(AllocatorCache *Cache)
     : Cache_(Cache) {}
 
   // Chunk recycling function, returns a quarantined chunk to the backend,
   // first making sure it hasn't been tampered with.
   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)
       getBackendAllocator().deallocatePrimary(Cache_, BackendPtr,
                                               Header.ClassId);
     else
       getBackendAllocator().deallocateSecondary(BackendPtr);
   }
struct ScudoAllocator {
   uptr HardRssLimitMb;
   uptr SoftRssLimitMb;
   atomic_uint8_t RssLimitExceeded;
   atomic_uint64_t RssLastCheckedAtNS;
 
   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
     // scenario happens in the last size class, when the backend allocation
     // would already be aligned on the requested alignment, which would happen
     // to be the maximum alignment that would fit in that size class. As a
     // result, the maximum offset will be at most the maximum alignment for the
     // last size class minus the header size, in multiples of MinAlignment.
     UnpackedHeader Header = {};
     const uptr MaxPrimaryAlignment =
         1 << MostSignificantSetBitIndex(SizeClassMap::kMaxSize - MinAlignment);
     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
     // last and last class sizes, as well as the dynamic base for the Primary.
     // 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() {
     SanitizerToolName = "Scudo";
     initFlags();
 
     performSanityChecks();
 
if (LIKELY(CurrentCheck < LastCheck + (100ULL * 1000000ULL)))
       return atomic_load_relaxed(&RssLimitExceeded);
     if (!atomic_compare_exchange_weak(&RssLastCheckedAtNS, &LastCheck,
                                       CurrentCheck, memory_order_relaxed))
       return atomic_load_relaxed(&RssLimitExceeded);
     // TODO(kostyak): We currently use sanitizer_common's GetRSS which reads the
     //                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)
           atomic_store_relaxed(&RssLimitExceeded, false);
       } 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);
         }
       }
     }
     return atomic_load_relaxed(&RssLimitExceeded);
   }
 
   // Allocates a chunk.
   void *allocate(uptr Size, uptr Alignment, AllocType Type,
void deallocate(void *Ptr, uptr DeleteSize, AllocType Type) {
     // where the only heap operation performed in a thread would be a free past
     // the TLS destructors, ending up in initialized thread specific data never
     // being destroyed properly. Any other heap operation will do a full init.
     initThreadMaybe(/*MinimalInit=*/true);
     if (SCUDO_CAN_USE_HOOKS && &__sanitizer_free_hook)
       __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);
   }
 
   // Reallocates a chunk. We can save on a new allocation if the new requested
   // 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
     // is reasonable.
     if (NewSize <= UsableSize &&
         (UsableSize - NewSize) < (SizeClassMap::kMaxSize / 2)) {
       UnpackedHeader NewHeader = OldHeader;
       NewHeader.SizeOrUnusedBytes =
struct ScudoAllocator {
   // Helper function that returns the actual usable size of a chunk.
   uptr getUsableSize(const void *Ptr) {
     initThreadMaybe();
     if (UNLIKELY(!Ptr))
       return 0;
     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);
   }
 
   void *calloc(uptr NMemB, uptr Size) {
     initThreadMaybe();
     if (UNLIKELY(CheckForCallocOverflow(NMemB, Size)))
       return FailureHandler::OnBadRequest();
     return allocate(NMemB * Size, MinAlignment, FromMalloc, true);

lib/scudo/scudo_termination.cpp

if (common_flags()->abort_on_error)
     Abort();
   internal__exit(common_flags()->exitcode);
 }
 
 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);
 }
 
 }  // namespace __sanitizer

lib/scudo/scudo_utils.cpp

 extern int VSNPrintf(char *buff, int buff_length, const char *format,
                      va_list args);
 
 }  // namespace __sanitizer
 
 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();
 }
 
 #if defined(__x86_64__) || defined(__i386__)
 // i386 and x86_64 specific code to detect CRC32 hardware support via CPUID.
 // CRC32 requires the SSE 4.2 instruction set.

test/scudo/alignment.c

int main(int argc, char **argv)
   if (!strcmp(argv[1], "pointers")) {
     void *p = malloc(1U << 16);
     assert(p);
     free((void *)((uintptr_t)p | 1));
   }
   return 0;
 }
 
-// CHECK: ERROR: attempted to deallocate a chunk not properly aligned
+// CHECK: ERROR: misaligned pointer when deallocating address

test/scudo/sized-delete.cpp

int main(int argc, char **argv)
   }
   if (!strcmp(argv[1], "baddelarr")) {
     char *p = new char[63];
     operator delete[](p, 64);
   }
   return 0;
 }
 
-// CHECK: ERROR: invalid sized delete on chunk at address
+// CHECK: ERROR: invalid sized delete when deallocating address