[llvm] [clang] [mlir] [compiler-rt] [libcxx] [sanitizers] Optimize locking StackDepotBase for fork (PR #76280)
Vitaly Buka via cfe-commits
cfe-commits at lists.llvm.org
Sat Dec 23 16:46:34 PST 2023
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/76280
>From e0c1de414d58b844382db93e3b3f4ac8cd8cf5b6 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Fri, 22 Dec 2023 23:26:11 -0800
Subject: [PATCH 1/4] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?=
=?UTF-8?q?anges=20to=20main=20this=20commit=20is=20based=20on?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
[skip ci]
---
compiler-rt/lib/asan/asan_posix.cpp | 48 ++++++++++---------
.../lib/dfsan/dfsan_chained_origin_depot.cpp | 6 +++
.../lib/dfsan/dfsan_chained_origin_depot.h | 3 ++
compiler-rt/lib/dfsan/dfsan_custom.cpp | 12 ++---
compiler-rt/lib/hwasan/hwasan_linux.cpp | 46 ++++++++++--------
compiler-rt/lib/lsan/lsan_posix.cpp | 30 +++++++-----
.../lib/msan/msan_chained_origin_depot.cpp | 6 +--
.../lib/msan/msan_chained_origin_depot.h | 4 +-
compiler-rt/lib/msan/msan_linux.cpp | 32 +++++++------
.../sanitizer_common/sanitizer_stackdepot.cpp | 4 +-
.../sanitizer_common/sanitizer_stackdepot.h | 4 +-
11 files changed, 109 insertions(+), 86 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_posix.cpp b/compiler-rt/lib/asan/asan_posix.cpp
index a5b87b7fbf1b5a..76564538bd5d77 100644
--- a/compiler-rt/lib/asan/asan_posix.cpp
+++ b/compiler-rt/lib/asan/asan_posix.cpp
@@ -146,33 +146,37 @@ void PlatformTSDDtor(void *tsd) {
# endif
AsanThread::TSDDtor(tsd);
}
-#endif
+# endif
+
+static void BeforeFork() {
+ if (CAN_SANITIZE_LEAKS) {
+ __lsan::LockGlobal();
+ }
+ // `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and lock the
+ // stuff we need.
+ __lsan::LockThreads();
+ __lsan::LockAllocator();
+ StackDepotLockBeforeFork();
+}
+
+static void AfterFork(bool fork_child) {
+ StackDepotUnlockAfterFork(fork_child);
+ // `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and unlock
+ // the stuff we need.
+ __lsan::UnlockAllocator();
+ __lsan::UnlockThreads();
+ if (CAN_SANITIZE_LEAKS) {
+ __lsan::UnlockGlobal();
+ }
+}
void InstallAtForkHandler() {
# if SANITIZER_SOLARIS || SANITIZER_NETBSD || SANITIZER_APPLE
return; // FIXME: Implement FutexWait.
# endif
- auto before = []() {
- if (CAN_SANITIZE_LEAKS) {
- __lsan::LockGlobal();
- }
- // `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and lock the
- // stuff we need.
- __lsan::LockThreads();
- __lsan::LockAllocator();
- StackDepotLockAll();
- };
- auto after = []() {
- StackDepotUnlockAll();
- // `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and unlock
- // the stuff we need.
- __lsan::UnlockAllocator();
- __lsan::UnlockThreads();
- if (CAN_SANITIZE_LEAKS) {
- __lsan::UnlockGlobal();
- }
- };
- pthread_atfork(before, after, after);
+ pthread_atfork(
+ &BeforeFork, []() { AfterFork(/* fork_child= */ false); },
+ []() { AfterFork(/* fork_child= */ true); });
}
void InstallAtExitCheckLeaks() {
diff --git a/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp b/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp
index 9ec598bf2ce9e4..6644bd6a7c6c0c 100644
--- a/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp
+++ b/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp
@@ -19,4 +19,10 @@ static ChainedOriginDepot chainedOriginDepot;
ChainedOriginDepot* GetChainedOriginDepot() { return &chainedOriginDepot; }
+void ChainedOriginDepotLockBeforeFork() { chainedOriginDepot.LockAll(); }
+
+void ChainedOriginDepotUnlockAfterFork(bool fork_child) {
+ chainedOriginDepot.UnlockAll();
+}
+
} // namespace __dfsan
diff --git a/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.h b/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.h
index d715ef707f41b7..83b9e29e1b710d 100644
--- a/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.h
+++ b/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.h
@@ -21,6 +21,9 @@ namespace __dfsan {
ChainedOriginDepot* GetChainedOriginDepot();
+void ChainedOriginDepotLockBeforeFork();
+void ChainedOriginDepotUnlockAfterFork(bool fork_child);
+
} // namespace __dfsan
#endif // DFSAN_CHAINED_ORIGIN_DEPOT_H
diff --git a/compiler-rt/lib/dfsan/dfsan_custom.cpp b/compiler-rt/lib/dfsan/dfsan_custom.cpp
index 38371d35336818..05b48fd0525e33 100644
--- a/compiler-rt/lib/dfsan/dfsan_custom.cpp
+++ b/compiler-rt/lib/dfsan/dfsan_custom.cpp
@@ -2893,13 +2893,13 @@ int __dfso___isoc99_sscanf(char *str, const char *format, dfsan_label str_label,
}
static void BeforeFork() {
- StackDepotLockAll();
- GetChainedOriginDepot()->LockAll();
+ StackDepotLockBeforeFork();
+ ChainedOriginDepotLockBeforeFork();
}
-static void AfterFork() {
- GetChainedOriginDepot()->UnlockAll();
- StackDepotUnlockAll();
+static void AfterFork(bool fork_child) {
+ ChainedOriginDepotUnlockAfterFork(fork_child);
+ StackDepotUnlockAfterFork(fork_child);
}
SANITIZER_INTERFACE_ATTRIBUTE
@@ -2913,7 +2913,7 @@ SANITIZER_INTERFACE_ATTRIBUTE
pid_t __dfso_fork(dfsan_label *ret_label, dfsan_origin *ret_origin) {
BeforeFork();
pid_t pid = __dfsw_fork(ret_label);
- AfterFork();
+ AfterFork(/* fork_child= */ pid == 0);
return pid;
}
diff --git a/compiler-rt/lib/hwasan/hwasan_linux.cpp b/compiler-rt/lib/hwasan/hwasan_linux.cpp
index 3271a955e7ed10..e6aa60b324fa7f 100644
--- a/compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -521,28 +521,32 @@ uptr TagMemoryAligned(uptr p, uptr size, tag_t tag) {
return AddTagToPointer(p, tag);
}
+static void BeforeFork() {
+ if (CAN_SANITIZE_LEAKS) {
+ __lsan::LockGlobal();
+ }
+ // `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and lock the
+ // stuff we need.
+ __lsan::LockThreads();
+ __lsan::LockAllocator();
+ StackDepotLockBeforeFork();
+}
+
+static void AfterFork(bool fork_child) {
+ StackDepotUnlockAfterFork(fork_child);
+ // `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and unlock
+ // the stuff we need.
+ __lsan::UnlockAllocator();
+ __lsan::UnlockThreads();
+ if (CAN_SANITIZE_LEAKS) {
+ __lsan::UnlockGlobal();
+ }
+}
+
void HwasanInstallAtForkHandler() {
- auto before = []() {
- if (CAN_SANITIZE_LEAKS) {
- __lsan::LockGlobal();
- }
- // `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and lock the
- // stuff we need.
- __lsan::LockThreads();
- __lsan::LockAllocator();
- StackDepotLockAll();
- };
- auto after = []() {
- StackDepotUnlockAll();
- // `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and unlock
- // the stuff we need.
- __lsan::UnlockAllocator();
- __lsan::UnlockThreads();
- if (CAN_SANITIZE_LEAKS) {
- __lsan::UnlockGlobal();
- }
- };
- pthread_atfork(before, after, after);
+ pthread_atfork(
+ &BeforeFork, []() { AfterFork(/* fork_child= */ false); },
+ []() { AfterFork(/* fork_child= */ true); });
}
void InstallAtExitCheckLeaks() {
diff --git a/compiler-rt/lib/lsan/lsan_posix.cpp b/compiler-rt/lib/lsan/lsan_posix.cpp
index 4bfadf1ef809c3..422c29acca69f3 100644
--- a/compiler-rt/lib/lsan/lsan_posix.cpp
+++ b/compiler-rt/lib/lsan/lsan_posix.cpp
@@ -100,23 +100,27 @@ void InstallAtExitCheckLeaks() {
Atexit(DoLeakCheck);
}
+static void BeforeFork() {
+ LockGlobal();
+ LockThreads();
+ LockAllocator();
+ StackDepotLockBeforeFork();
+}
+
+static void AfterFork(bool fork_child) {
+ StackDepotUnlockAfterFork(fork_child);
+ UnlockAllocator();
+ UnlockThreads();
+ UnlockGlobal();
+}
+
void InstallAtForkHandler() {
# if SANITIZER_SOLARIS || SANITIZER_NETBSD || SANITIZER_APPLE
return; // FIXME: Implement FutexWait.
# endif
- auto before = []() {
- LockGlobal();
- LockThreads();
- LockAllocator();
- StackDepotLockAll();
- };
- auto after = []() {
- StackDepotUnlockAll();
- UnlockAllocator();
- UnlockThreads();
- UnlockGlobal();
- };
- pthread_atfork(before, after, after);
+ pthread_atfork(
+ &BeforeFork, []() { AfterFork(/* fork_child= */ false); },
+ []() { AfterFork(/* fork_child= */ true); });
}
} // namespace __lsan
diff --git a/compiler-rt/lib/msan/msan_chained_origin_depot.cpp b/compiler-rt/lib/msan/msan_chained_origin_depot.cpp
index 49b14131a89be0..c3bd54141e6c38 100644
--- a/compiler-rt/lib/msan/msan_chained_origin_depot.cpp
+++ b/compiler-rt/lib/msan/msan_chained_origin_depot.cpp
@@ -31,11 +31,9 @@ u32 ChainedOriginDepotGet(u32 id, u32 *other) {
return chainedOriginDepot.Get(id, other);
}
-void ChainedOriginDepotLockAll() {
- chainedOriginDepot.LockAll();
-}
+void ChainedOriginDepotBeforeFork() { chainedOriginDepot.LockAll(); }
-void ChainedOriginDepotUnlockAll() {
+void ChainedOriginDepotAfterFork(bool fork_child) {
chainedOriginDepot.UnlockAll();
}
diff --git a/compiler-rt/lib/msan/msan_chained_origin_depot.h b/compiler-rt/lib/msan/msan_chained_origin_depot.h
index ea51c77a905b5d..7518745dc8520a 100644
--- a/compiler-rt/lib/msan/msan_chained_origin_depot.h
+++ b/compiler-rt/lib/msan/msan_chained_origin_depot.h
@@ -30,8 +30,8 @@ bool ChainedOriginDepotPut(u32 here_id, u32 prev_id, u32 *new_id);
// Retrieves the stored StackDepot ID for the given origin ID.
u32 ChainedOriginDepotGet(u32 id, u32 *other);
-void ChainedOriginDepotLockAll();
-void ChainedOriginDepotUnlockAll();
+void ChainedOriginDepotBeforeFork();
+void ChainedOriginDepotAfterFork(bool fork_child);
} // namespace __msan
diff --git a/compiler-rt/lib/msan/msan_linux.cpp b/compiler-rt/lib/msan/msan_linux.cpp
index 04af6f4b27ac89..c7ecb7cad56661 100644
--- a/compiler-rt/lib/msan/msan_linux.cpp
+++ b/compiler-rt/lib/msan/msan_linux.cpp
@@ -256,22 +256,26 @@ void MsanTSDDtor(void *tsd) {
atomic_signal_fence(memory_order_seq_cst);
MsanThread::TSDDtor(tsd);
}
-#endif
+# endif
+
+static void BeforeFork() {
+ // Usually we lock ThreadRegistry, but msan does not have one.
+ LockAllocator();
+ StackDepotLockBeforeFork();
+ ChainedOriginDepotBeforeFork();
+}
+
+static void AfterFork(bool fork_child) {
+ ChainedOriginDepotAfterFork(fork_child);
+ StackDepotUnlockAfterFork(fork_child);
+ UnlockAllocator();
+ // Usually we unlock ThreadRegistry, but msan does not have one.
+}
void InstallAtForkHandler() {
- auto before = []() {
- // Usually we lock ThreadRegistry, but msan does not have one.
- LockAllocator();
- StackDepotLockAll();
- ChainedOriginDepotLockAll();
- };
- auto after = []() {
- ChainedOriginDepotUnlockAll();
- StackDepotUnlockAll();
- UnlockAllocator();
- // Usually we unlock ThreadRegistry, but msan does not have one.
- };
- pthread_atfork(before, after, after);
+ pthread_atfork(
+ &BeforeFork, []() { AfterFork(/* fork_child= */ false); },
+ []() { AfterFork(/* fork_child= */ true); });
}
} // namespace __msan
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
index a746d4621936c6..ce21f3c178bce0 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
@@ -215,13 +215,13 @@ StackTrace StackDepotGet(u32 id) {
return theDepot.Get(id);
}
-void StackDepotLockAll() {
+void StackDepotLockBeforeFork() {
theDepot.LockAll();
compress_thread.LockAndStop();
stackStore.LockAll();
}
-void StackDepotUnlockAll() {
+void StackDepotUnlockAfterFork(bool fork_child) {
stackStore.UnlockAll();
compress_thread.Unlock();
theDepot.UnlockAll();
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h
index cca6fd53468839..82cf7578d0fb9b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h
@@ -39,8 +39,8 @@ StackDepotHandle StackDepotPut_WithHandle(StackTrace stack);
// Retrieves a stored stack trace by the id.
StackTrace StackDepotGet(u32 id);
-void StackDepotLockAll();
-void StackDepotUnlockAll();
+void StackDepotLockBeforeFork();
+void StackDepotUnlockAfterFork(bool fork_child);
void StackDepotPrintAll();
void StackDepotStopBackgroundThread();
>From 81f41d59616955be777577bfe82591d9daac2c46 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sat, 23 Dec 2023 09:48:18 -0800
Subject: [PATCH 2/4] LIT_OPTS
Created using spr 1.3.4
---
.ci/monolithic-linux.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.ci/monolithic-linux.sh b/.ci/monolithic-linux.sh
index f0577d1069d5de..20d2022b2894ee 100755
--- a/.ci/monolithic-linux.sh
+++ b/.ci/monolithic-linux.sh
@@ -54,4 +54,4 @@ cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
echo "--- ninja"
# Targets are not escaped as they are passed as separate arguments.
-ninja -C "${BUILD_DIR}" ${targets}
+LIT_OPTS="--timeout=1500" ninja -C "${BUILD_DIR}" ${targets}
>From 1407dec21ef9b8d45e124f8935c022303d63a7f0 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sat, 23 Dec 2023 15:37:18 -0800
Subject: [PATCH 3/4] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?=
=?UTF-8?q?anges=20introduced=20through=20rebase?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
[skip ci]
---
.ci/monolithic-linux.sh | 2 +-
.ci/monolithic-windows.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/.ci/monolithic-linux.sh b/.ci/monolithic-linux.sh
index f0577d1069d5de..1e7b2d2a36c245 100755
--- a/.ci/monolithic-linux.sh
+++ b/.ci/monolithic-linux.sh
@@ -45,7 +45,7 @@ cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
-D LLVM_ENABLE_ASSERTIONS=ON \
-D LLVM_BUILD_EXAMPLES=ON \
-D COMPILER_RT_BUILD_LIBFUZZER=OFF \
- -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml" \
+ -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml --timeout=1200 --time-tests" \
-D LLVM_ENABLE_LLD=ON \
-D CMAKE_CXX_FLAGS=-gmlt \
-D BOLT_CLANG_EXE=/usr/bin/clang \
diff --git a/.ci/monolithic-windows.sh b/.ci/monolithic-windows.sh
index 7ac806a0b399ad..a704e855f011cb 100755
--- a/.ci/monolithic-windows.sh
+++ b/.ci/monolithic-windows.sh
@@ -45,7 +45,7 @@ cmake -S ${MONOREPO_ROOT}/llvm -B ${BUILD_DIR} \
-D LLVM_ENABLE_ASSERTIONS=ON \
-D LLVM_BUILD_EXAMPLES=ON \
-D COMPILER_RT_BUILD_LIBFUZZER=OFF \
- -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml" \
+ -D LLVM_LIT_ARGS="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml --timeout=1200 --time-tests" \
-D COMPILER_RT_BUILD_ORC=OFF \
-D CMAKE_C_COMPILER_LAUNCHER=sccache \
-D CMAKE_CXX_COMPILER_LAUNCHER=sccache \
>From e5631e1f2095bacd0778c327912842efefefcb95 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Sat, 23 Dec 2023 16:46:22 -0800
Subject: [PATCH 4/4] format
Created using spr 1.3.4
---
compiler-rt/lib/sanitizer_common/sanitizer_flat_map.h | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_flat_map.h b/compiler-rt/lib/sanitizer_common/sanitizer_flat_map.h
index ee04c9b7e9c310..d246781fe1df5e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_flat_map.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_flat_map.h
@@ -109,13 +109,9 @@ class TwoLevelMap {
return *AddressSpaceView::LoadWritable(&map2[idx % kSize2]);
}
- void Lock() SANITIZER_NO_THREAD_SAFETY_ANALYSIS {
- mu_.Lock();
- }
+ void Lock() SANITIZER_NO_THREAD_SAFETY_ANALYSIS { mu_.Lock(); }
- void Unlock() SANITIZER_NO_THREAD_SAFETY_ANALYSIS {
- mu_.Unlock();
- }
+ void Unlock() SANITIZER_NO_THREAD_SAFETY_ANALYSIS { mu_.Unlock(); }
private:
constexpr uptr MmapSize() const {
More information about the cfe-commits
mailing list