Release memory to OS only when the requested range covers the entire page

[alekseyshl]

Summary

The current code was sometimes attempting to release huge chunks of
memory due to undesired RoundUp/RoundDown interaction when the requested
range is fully contained within one memory page.

[eugenis]

Nice.
I understand this is never a problem in practice, because the kernel would never release a region of size (uptr)(-4096).

lib/sanitizer_common/sanitizer_posix_libcdep.cc

59 ↗

(On Diff #79645)

Is this function still used?

[alekseyshl]

It is not _really_a problem, but making pontless call, the one we know is going to fail, is not a good idea to me.

lib/sanitizer_common/sanitizer_posix_libcdep.cc

59 ↗

(On Diff #79645)

Yes, it is, tsan has a bit more elaborated logic and I decided not to touch it, at least in this patch.

[eugenis]

I just don't like that now we have two functions doing the same thing, one takes (start, end) pointers, the other - (start, size), and the difference in function names (the word "Pages") has nothing to do with the difference in their behavior.

How about we add the rounding logic to ReleaseMemoryToOS() and remove it from the call sites?

[alekseyshl]

• Remove unused ReleaseMemoryToOS

[alekseyshl]

Good point, removed it.

[eugenis]

LGTM

compiler-rt/trunk/lib/asan/asan_poisoning.h

if (i + SHADOW_GRANULARITY <= size) {
       *shadow = (SHADOW_GRANULARITY == 128) ? 0xff : value;  // unaddressable
     } else {
       // first size-i bytes are addressable
       *shadow = poison_partial ? static_cast<u8>(size - i) : 0;
     }
   }
 }
 
-// Calls __sanitizer::ReleaseMemoryToOS() on
-// [MemToShadow(p), MemToShadow(p+size)] with proper rounding.
+// Calls __sanitizer::ReleaseMemoryPagesToOS() on
+// [MemToShadow(p), MemToShadow(p+size)].
 void FlushUnneededASanShadowMemory(uptr p, uptr size);
 
 }  // namespace __asan

compiler-rt/trunk/lib/asan/asan_poisoning.cc

struct ShadowSegmentEndpoint {
   explicit ShadowSegmentEndpoint(uptr address) {
     chunk = (u8*)MemToShadow(address);
     offset = address & (SHADOW_GRANULARITY - 1);
     value = *chunk;
   }
 };
 
 void FlushUnneededASanShadowMemory(uptr p, uptr size) {
-    // Since asan's mapping is compacting, the shadow chunk may be
-    // not page-aligned, so we only flush the page-aligned portion.
-    uptr page_size = GetPageSizeCached();
-    uptr shadow_beg = RoundUpTo(MemToShadow(p), page_size);
-    uptr shadow_end = RoundDownTo(MemToShadow(p + size), page_size);
-    ReleaseMemoryToOS(shadow_beg, shadow_end - shadow_beg);
+  ReleaseMemoryPagesToOS(MemToShadow(p), MemToShadow(p + size));
 }
 
 void AsanPoisonOrUnpoisonIntraObjectRedzone(uptr ptr, uptr size, bool poison) {
   uptr end = ptr + size;
   if (Verbosity()) {
     Printf("__asan_%spoison_intra_object_redzone [%p,%p) %zd\n",
            poison ? "" : "un", ptr, end, size);
     if (Verbosity() >= 2)

compiler-rt/trunk/lib/msan/msan_allocator.cc

 
 struct MsanMapUnmapCallback {
   void OnMap(uptr p, uptr size) const {}
   void OnUnmap(uptr p, uptr size) const {
     __msan_unpoison((void *)p, size);
 
     // We are about to unmap a chunk of user memory.
     // Mark the corresponding shadow memory as not needed.
-    ReleaseMemoryToOS(MEM_TO_SHADOW(p), size);
-    if (__msan_get_track_origins())
-      ReleaseMemoryToOS(MEM_TO_ORIGIN(p), size);
+    uptr shadow_p = MEM_TO_SHADOW(p);
+    ReleaseMemoryPagesToOS(shadow_p, shadow_p + size);
+    if (__msan_get_track_origins()) {
+      uptr origin_p = MEM_TO_ORIGIN(p);
+      ReleaseMemoryPagesToOS(origin_p, origin_p + size);
+    }
   }
 };
 
 #if defined(__mips64)
   static const uptr kMaxAllowedMallocSize = 2UL << 30;
   static const uptr kRegionSizeLog = 20;
   static const uptr kNumRegions = SANITIZER_MMAP_RANGE_SIZE >> kRegionSizeLog;
   typedef TwoLevelByteMap<(kNumRegions >> 12), 1 << 12> ByteMap;

compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_primary64.h

if (region->mapped_user + region->mapped_meta >
         kRegionSize - kFreeArraySize) {
       Printf("%s: Out of memory. Dying. ", SanitizerToolName);
       Printf("The process has exhausted %zuMB for size class %zu.\n",
           kRegionSize / 1024 / 1024, size);
       Die();
     }
   }
 
-  bool MaybeReleaseChunkRange(uptr region_beg, uptr chunk_size,
+  void MaybeReleaseChunkRange(uptr region_beg, uptr chunk_size,
                               CompactPtrT first, CompactPtrT last) {
     uptr beg_ptr = CompactPtrToPointer(region_beg, first);
     uptr end_ptr = CompactPtrToPointer(region_beg, last) + chunk_size;
-    const uptr page_size = GetPageSizeCached();
-    CHECK_GE(end_ptr - beg_ptr, page_size);
-    beg_ptr = RoundUpTo(beg_ptr, page_size);
-    end_ptr = RoundDownTo(end_ptr, page_size);
-    if (end_ptr == beg_ptr) return false;
-    ReleaseMemoryToOS(beg_ptr, end_ptr - beg_ptr);
-    return true;
+    ReleaseMemoryPagesToOS(beg_ptr, end_ptr);
   }
 
   // Attempts to release some RAM back to OS. The region is expected to be
   // locked.
   // Algorithm:
   // * Sort the chunks.
   // * Find ranges fully covered by free-d chunks
   // * Release them to OS with madvise.

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

 bool MprotectNoAccess(uptr addr, uptr size);
 bool MprotectReadOnly(uptr addr, uptr size);
 
 // Find an available address space.
 uptr FindAvailableMemoryRange(uptr size, uptr alignment, uptr left_padding);
 
 // Used to check if we can map shadow memory to a fixed location.
 bool MemoryRangeIsAvailable(uptr range_start, uptr range_end);
-void ReleaseMemoryToOS(uptr addr, uptr size);
+// Releases memory pages entirely within the [beg, end] address range. Noop if
+// the provided range does not contain at least one entire page.
+void ReleaseMemoryPagesToOS(uptr beg, uptr end);
 void IncreaseTotalMmap(uptr size);
 void DecreaseTotalMmap(uptr size);
 uptr GetRSS();
 void NoHugePagesInRegion(uptr addr, uptr length);
 void DontDumpShadowMemory(uptr addr, uptr length);
 // Check if the built VMA size matches the runtime one.
 void CheckVMASize();
 void RunMallocHooks(const void *ptr, uptr size);

compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc

 u32 GetUid() {
   return getuid();
 }
 
 uptr GetThreadSelf() {
   return (uptr)pthread_self();
 }
 
-void ReleaseMemoryToOS(uptr addr, uptr size) {
-  madvise((void*)addr, size, MADV_DONTNEED);
+void ReleaseMemoryPagesToOS(uptr beg, uptr end) {
+  uptr page_size = GetPageSizeCached();
+  uptr beg_aligned = RoundUpTo(beg, page_size);
+  uptr end_aligned = RoundDownTo(end, page_size);
+  if (beg_aligned < end_aligned)
+    madvise((void*)beg_aligned, end_aligned - beg_aligned, MADV_DONTNEED);
 }
 
 void NoHugePagesInRegion(uptr addr, uptr size) {
 #ifdef MADV_NOHUGEPAGE  // May not be defined on old systems.
   madvise((void *)addr, size, MADV_NOHUGEPAGE);
 #endif  // MADV_NOHUGEPAGE
 }
 

compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

void *MmapNoAccess(uptr size) {
   return res;
 }
 
 bool MprotectNoAccess(uptr addr, uptr size) {
   DWORD old_protection;
   return VirtualProtect((LPVOID)addr, size, PAGE_NOACCESS, &old_protection);
 }
 
-
-void ReleaseMemoryToOS(uptr addr, uptr size) {
+void ReleaseMemoryPagesToOS(uptr beg, uptr end) {
   // This is almost useless on 32-bits.
   // FIXME: add madvise-analog when we move to 64-bits.
 }
 
 void NoHugePagesInRegion(uptr addr, uptr size) {
   // FIXME: probably similar to ReleaseMemoryToOS.
 }
 

compiler-rt/trunk/lib/tsan/rtl/tsan_mman.cc

void OnUnmap(uptr p, uptr size) const {
     uptr diff = RoundUp(p, kPageSize) - p;
     if (diff != 0) {
       p += diff;
       size -= diff;
     }
     diff = p + size - RoundDown(p + size, kPageSize);
     if (diff != 0)
       size -= diff;
-    ReleaseMemoryToOS((uptr)MemToMeta(p), size / kMetaRatio);
+    uptr p_meta = (uptr)MemToMeta(p);
+    ReleaseMemoryPagesToOS(p_meta, p_meta + size / kMetaRatio);
   }
 };
 
 static char allocator_placeholder[sizeof(Allocator)] ALIGNED(64);
 Allocator *allocator() {
   return reinterpret_cast<Allocator*>(&allocator_placeholder);
 }
 

compiler-rt/trunk/lib/tsan/rtl/tsan_platform_linux.cc

internal_snprintf(buf, buf_size,
       stacks->allocated >> 20, stacks->n_uniq_ids,
       nlive, nthread);
 }
 
 #if SANITIZER_LINUX
 void FlushShadowMemoryCallback(
     const SuspendedThreadsList &suspended_threads_list,
     void *argument) {
-  ReleaseMemoryToOS(ShadowBeg(), ShadowEnd() - ShadowBeg());
+  ReleaseMemoryPagesToOS(ShadowBeg(), ShadowEnd());
 }
 #endif
 
 void FlushShadowMemory() {
 #if SANITIZER_LINUX
   StopTheWorld(FlushShadowMemoryCallback, 0);
 #endif
 }

compiler-rt/trunk/lib/tsan/rtl/tsan_rtl.cc

static void StopBackgroundThread() {
   atomic_store(&ctx->stop_background_thread, 1, memory_order_relaxed);
   internal_join_thread(ctx->background_thread);
   ctx->background_thread = 0;
 }
 #endif
 #endif
 
 void DontNeedShadowFor(uptr addr, uptr size) {
-  uptr shadow_beg = MemToShadow(addr);
-  uptr shadow_end = MemToShadow(addr + size);
-  ReleaseMemoryToOS(shadow_beg, shadow_end - shadow_beg);
+  ReleaseMemoryPagesToOS(MemToShadow(addr), MemToShadow(addr + size));
 }
 
 void MapShadow(uptr addr, uptr size) {
   // Global data is not 64K aligned, but there are no adjacent mappings,
   // so we can get away with unaligned mapping.
   // CHECK_EQ(addr, addr & ~((64 << 10) - 1));  // windows wants 64K alignment
   const uptr kPageSize = GetPageSizeCached();
   uptr shadow_begin = RoundDownTo((uptr)MemToShadow(addr), kPageSize);

compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_thread.cc

void ThreadContext::OnCreated(void *arg) {
   ReleaseImpl(args->thr, 0, &sync);
   creation_stack_id = CurrentStackId(args->thr, args->pc);
   if (reuse_count == 0)
     StatInc(args->thr, StatThreadMaxTid);
 }
 
 void ThreadContext::OnReset() {
   CHECK_EQ(sync.size(), 0);
-  ReleaseMemoryToOS(GetThreadTrace(tid), TraceSize() * sizeof(Event));
+  uptr trace_p = GetThreadTrace(tid);
+  ReleaseMemoryPagesToOS(trace_p, trace_p + TraceSize() * sizeof(Event));
   //!!! ReleaseMemoryToOS(GetThreadTraceHeader(tid), sizeof(Trace));
 }
 
 void ThreadContext::OnDetached(void *arg) {
   ThreadState *thr1 = static_cast<ThreadState*>(arg);
   sync.Reset(&thr1->proc()->clock_cache);
 }