[compiler-rt] 4d1d8ae - Replace shadow space zero-out by madvise at mmap

Jianzhou Zhao via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 6 14:30:31 PDT 2020


Author: Jianzhou Zhao
Date: 2020-10-06T21:29:50Z
New Revision: 4d1d8ae7100ec3c7e1709addb7b3ec6f9ad0b44f

URL: https://github.com/llvm/llvm-project/commit/4d1d8ae7100ec3c7e1709addb7b3ec6f9ad0b44f
DIFF: https://github.com/llvm/llvm-project/commit/4d1d8ae7100ec3c7e1709addb7b3ec6f9ad0b44f.diff

LOG: Replace shadow space zero-out by madvise at mmap

After D88686, munmap uses MADV_DONTNEED to ensure zero-out before the
next access. Because the entire shadow space is created by MAP_PRIVATE
and MAP_ANONYMOUS, the first access is also on zero-filled values.

So it is fine to not zero-out data, but use madvise(MADV_DONTNEED) at
mmap. This reduces runtime
overhead.

Reviewed-by: morehouse

Differential Revision: https://reviews.llvm.org/D88755

Added: 
    compiler-rt/test/dfsan/release_shadow_space.c

Modified: 
    compiler-rt/lib/dfsan/dfsan_interceptors.cpp

Removed: 
    compiler-rt/test/dfsan/munmap_release_shadow.c


################################################################################
diff  --git a/compiler-rt/lib/dfsan/dfsan_interceptors.cpp b/compiler-rt/lib/dfsan/dfsan_interceptors.cpp
index 5ab7c2b4828c..e322d18f7d33 100644
--- a/compiler-rt/lib/dfsan/dfsan_interceptors.cpp
+++ b/compiler-rt/lib/dfsan/dfsan_interceptors.cpp
@@ -20,7 +20,19 @@
 
 using namespace __sanitizer;
 
-static bool interceptors_initialized;
+namespace {
+
+bool interceptors_initialized;
+
+void ReleaseShadowMemoryPagesToOS(void *addr, SIZE_T length) {
+  uptr beg_shadow_addr = (uptr)__dfsan::shadow_for(addr);
+  void *end_addr =
+      (void *)((uptr)addr + RoundUpTo(length, GetPageSizeCached()));
+  uptr end_shadow_addr = (uptr)__dfsan::shadow_for(end_addr);
+  ReleaseMemoryPagesToOS(beg_shadow_addr, end_shadow_addr);
+}
+
+}  // namespace
 
 INTERCEPTOR(void *, mmap, void *addr, SIZE_T length, int prot, int flags,
             int fd, OFF_T offset) {
@@ -34,7 +46,7 @@ INTERCEPTOR(void *, mmap, void *addr, SIZE_T length, int prot, int flags,
     res = REAL(mmap)(addr, length, prot, flags, fd, offset);
 
   if (res != (void *)-1)
-    dfsan_set_label(0, res, RoundUpTo(length, GetPageSize()));
+    ReleaseShadowMemoryPagesToOS(res, length);
   return res;
 }
 
@@ -42,18 +54,14 @@ INTERCEPTOR(void *, mmap64, void *addr, SIZE_T length, int prot, int flags,
             int fd, OFF64_T offset) {
   void *res = REAL(mmap64)(addr, length, prot, flags, fd, offset);
   if (res != (void *)-1)
-    dfsan_set_label(0, res, RoundUpTo(length, GetPageSize()));
+    ReleaseShadowMemoryPagesToOS(res, length);
   return res;
 }
 
 INTERCEPTOR(int, munmap, void *addr, SIZE_T length) {
   int res = REAL(munmap)(addr, length);
   if (res != -1) {
-    uptr beg_shadow_addr = (uptr)__dfsan::shadow_for(addr);
-    void *end_addr =
-        (void *)((uptr)addr + RoundUpTo(length, GetPageSizeCached()));
-    uptr end_shadow_addr = (uptr)__dfsan::shadow_for(end_addr);
-    ReleaseMemoryPagesToOS(beg_shadow_addr, end_shadow_addr);
+    ReleaseShadowMemoryPagesToOS(addr, length);
   }
   return res;
 }

diff  --git a/compiler-rt/test/dfsan/munmap_release_shadow.c b/compiler-rt/test/dfsan/munmap_release_shadow.c
deleted file mode 100644
index 03197dfb8641..000000000000
--- a/compiler-rt/test/dfsan/munmap_release_shadow.c
+++ /dev/null
@@ -1,54 +0,0 @@
-// RUN: %clang_dfsan %s -o %t && %run %t
-
-#include <assert.h>
-#include <sanitizer/dfsan_interface.h>
-#include <stdbool.h>
-#include <stdio.h>
-#include <string.h>
-#include <sys/mman.h>
-#include <unistd.h>
-
-size_t get_rss_kb() {
-  long rss = 0L;
-  FILE *f = NULL;
-  assert((f = fopen("/proc/self/statm", "r")));
-  assert(fscanf(f, "%*s%ld", &rss) == 1);
-  fclose(f);
-  return ((size_t)rss * (size_t)sysconf(_SC_PAGESIZE)) >> 10;
-}
-
-int main(int argc, char **argv) {
-  const size_t map_size = 100 << 20;
-  size_t before = get_rss_kb();
-
-  char *p = mmap(NULL, map_size, PROT_READ | PROT_WRITE,
-                 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-  const dfsan_label label = dfsan_create_label("l", 0);
-  char val = 0xff;
-  dfsan_set_label(label, &val, sizeof(val));
-  memset(p, val, map_size);
-  size_t after_mmap = get_rss_kb();
-
-  munmap(p, map_size);
-  size_t after_munmap = get_rss_kb();
-
-  p = mmap(NULL, map_size, PROT_READ | PROT_WRITE,
-                 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-  dfsan_set_label(label, &val, sizeof(val));
-  memset(p, val, map_size);
-  size_t after_mmap2 = get_rss_kb();
-
-  fprintf(stderr, "RSS at start: %td, after mmap: %td, after mumap: %td, after mmap2: %td\n",
-          before, after_mmap, after_munmap, after_mmap2);
-
-  // The memory after mmap increases 3 times of map_size because the overhead of
-  // shadow memory is 2x.
-  const size_t mmap_cost_kb = 3 * (map_size >> 10);
-  assert(after_mmap >= before + mmap_cost_kb);
-  // OS does not release memory to the same level as the start of the program.
-  // The assert checks the memory after munmap up to a delta.
-  const size_t delta = 50000;
-  assert(after_mmap2 <= after_mmap + delta);
-
-  return 0;
-}

diff  --git a/compiler-rt/test/dfsan/release_shadow_space.c b/compiler-rt/test/dfsan/release_shadow_space.c
new file mode 100644
index 000000000000..40c0f3841eee
--- /dev/null
+++ b/compiler-rt/test/dfsan/release_shadow_space.c
@@ -0,0 +1,83 @@
+// RUN: %clang_dfsan %s -o %t && %run %t
+
+#include <assert.h>
+#include <sanitizer/dfsan_interface.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+size_t get_rss_kb() {
+  size_t ret = 0;
+  pid_t pid = getpid();
+
+  char fname[256];
+  sprintf(fname, "/proc/%ld/task/%ld/smaps", (long)pid, (long)pid);
+  FILE *f = fopen(fname, "r");
+  assert(f);
+
+  char buf[256];
+  while (fgets(buf, sizeof(buf), f) != NULL) {
+    int64_t rss;
+    if (sscanf(buf, "Rss: %ld kB", &rss) == 1)
+      ret += rss;
+  }
+  assert(feof(f));
+  fclose(f);
+
+  return ret;
+}
+
+int main(int argc, char **argv) {
+  const size_t map_size = 100 << 20;
+  size_t before = get_rss_kb();
+
+  // mmap and touch all addresses. The overhead is 1x.
+  char *p = mmap(NULL, map_size, PROT_READ | PROT_WRITE,
+                 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  memset(p, 0xff, map_size);
+  size_t after_mmap = get_rss_kb();
+
+  // store labels to all addresses. The overhead is 2x.
+  const dfsan_label label = dfsan_create_label("l", 0);
+  char val = 0xff;
+  dfsan_set_label(label, &val, sizeof(val));
+  memset(p, val, map_size);
+  size_t after_mmap_and_set_label = get_rss_kb();
+
+  // fixed-mmap the same address. OS recyles pages and reinitializes data at the
+  // address. This should be the same to calling munmap.
+  p = mmap(p, map_size, PROT_READ | PROT_WRITE,
+           MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
+  size_t after_fixed_mmap = get_rss_kb();
+
+  // store labels to all addresses.
+  memset(p, val, map_size);
+  size_t after_mmap_and_set_label2 = get_rss_kb();
+
+  // munmap the addresses.
+  munmap(p, map_size);
+  size_t after_munmap = get_rss_kb();
+
+  fprintf(
+      stderr,
+      "RSS at start: %zu, after mmap: %zu, after mmap+set label: %zu, after "
+      "fixed map: %zu, after another mmap+set label: %zu, after munmap: %zu\n",
+      before, after_mmap, after_mmap_and_set_label, after_fixed_mmap,
+      after_mmap_and_set_label2, after_munmap);
+
+  const size_t mmap_cost_kb = map_size >> 10;
+  const size_t mmap_shadow_cost_kb = 2 * mmap_cost_kb;
+  assert(after_mmap >= before + mmap_cost_kb);
+  assert(after_mmap_and_set_label >= after_mmap + mmap_shadow_cost_kb);
+  assert(after_mmap_and_set_label2 >= before + mmap_cost_kb + mmap_shadow_cost_kb);
+
+  // RSS may not change memory amount after munmap to the same level as the
+  // start of the program. The assert checks the memory up to a delta.
+  const size_t delta = 5000;
+  assert(after_fixed_mmap <= before + delta);
+  assert(after_munmap <= before + delta);
+
+  return 0;
+}


        


More information about the llvm-commits mailing list