Add Soft/Hard RSS Limits to Scudo Standalone

[mcgrathr]

This needs a stub GetRSS for Fuchsia, at least.
Since there's no way to get a useful value for this on Fuchsia as yet, it would be preferable to conditionalize the machinery so it isn't doing no-op work.

[bsdaemon]

The feature right now is only supported on LINUX. isRssLimitExceeded() is protected by #if and inlined in any non-linux system and a generic GetRSS symbol is exported in any non-Linux systems (so even with LD_BIND_NOW the symbol exists/resolves). GetRSS() never gets called since it is only called by isRssLimitExceeded() and by a flow conditioned in the isRssLimitExceeded result.

With optimizations on, even the references to the functions and the entire conditional block get removed.

[phosek]

Can you reupload the patch with full context?

compiler-rt/lib/scudo/standalone/linux.cpp

185 ↗

(On Diff #435038)

The opening brace should be on the previous line.

190 ↗

(On Diff #435038)

The opening brace should be on the previous line.

195 ↗

(On Diff #435038)

The opening brace should be on the previous line.

compiler-rt/lib/scudo/standalone/report.cpp

40

The opening brace should be on the previous line.

[bsdaemon]

Fixed the few style errors pointed out (thank you!) and submitting a full context patch.

[bsdaemon]

Fixed the few style errors pointed out (thank you!) and submitting a full context patch.

[bsdaemon]

Fixed compiler warnings due to implicit type conversions, got more pedantic on using the right types (even for compatible ones).

[bsdaemon]

This comment was removed by bsdaemon.

[bsdaemon]

Added LLVM code-style fixes

[mcgrathr]

compiler-rt/lib/scudo/standalone/atomic_helpers.h

127 ↗

(On Diff #440238)

This should use the call above with true instead of false for the bool argument.

compiler-rt/lib/scudo/standalone/combined.h

353

excess space between parens

732

It's better not to use ALWAYS_INLINE and leave it to the compiler to decide (which it will).
Provide the stub GetRSS function even when you expect calls to it to be optimized away.

736

Use false, not 0.

1021

Put spaces around the =.

[vitalybuka]

compiler-rt/lib/scudo/standalone/atomic_helpers.h

127 ↗

(On Diff #440238)

please move this into a separate patch and extend compiler-rt/lib/scudo/standalone/tests/atomic_test.cpp

for context, you can organize patches into "stack" using "edit related revision" in the Phabricator menu.

compiler-rt/lib/scudo/standalone/combined.h

353

just clangformat entire patch

357

static_cast

compiler-rt/lib/scudo/standalone/flags.inc

49

limits need own tests as well

compiler-rt/lib/scudo/standalone/linux.cpp

196 ↗

(On Diff #440238)

How about moving GetRSS() (with a test) implementation into a separate patch?

[bsdaemon]

Bastian Kersting started splitting the patch and making the suggested changes. The first in the series is here: https://reviews.llvm.org/D138406 - I appreciate if folks here help reviewing/merging.

[vitalybuka]

Can you please rebase this one?

compiler-rt/lib/scudo/standalone/combined.h

703

can you measure a meaningful difference or real apps between weak vs strong here?

[1c3t3a]

Rebase on commit chain and latest main.

[1c3t3a]

Add requested changes and tests for Hard and Soft RSS limits

[1c3t3a]

compiler-rt/lib/scudo/standalone/combined.h

703

I benchmarked this method and found not a big difference but a difference between weak and strong here. I benchmarked this on an ARM (aarch64) machine with the following findings:

2022-12-15T09:02:48-08:00
Running third_party/llvm/llvm-project/compiler-rt/scudo_benchmarks
Run on (64 X 3000 MHz CPU s)
CPU Caches:
  L1 Data 64 KiB (x64)
  L1 Instruction 64 KiB (x64)
  L2 Unified 1024 KiB (x64)
  L3 Unified 32768 KiB (x1)
Load Average: 4.02, 9.90, 14.71
-----------------------------------------------------------------
Benchmark                       Time             CPU   Iterations
-----------------------------------------------------------------
BM_IsRssExceeded             47.6 ns         47.5 ns     14772023
BM_IsRssExceededStrong       47.8 ns         47.7 ns     14673854

I get that this is not a big difference, but I would still argue that it is enough to use weak here. We could introduce a compilation parameter with the default weak.

[vitalybuka]

compiler-rt/lib/scudo/standalone/combined.h

171

zero initialized value will work the same

694

Probably nicer to put this method and related fields into another class, e.g

class RssLimitChecker {
 atomic_u64 RssLastCheckedAtNS = {};
 atomic_u8 RssLimitExceeded = {};

  // Slow part in cpp file
  bool Check();

public:
 // Inline fast path.
 bool isExceeded() {
    u64 LastCheck = atomic_load_relaxed(&RssLastCheckedAtNS);
    const u64 CurrentCheck = getMonotonicTime();
     if (UNLIKELY(CurrentCheck >= LastCheck + (250ULL * 1000000ULL)))
       Check();
     return atomic_load_relaxed(&RssLimitExceeded);

 }
 
}

700

instead of adding interval here,
add it when you store it in compare_exchange
then it should be named LastCheck -> NextCheck

CurrentCheck could be named e.g. "Now".

703

Then please just switch to strong

718–725

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

741

This relies on the order of the test execution
everything after this will see scudo::getFlags()->soft_rss_limit_mb = 1

for avoid this the test should backup and restore flags

757

this code does not use {} for one line loop bodies

764

restore flags

[1c3t3a]

Add requested changes

[vitalybuka]

This looks good to me with nits

compiler-rt/lib/scudo/standalone/combined.h

169

remove {}

353–354

redundant ()

compiler-rt/lib/scudo/standalone/common.h

222 ↗

(On Diff #484551)

atomic_compare_exchange_strong is in UNLIKELY branch already
sink it into the Check() to avoid unnecessary inlining it

240 ↗

(On Diff #484551)

Remove redundant ULL as result fits into int32
Remove redundant ()

compiler-rt/lib/scudo/standalone/linux.cpp

230 ↗

(On Diff #484551)

remove {}

231 ↗

(On Diff #484551)

maybe nicer to RssLimitExceeded -> enum {Neither, Soft, Hard}
and move reportRSSCheck() same place where you report soft limit

[vitalybuka]

you don't need getOptionsForTesting if you can run init after updating flags

compiler-rt/lib/scudo/standalone/combined.h

169

get from RssChecker

358

please add and use RssChecker.getSoftRssLimit() RssChecker.getHardRssLimit()
So we have not concerns if IsExceeded and this message use the same value.

compiler-rt/lib/scudo/standalone/linux.cpp

217 ↗

(On Diff #484551)

Return here if they are both zeroes and eliminate CheckRssLimit?

[1c3t3a]

Add requested changes and reorganize tests

[1c3t3a]

I also restructured the tests to get rid of getOptionsForTesting and the flags since this is now read at the creation of the RssChecker.

compiler-rt/lib/scudo/standalone/common.h

222 ↗

(On Diff #484551)

That would mean Check needs the NextCheck variable for context and that would become a parameter of Check right?

compiler-rt/lib/scudo/standalone/linux.cpp

217 ↗

(On Diff #484551)

Done. What do you mean by "eliminate CheckRssLimit"?

[1c3t3a]

Remove unused headers

[1c3t3a]

Fix init of limits

[vitalybuka]

update

[vitalybuka]

rebase

compiler-rt/lib/scudo/standalone/CMakeLists.txt

set(SCUDO_HEADERS
   mutex.h
   options.h
   platform.h
   primary32.h
   primary64.h
   quarantine.h
   release.h
   report.h
+  rss_limit_checker.h
   secondary.h
   size_class_map.h
   stack_depot.h
   stats.h
   string_utils.h
   tsd_exclusive.h
   tsd_shared.h
   tsd.h
set(SCUDO_SOURCES
   common.cpp
   crc32_hw.cpp
   flags_parser.cpp
   flags.cpp
   fuchsia.cpp
   linux.cpp
   release.cpp
   report.cpp
+  rss_limit_checker.cpp
   string_utils.cpp
   )
 
 # Enable the necessary instruction set for scudo_crc32.cpp, if available.
 # Newer compiler versions use -mcrc32 rather than -msse4.2.
 if (COMPILER_RT_HAS_MCRC32_FLAG)
   set_source_files_properties(crc32_hw.cpp PROPERTIES COMPILE_FLAGS -mcrc32)
 elseif (COMPILER_RT_HAS_MSSE4_2_FLAG)

compiler-rt/lib/scudo/standalone/combined.h

 #include "common.h"
 #include "flags.h"
 #include "flags_parser.h"
 #include "local_cache.h"
 #include "memtag.h"
 #include "options.h"
 #include "quarantine.h"
 #include "report.h"
+#include "rss_limit_checker.h"
 #include "secondary.h"
 #include "stack_depot.h"
 #include "string_utils.h"
 #include "tsd.h"
 
 #include "scudo/interface.h"
 
 #ifdef GWP_ASAN_HOOKS
void init() {
 
     if (UNLIKELY(!getRandom(&Cookie, sizeof(Cookie))))
       Cookie = static_cast<u32>(getMonotonicTime() ^
                                 (reinterpret_cast<uptr>(this) >> 4));
 
     initFlags();
     reportUnrecognizedFlags();
 
+    RssChecker.init(scudo::getFlags()->soft_rss_limit_mb,
+                    scudo::getFlags()->hard_rss_limit_mb);
+
     // Store some flags locally.
     if (getFlags()->may_return_null)
       Primary.Options.set(OptionBit::MayReturnNull);
     if (getFlags()->zero_contents)
       Primary.Options.setFillContentsMode(ZeroFill);
     else if (getFlags()->pattern_fill_contents)
       Primary.Options.setFillContentsMode(PatternOrZeroFill);
     if (getFlags()->dealloc_type_mismatch)
       Primary.Options.set(OptionBit::DeallocTypeMismatch);
     if (getFlags()->delete_size_mismatch)
       Primary.Options.set(OptionBit::DeleteSizeMismatch);
     if (allocatorSupportsMemoryTagging<Params>() &&
         systemSupportsMemoryTagging())
       Primary.Options.set(OptionBit::UseMemoryTagging);
     Primary.Options.set(OptionBit::UseOddEvenTags);
 
     QuarantineMaxChunkSize =
         static_cast<u32>(getFlags()->quarantine_max_chunk_size);
 
     Stats.init();
     const s32 ReleaseToOsIntervalMs = getFlags()->release_to_os_interval_ms;
     Primary.init(ReleaseToOsIntervalMs);
     Secondary.init(&Stats, ReleaseToOsIntervalMs);
     Quarantine.init(
         static_cast<uptr>(getFlags()->quarantine_size_kb << 10),
         static_cast<uptr>(getFlags()->thread_local_quarantine_size_kb << 10));
#endif // GWP_ASAN_HOOKS
     static_assert(MaxAllowedMallocSize < UINTPTR_MAX - MaxAlignment, "");
     if (UNLIKELY(Size >= MaxAllowedMallocSize)) {
       if (Options.get(OptionBit::MayReturnNull))
         return nullptr;
       reportAllocationSizeTooBig(Size, NeededSize, MaxAllowedMallocSize);
     }
     DCHECK_LE(Size, NeededSize);
 
+    switch (RssChecker.getRssLimitExceeded()) {
+    case RssLimitChecker::Neither:
+      break;
+    case RssLimitChecker::Soft:
+      if (Options.get(OptionBit::MayReturnNull))
+        return nullptr;
+      reportSoftRSSLimit(RssChecker.getSoftRssLimit());
+      break;
+    case RssLimitChecker::Hard:
+      reportHardRSSLimit(RssChecker.getHardRssLimit());
+      break;
+    }
+
     void *Block = nullptr;
     uptr ClassId = 0;
     uptr SecondaryBlockEnd = 0;
     if (LIKELY(PrimaryT::canAllocate(NeededSize))) {
       ClassId = SizeClassMap::getClassIdBySize(NeededSize);
       DCHECK_NE(ClassId, 0U);
       bool UnlockRequired;
       auto *TSD = TSDRegistry.getTSDAndLock(&UnlockRequired);
if (LIKELY(NewPtr)) {
       quarantineOrDeallocateChunk(Options, OldTaggedPtr, &OldHeader, OldSize);
     }
     return NewPtr;
   }
 
   // TODO(kostyak): disable() is currently best-effort. There are some small
   //                windows of time when an allocation could still succeed after
   //                this function finishes. We will revisit that later.
   void disable() {
     initThreadMaybe();
 #ifdef GWP_ASAN_HOOKS
     GuardedAlloc.disable();
 #endif
     TSDRegistry.disable();
     Stats.disable();
     Quarantine.disable();
     Primary.disable();
     Secondary.disable();
   }
 
   void enable() {
     initThreadMaybe();
     Secondary.enable();
     Primary.enable();
     Quarantine.enable();
     Stats.enable();
     TSDRegistry.enable();
 #ifdef GWP_ASAN_HOOKS
     GuardedAlloc.enable();
 #endif
   }
 
   // The function returns the amount of bytes required to store the statistics,
   // which might be larger than the amount of bytes provided. Note that the
   // statistics buffer is not necessarily constant between calls to this
   // function. This can be called with a null buffer or zero size for buffer
   // sizing purposes.
   uptr getStats(char *Buffer, uptr Size) {
     ScopedString Str;
     disable();
     const uptr Length = getStats(&Str) + 1;
     enable();
     if (Length < Size)
       Size = Length;
     if (Buffer && Size) {
       memcpy(Buffer, Str.data(), Size);
       Buffer[Size - 1] = '\0';
     }
     return Length;
   }
 
   void printStats() {
     ScopedString Str;
     disable();
     getStats(&Str);
     enable();
     Str.output();
   }
 
#endif // GWP_ASAN_HOOKS
     if (!Ptr || !isAligned(reinterpret_cast<uptr>(Ptr), MinAlignment))
       return false;
     Ptr = getHeaderTaggedPointer(const_cast<void *>(Ptr));
     Chunk::UnpackedHeader Header;
     return Chunk::isValid(Cookie, Ptr, &Header) &&
            Header.State == Chunk::State::Allocated;
   }
 
+  void setRssLimitsTestOnly(int SoftRssLimitMb, int HardRssLimitMb,
+                            bool MayReturnNull) {
+    RssChecker.init(SoftRssLimitMb, HardRssLimitMb);
+    if (MayReturnNull)
+      Primary.Options.set(OptionBit::MayReturnNull);
+  }
+
   bool useMemoryTaggingTestOnly() const {
     return useMemoryTagging<Params>(Primary.Options.load());
   }
   void disableMemoryTagging() {
     // If we haven't been initialized yet, we need to initialize now in order to
     // prevent a future call to initThreadMaybe() from enabling memory tagging
     // based on feature detection. But don't call initThreadMaybe() because it
     // may end up calling the allocator (via pthread_atfork, via the post-init
private:
   u32 QuarantineMaxChunkSize = 0;
 
   GlobalStats Stats;
   PrimaryT Primary;
   SecondaryT Secondary;
   QuarantineT Quarantine;
   TSDRegistryT TSDRegistry;
   pthread_once_t PostInitNonce = PTHREAD_ONCE_INIT;
+  RssLimitChecker RssChecker;
 
 #ifdef GWP_ASAN_HOOKS
   gwp_asan::GuardedPoolAllocator GuardedAlloc;
   uptr GuardedAllocSlotSize = 0;
 #endif // GWP_ASAN_HOOKS
 
   StackDepot Depot;
 

compiler-rt/lib/scudo/standalone/flags.inc

 SCUDO_FLAG(bool, may_return_null, true,
            "Indicate whether the allocator should terminate instead of "
            "returning NULL in otherwise non-fatal error scenarios, eg: OOM, "
            "invalid allocation alignments, etc.")
 
 SCUDO_FLAG(int, release_to_os_interval_ms, SCUDO_ANDROID ? INT32_MIN : 5000,
            "Interval (in milliseconds) at which to attempt release of unused "
            "memory to the OS. Negative values disable the feature.")
+
+SCUDO_FLAG(int, hard_rss_limit_mb, 0,
+           "Hard RSS Limit in Mb. If non-zero, once the limit is achieved, "
+           "abort the process")
+
+SCUDO_FLAG(int, soft_rss_limit_mb, 0,
+           "Soft RSS Limit in Mb. If non-zero, once the limit is reached, all "
+           "subsequent calls will fail or return NULL until the RSS goes below "
+           "the soft limit")

compiler-rt/lib/scudo/standalone/report.h

 // Sanity checks related error.
 void NORETURN reportSanityCheckError(const char *Field);
 
 // Combined allocator errors.
 void NORETURN reportAlignmentTooBig(uptr Alignment, uptr MaxAlignment);
 void NORETURN reportAllocationSizeTooBig(uptr UserSize, uptr TotalSize,
                                          uptr MaxSize);
 void NORETURN reportOutOfMemory(uptr RequestedSize);
+void NORETURN reportSoftRSSLimit(uptr RssLimitMb);
+void NORETURN reportHardRSSLimit(uptr RssLimitMb);
 enum class AllocatorAction : u8 {
   Recycling,
   Deallocating,
   Reallocating,
   Sizing,
 };
 void NORETURN reportInvalidChunkState(AllocatorAction Action, void *Ptr);
 void NORETURN reportMisalignedPointer(AllocatorAction Action, void *Ptr);

compiler-rt/lib/scudo/standalone/report.cpp

public:
   }
 
 private:
   ScopedString Message;
 };
 
 inline void NORETURN trap() { __builtin_trap(); }
 
+void NORETURN reportSoftRSSLimit(uptr RssLimitMb) {
+  ScopedErrorReport Report;
+  Report.append("Soft RSS limit of %zu MB exhausted, current RSS is %zu MB\n",
+                RssLimitMb, GetRSS() >> 20);
+}
+
+void NORETURN reportHardRSSLimit(uptr RssLimitMb) {
+  ScopedErrorReport Report;
+  Report.append("Hard RSS limit of %zu MB exhausted, current RSS is %zu MB\n",
+                RssLimitMb, GetRSS() >> 20);
+}
+
 // This could potentially be called recursively if a CHECK fails in the reports.
 void NORETURN reportCheckFailed(const char *File, int Line,
                                 const char *Condition, u64 Value1, u64 Value2) {
   static atomic_u32 NumberOfCalls;
   if (atomic_fetch_add(&NumberOfCalls, 1, memory_order_relaxed) > 2) {
     // TODO(kostyak): maybe sleep here?
     trap();
   }

compiler-rt/lib/scudo/standalone/rss_limit_checker.h

+//===-- common.h ------------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SCUDO_RSS_LIMIT_CHECKER_H_
+#define SCUDO_RSS_LIMIT_CHECKER_H_
+
+#include "atomic_helpers.h"
+#include "common.h"
+#include "internal_defs.h"
+
+namespace scudo {
+
+class RssLimitChecker {
+public:
+  enum RssLimitExceeded {
+    Neither,
+    Soft,
+    Hard,
+  };
+
+  void init(int SoftRssLimitMb, int HardRssLimitMb) {
+    CHECK_GE(SoftRssLimitMb, 0);
+    CHECK_GE(HardRssLimitMb, 0);
+    this->SoftRssLimitMb = static_cast<uptr>(SoftRssLimitMb);
+    this->HardRssLimitMb = static_cast<uptr>(HardRssLimitMb);
+  }
+
+  // Opportunistic RSS limit check. This will update the RSS limit status, if
+  // it can, every 250ms, otherwise it will just return the current one.
+  RssLimitExceeded getRssLimitExceeded() {
+    if (!HardRssLimitMb && !SoftRssLimitMb)
+      return RssLimitExceeded::Neither;
+
+    u64 NextCheck = atomic_load_relaxed(&RssNextCheckAtNS);
+    u64 Now = getMonotonicTime();
+
+    if (UNLIKELY(Now >= NextCheck))
+      check(NextCheck);
+
+    return static_cast<RssLimitExceeded>(atomic_load_relaxed(&RssLimitStatus));
+  }
+
+  uptr getSoftRssLimit() const { return SoftRssLimitMb; }
+  uptr getHardRssLimit() const { return HardRssLimitMb; }
+
+private:
+  void check(u64 NextCheck);
+
+  uptr SoftRssLimitMb = 0;
+  uptr HardRssLimitMb = 0;
+
+  atomic_u64 RssNextCheckAtNS = {};
+  atomic_u8 RssLimitStatus = {};
+};
+
+} // namespace scudo
+
+#endif // SCUDO_RSS_LIMIT_CHECKER_H_

compiler-rt/lib/scudo/standalone/rss_limit_checker.cpp

+//===-- common.cpp ----------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "rss_limit_checker.h"
+#include "atomic_helpers.h"
+#include "string_utils.h"
+
+namespace scudo {
+
+void RssLimitChecker::check(u64 NextCheck) {
+  // The interval for the checks is 250ms.
+  static constexpr u64 CheckInterval = 250 * 1000000;
+
+  // Early return in case another thread already did the calculation.
+  if (!atomic_compare_exchange_strong(&RssNextCheckAtNS, &NextCheck,
+                                      getMonotonicTime() + CheckInterval,
+                                      memory_order_relaxed)) {
+    return;
+  }
+
+  const uptr CurrentRssMb = GetRSS() >> 20;
+
+  RssLimitExceeded Result = RssLimitExceeded::Neither;
+  if (UNLIKELY(HardRssLimitMb && HardRssLimitMb < CurrentRssMb))
+    Result = RssLimitExceeded::Hard;
+  else if (UNLIKELY(SoftRssLimitMb && SoftRssLimitMb < CurrentRssMb))
+    Result = RssLimitExceeded::Soft;
+
+  atomic_store_relaxed(&RssLimitStatus, static_cast<u8>(Result));
+}
+
+} // namespace scudo

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

TEST(ScudoCombinedTest, BasicTrustyConfig) {
   auto *TSD = Allocator->getTSDRegistry()->getTSDAndLock(&UnlockRequired);
   TSD->Cache.drain();
 
   Allocator->releaseToOS();
 }
 
 #endif
 #endif
+
+#if SCUDO_LINUX
+
+SCUDO_TYPED_TEST(ScudoCombinedTest, SoftRssLimit) {
+  auto *Allocator = this->Allocator.get();
+  Allocator->setRssLimitsTestOnly(1, 0, true);
+
+  size_t Megabyte = 1024 * 1024;
+  size_t ChunkSize = 16;
+  size_t Error = 256;
+
+  std::vector<void *> Ptrs;
+  for (size_t index = 0; index < Megabyte + Error; index += ChunkSize) {
+    void *Ptr = Allocator->allocate(ChunkSize, Origin);
+    Ptrs.push_back(Ptr);
+  }
+
+  EXPECT_EQ(nullptr, Allocator->allocate(ChunkSize, Origin));
+
+  for (void *Ptr : Ptrs)
+    Allocator->deallocate(Ptr, Origin);
+}
+
+SCUDO_TYPED_TEST(ScudoCombinedTest, HardRssLimit) {
+  auto *Allocator = this->Allocator.get();
+  Allocator->setRssLimitsTestOnly(0, 1, false);
+
+  size_t Megabyte = 1024 * 1024;
+
+  EXPECT_DEATH(
+      {
+        disableDebuggerdMaybe();
+        Allocator->allocate(Megabyte, Origin);
+      },
+      "");
+}
+
+#endif