[PATCH] D72364: [scudo][standalone] Modify malloc_{enable,disable} wrt fork

Mitch Phillips via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 7 15:23:57 PST 2020


hctim added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp:314
+
+// If a fork occurs while the allocator was disabled, we want to ensure that the
+// child can use allocation primitives without enabling the allocator first.
----------------
Maybe add a test with a fork() where malloc isn't disabled?


================
Comment at: compiler-rt/lib/scudo/standalone/wrappers_c.inc:158
+static void SCUDO_PREFIX(setupAtFork)() {
+  pthread_atfork(nullptr, nullptr, SCUDO_PREFIX(malloc_enable));
+}
----------------
instead of unconditionally calling `malloc_enable` on fork, can you guard it behind an `if Disabled`? Otherwise you call `unlock()` on an already-unlocked mutex in the allocators.


================
Comment at: compiler-rt/lib/scudo/standalone/wrappers_c.inc:163
+  scudo::ScopedLock L(SCUDO_PREFIX(Mutex));
+  if (SCUDO_PREFIX(Disabled))
+    return;
----------------
Just to clarify, this is non-racy because malloc_enable and malloc_disable are always called in a single-threaded context? libmemunreachable I know `PTRACE_INTERRUPT`'s the other threads, but unsure about other uses of malloc_enable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72364/new/

https://reviews.llvm.org/D72364





More information about the llvm-commits mailing list