[scudo] Tuning changes based on feedback from current use

[cryptoad]

Summary

This tunes several of the default parameters used within the allocator:

• disable the deallocation type mismatch on Android by default; this was causing too many issues with third party libraries;
• change the default SizeClassMap to Dense, it caches less entries and is way more memory efficient overall;
• relax the timing of the RSS checks, 10 times per second was too much, lower it to 4 times (every 250ms), and update the test so that it passes with the new default.

[eugenis]

LGTM

lib/scudo/scudo_allocator.cpp

NOINLINE void Allocator::performSanityChecks() {
 
   const uptr LargestClassId = SizeClassMap::kLargestClassID;
   Header.ClassId = LargestClassId;
   if (Header.ClassId != LargestClassId)
     dieWithMessage("largest class ID doesn't fit in header\n");
 }
 
 // Opportunistic RSS limit check. This will update the RSS limit status, if
-// it can, every 100ms, otherwise it will just return the current one.
+// it can, every 250ms, otherwise it will just return the current one.
 NOINLINE bool Allocator::isRssLimitExceeded() {
   u64 LastCheck = atomic_load_relaxed(&RssLastCheckedAtNS);
   const u64 CurrentCheck = MonotonicNanoTime();
-  if (LIKELY(CurrentCheck < LastCheck + (100ULL * 1000000ULL)))
+  if (LIKELY(CurrentCheck < LastCheck + (250ULL * 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;

lib/scudo/scudo_flags.inc

SCUDO_FLAG(int, ThreadLocalQuarantineSizeKb, -1,
           "the contention on the global quarantine. Defaults to 16KB (32-bit) "
           "or 64KB (64-bit)")
 
 // Default value is set in scudo_flags.cpp based on architecture.
 SCUDO_FLAG(int, QuarantineChunksUpToSize, -1,
           "Size in bytes up to which chunks will be quarantined (if lower than"
           "or equal to). Defaults to 256 (32-bit) or 2048 (64-bit)")
 
-SCUDO_FLAG(bool, DeallocationTypeMismatch, true,
+// Disable the deallocation type check by default on Android, it causes too many
+// issues with third party libraries.
+SCUDO_FLAG(bool, DeallocationTypeMismatch, !SANITIZER_ANDROID,
           "Report errors on malloc/delete, new/free, new/delete[], etc.")
 
 SCUDO_FLAG(bool, DeleteSizeMismatch, true,
            "Report errors on mismatch between size of new and delete.")
 
 SCUDO_FLAG(bool, ZeroContents, false,
           "Zero chunk contents on allocation and deallocation.")

lib/scudo/scudo_platform.h

 # else
 const uptr AllocatorSize = 0x40000000000ULL;  // 4T.
 # endif
 #else
 const uptr RegionSizeLog = SANITIZER_ANDROID ? 19 : 20;
 #endif  // SANITIZER_CAN_USE_ALLOCATOR64
 
 #if !defined(SCUDO_SIZE_CLASS_MAP)
-# define SCUDO_SIZE_CLASS_MAP Default
+# define SCUDO_SIZE_CLASS_MAP Dense
 #endif
 
 #define SIZE_CLASS_MAP_TYPE SIZE_CLASS_MAP_TYPE_(SCUDO_SIZE_CLASS_MAP)
 #define SIZE_CLASS_MAP_TYPE_(T) SIZE_CLASS_MAP_TYPE__(T)
 #define SIZE_CLASS_MAP_TYPE__(T) T##SizeClassMap
 
 typedef SIZE_CLASS_MAP_TYPE SizeClassMap;
 
 }  // namespace __scudo
 
 #endif // SCUDO_PLATFORM_H_

test/scudo/rss.c

 // RUN: %clang_scudo %s -o %t
 // RUN:                                                                                                  %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
-// RUN: %env_scudo_opts="soft_rss_limit_mb=256"                                                          %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
-// RUN: %env_scudo_opts="hard_rss_limit_mb=256"                                                          %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
-// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=0"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
-// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=1"                               %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
-// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
-// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=1:can_use_proc_maps_statm=0"     %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
-// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=0"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
-// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=1"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
-// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
-// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=1:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
+// RUN: %env_scudo_opts="soft_rss_limit_mb=128"                                                          %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
+// RUN: %env_scudo_opts="hard_rss_limit_mb=128"                                                          %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
+// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=0"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
+// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=1"                               %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
+// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
+// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=1:can_use_proc_maps_statm=0"     %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
+// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=0"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
+// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=1"                           not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
+// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
+// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=1:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
 
 // Tests that the soft and hard RSS limits work as intended. Without limit or
 // with a high limit, the test should pass without any malloc returning NULL or
 // the program dying.
 // If a limit is specified, it should return some NULL or die depending on
 // allocator_may_return_null. This should also work without statm.
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
-static const size_t kNumAllocs = 128;
+static const size_t kNumAllocs = 64;
 static const size_t kAllocSize = 1 << 20;  // 1MB.
 
 static void *allocs[kNumAllocs];
 
 int main(int argc, char *argv[]) {
   int returned_null = 0;
   for (int i = 0; i < kNumAllocs; i++) {
     if ((i & 0xf) == 0)
-      usleep(50000);
+      usleep(100000);
     allocs[i] = malloc(kAllocSize);
     if (allocs[i])
       memset(allocs[i], 0xff, kAllocSize);  // Dirty the pages.
     else
       returned_null++;
   }
   for (int i = 0; i < kNumAllocs; i++)
     free(allocs[i]);