[compiler-rt] [scudo] Update error handling for cache entry count in secondary cach… (PR #95595)

Joshua Baehring via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 14 15:02:30 PDT 2024


https://github.com/JoshuaMBa updated https://github.com/llvm/llvm-project/pull/95595

>From 94531fd182ca6fa4621b2dc5a55c8c3da70250a6 Mon Sep 17 00:00:00 2001
From: Joshua Baehring <jmbaehring at google.com>
Date: Fri, 14 Jun 2024 19:34:25 +0000
Subject: [PATCH 1/2] [scudo] Update error handling for cache entry count in
 secondary cache option handler.

Initially, the scudo allocator would return an error if the user attempted to set the cache
capacity (i.e. the number of possible entries in the cache) above the maximum cache capacity.
Now the allocator will resort to using the maximum cache capacity in this event. An
error will still be returned if the user attempts to set the number of entries to a
negative value.
---
 compiler-rt/lib/scudo/standalone/secondary.h              | 8 ++++----
 compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index d8c9f5bcfcaf6..0e0788f830431 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -391,10 +391,10 @@ template <typename Config> class MapAllocatorCache {
       return true;
     }
     if (O == Option::MaxCacheEntriesCount) {
-      const u32 MaxCount = static_cast<u32>(Value);
-      if (MaxCount > Config::getEntriesArraySize())
-        return false;
-      atomic_store_relaxed(&MaxEntriesCount, MaxCount);
+      if (Value < 0) return false;
+      atomic_store_relaxed(
+          &MaxEntriesCount,
+          Min<u32>(static_cast<u32>(Value), Config::getEntriesArraySize()));
       return true;
     }
     if (O == Option::MaxCacheEntrySize) {
diff --git a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
index 8f0250e88ebf3..af69313214ea6 100644
--- a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
@@ -192,9 +192,9 @@ TEST_F(MapAllocatorTest, SecondaryIterate) {
 
 TEST_F(MapAllocatorTest, SecondaryOptions) {
   // Attempt to set a maximum number of entries higher than the array size.
-  EXPECT_FALSE(
-      Allocator->setOption(scudo::Option::MaxCacheEntriesCount, 4096U));
-  // A negative number will be cast to a scudo::u32, and fail.
+  EXPECT_TRUE(Allocator->setOption(scudo::Option::MaxCacheEntriesCount, 4096U));
+
+  // Attempt to set an invalid (negative) number of entries
   EXPECT_FALSE(Allocator->setOption(scudo::Option::MaxCacheEntriesCount, -1));
   if (Allocator->canCache(0U)) {
     // Various valid combinations.

>From b4d5564d0e63d2a9e991dc07bae973dab4a222b6 Mon Sep 17 00:00:00 2001
From: Joshua Baehring <jmbaehring at google.com>
Date: Fri, 14 Jun 2024 21:50:30 +0000
Subject: [PATCH 2/2] Ran clang-format

---
 compiler-rt/lib/scudo/standalone/secondary.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index 0e0788f830431..9a8e53be388b7 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -391,7 +391,8 @@ template <typename Config> class MapAllocatorCache {
       return true;
     }
     if (O == Option::MaxCacheEntriesCount) {
-      if (Value < 0) return false;
+      if (Value < 0)
+        return false;
       atomic_store_relaxed(
           &MaxEntriesCount,
           Min<u32>(static_cast<u32>(Value), Config::getEntriesArraySize()));



More information about the llvm-commits mailing list