[scudo] Use MAP_NORESERVE to release pages

[vitalybuka]

Summary

This allows to avoid memset in workaround
of QEMU-user implementation of MADV_DONTNEED.

[cryptoad]

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

171

Looks like Data is still UNUSED

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

156 ↗

(On Diff #347236)

Maybe move that up a block since it's common to the if & else

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

1

There is a misalignment of - compared to the bottom one. Not sure which one has the right count.

18–20

Can this be put in SCUDO_LINUX block and a UNREACHABLE or equivalent for other systems?

37

= {};
Not important for Linux but will be for Fuchsia.

[vitalybuka]

addressing @cryptoad comments

[vitalybuka]

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

37

This file is done on D102994

[hctim]

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

270 ↗

(On Diff #347276)

More descriptive defaults: MAP_ALLOWNOMEM | MAP_RESIZABLE?

357 ↗

(On Diff #347276)

MTE mode can change at runtime after initLinkerInitialized is called, so we do need to re-check useMemoryTagging<>() every time we allocate. Maybe call the member PersistentMapFlags (i.e. it persists across the allocator and is once-init), and add a member getMapFlags() that returns PersistentMapFlags | (useMemoryTagging<>() ? MAP_MEMTAG : 0).

[pcc]

TBH, I'm not sure it's worth going to this much effort to work around this QEMU bug. It seems better to figure out a way to use QEMU in system mode instead.

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

358 ↗

(On Diff #347276)

This will continue to create mappings with PROT_MTE after MTE is disabled.

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

385 ↗

(On Diff #347276)

This will break the split mapping of MTE enabled secondary allocations. The first few pages need to have MTE enabled and the rest have MTE disabled.

[pcc]

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

270 ↗

(On Diff #347276)

Initialize to 0 so that the combined allocator remains in bss.

[vitalybuka]

In D102979#2777588, @pcc wrote:

TBH, I'm not sure it's worth going to this much effort to work around this QEMU bug. It seems better to figure out a way to use QEMU in system mode instead.

This does not looks like a lot of work to maintain this, but I propose to threat this as default behavior D102980.
Let's chat about if we need QEMU-user later today.

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

270 ↗

(On Diff #347276)

I replaced with function which will return flags.

357 ↗

(On Diff #347276)

Actually missed this point, and tried to keep same protection.
If so I can just read Options when create ReleaseRecorder.

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

385 ↗

(On Diff #347276)

Thanks, I missed this one. Seems easy to fix.
Before that I'd like to NFC refactor this code to make it more obvious.

[vitalybuka]

addressed most comments

[vitalybuka]

Avoid tracking Prot flags in allocators

[vitalybuka]

fix x86 build

[vitalybuka]

need reconsider as I found another related issue

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

 #include "linux.h"
 #include "mutex.h"
 #include "string_utils.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/futex.h>
 #include <sched.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/syscall.h>
 #include <sys/time.h>
 #include <time.h>
 #include <unistd.h>
static bool madviseNotNeedFailsCached() {
   State NeedsMemset = static_cast<State>(atomic_load_relaxed(&Cache));
   if (NeedsMemset == Unknown) {
     NeedsMemset = madviseNotNeedFails() ? Yes : No;
     atomic_store_relaxed(&Cache, NeedsMemset);
   }
   return NeedsMemset == Yes;
 }
 
+static uptr getProt(FILE *F, uptr Ptr, int *Prot) {
+  CHECK(F);
+  char line[1024];
+  u64 B, E;
+  do {
+    CHECK_NE(fgets(line, sizeof(line), F), nullptr);
+  } while (sscanf(line, "%llx-%llx ", &B, &E) != 2 || Ptr < B || Ptr >= E);
+
+  CHECK_LE(B, Ptr);
+  CHECK_LT(Ptr, E);
+
+  static const char Field[] = "VmFlags:";
+  do {
+    CHECK_NE(fgets(line, sizeof(line), F), nullptr);
+  } while (strncmp(line, Field, sizeof(Field) - 1) != 0);
+
+  *Prot = 0;
+  char *Saved = nullptr;
+  char *Token = strtok_r(line + sizeof(Field) - 1, " \n", &Saved);
+  for (; Token; Token = strtok_r(nullptr, " \n", &Saved)) {
+    if (strcmp(Token, "rd") == 0)
+      *Prot |= PROT_READ;
+    else if (strcmp(Token, "wr") == 0)
+      *Prot |= PROT_WRITE;
+    else if (strcmp(Token, "mt") == 0)
+      *Prot |= PROT_MTE;
+  }
+  return E;
+}
+
+static void releaseByReMap(void *Addr, uptr Size) {
+  FILE *F = fopen("/proc/self/smaps", "r");
+  while (Size) {
+    uptr B = reinterpret_cast<uptr>(Addr);
+    int Prot;
+    // FIXME: Tests do not catch if we just set PROT_READ | PROT_WRITE.
+    uptr E = getProt(F, B, &Prot);
+    uptr MapSize = Min(E - B, Size);
+    if (Addr != mmap(Addr, MapSize, Prot,
+                     MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE,
+                     -1, 0)) {
+      dieOnMapUnmapError();
+    }
+    Addr = reinterpret_cast<void *>(B + MapSize);
+    Size -= MapSize;
+  }
+  fclose(F);
+}
+
 void releasePagesToOS(uptr BaseAddress, uptr Offset, uptr Size,
                       UNUSED MapPlatformData *Data) {
   void *Addr = reinterpret_cast<void *>(BaseAddress + Offset);
   if (madviseNotNeedFailsCached()) {
     // Workaround for QEMU-user ignoring MADV_DONTNEED.
     // https://github.com/qemu/qemu/blob/b1cffefa1b163bce9aebc3416f562c1d3886eeaa/linux-user/syscall.c#L11941
     // https://bugs.launchpad.net/qemu/+bug/1926521
-    memset(Addr, 0, Size);
+    return releaseByReMap(Addr, Size);
   }
   while (madvise(Addr, Size, MADV_DONTNEED) == -1 && errno == EAGAIN) {
   }
 }
 
 // Calling getenv should be fine (c)(tm) at any time.
 const char *getEnv(const char *Name) { return getenv(Name); }
 

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

 //===-- common_test.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 "internal_defs.h"
 #include "tests/scudo_unit_test.h"
 
 #include "common.h"
 #include <algorithm>
 #include <fstream>
 
 namespace scudo {
 
 static uptr getResidentMemorySize() {
   if (!SCUDO_LINUX)
     UNREACHABLE("Not implemented!");
   uptr Size;
   uptr Resident;
   std::ifstream IFS("/proc/self/statm");
   IFS >> Size;
   IFS >> Resident;
   return Resident * getPageSizeCached();
 }
 
 // Fuchsia needs getResidentMemorySize implementation.
 TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) {
   uptr OnStart = getResidentMemorySize();
   EXPECT_GT(OnStart, 0UL);
 
   const uptr Size = 1ull << 30;
   const uptr Threshold = Size >> 3;
 
   MapPlatformData Data = {};
   uptr *P = reinterpret_cast<uptr *>(
       map(nullptr, Size, "ResidentMemorySize", 0, &Data));
   const ptrdiff_t N = Size / sizeof(*P);
   ASSERT_NE(nullptr, P);
   EXPECT_LT(getResidentMemorySize() - OnStart, Threshold);
   EXPECT_EQ(std::count(P, P + N, 0), N);
 
   memset(P, 1, Size);
   EXPECT_EQ(std::count(P, P + N, 0), 0);
   EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold);
 
   releasePagesToOS((uptr)P, 0, Size, &Data);
+  EXPECT_LT(getResidentMemorySize() - OnStart, Threshold);
   EXPECT_EQ(std::count(P, P + N, 0), N);
-  // FIXME: does not work with QEMU-user.
-  // EXPECT_LT(getResidentMemorySize() - OnStart, Threshold);
 
   memset(P, 1, Size);
   EXPECT_EQ(std::count(P, P + N, 0), 0);
   EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold);
 
   unmap(P, Size, 0, &Data);
 }
 
 } // namespace scudo