summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVitaly Buka <vitalybuka@google.com>2023-12-23 12:50:52 -0800
committerVitaly Buka <vitalybuka@google.com>2023-12-23 12:50:52 -0800
commitdcf64907d99281b80a3405d7b34e3836fd021819 (patch)
tree12f034c6f3656d376e047d44d0f7d1ed7f1b0294
parent061e4f24b24a3b59d73a94dc6f2f0d21a2b7beac (diff)
Created using spr 1.3.4 [skip ci]
-rw-r--r--compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp6
-rw-r--r--compiler-rt/lib/msan/msan_chained_origin_depot.cpp9
-rw-r--r--compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp14
-rw-r--r--compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h3
4 files changed, 26 insertions, 6 deletions
diff --git a/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp b/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp
index 6644bd6a7c6c..d078265258ca 100644
--- a/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp
+++ b/compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp
@@ -19,9 +19,13 @@ static ChainedOriginDepot chainedOriginDepot;
ChainedOriginDepot* GetChainedOriginDepot() { return &chainedOriginDepot; }
-void ChainedOriginDepotLockBeforeFork() { chainedOriginDepot.LockAll(); }
+void ChainedOriginDepotLockBeforeFork() {
+ // TODO: Consider same optimization as `StackDepotLockBeforeFork`.
+ chainedOriginDepot.LockAll();
+}
void ChainedOriginDepotUnlockAfterFork(bool fork_child) {
+ // TODO: Consider same optimization as `StackDepotUnlockAfterFork`.
chainedOriginDepot.UnlockAll();
}
diff --git a/compiler-rt/lib/msan/msan_chained_origin_depot.cpp b/compiler-rt/lib/msan/msan_chained_origin_depot.cpp
index c3bd54141e6c..66c807086693 100644
--- a/compiler-rt/lib/msan/msan_chained_origin_depot.cpp
+++ b/compiler-rt/lib/msan/msan_chained_origin_depot.cpp
@@ -31,10 +31,15 @@ u32 ChainedOriginDepotGet(u32 id, u32 *other) {
return chainedOriginDepot.Get(id, other);
}
-void ChainedOriginDepotBeforeFork() { chainedOriginDepot.LockAll(); }
+void ChainedOriginDepotBeforeFork() {
+ // Don't `chainedOriginDepot.LockAll()`, see `StackDepotLockBeforeFork`.
+}
void ChainedOriginDepotAfterFork(bool fork_child) {
- chainedOriginDepot.UnlockAll();
+ // See `StackDepotUnlockAfterFork`.
+ if (fork_child) {
+ chainedOriginDepot.UnlockAll();
+ }
}
} // namespace __msan
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
index ce21f3c178bc..8d134ca5a41a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp
@@ -216,7 +216,13 @@ StackTrace StackDepotGet(u32 id) {
}
void StackDepotLockBeforeFork() {
- theDepot.LockAll();
+ // Do not `theDepot.LockAll()`. It's very slow, but not rely needed. The
+ // parent process will neither lock nor unlock. Child process risks to be
+ // deadlocked on already locked buckets. To avoid deadlock we will unlock
+ // every locked buckets. This may affect consistency of the hash table, but
+ // the only possible issue is a few items inserted by parent process will be
+ // not found by child, and the child may insert them again, wasting some space
+ // in `stackStore`.
compress_thread.LockAndStop();
stackStore.LockAll();
}
@@ -224,7 +230,11 @@ void StackDepotLockBeforeFork() {
void StackDepotUnlockAfterFork(bool fork_child) {
stackStore.UnlockAll();
compress_thread.Unlock();
- theDepot.UnlockAll();
+ if (fork_child) {
+ // Only child process needs to unlock to avoid deadlock. See
+ // `StackDepotLockAll`.
+ theDepot.UnlockAll();
+ }
}
void StackDepotPrintAll() {
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h
index 96d1ddc87fd0..3440a8f628e9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h
@@ -171,7 +171,8 @@ void StackDepotBase<Node, kReservedBits, kTabSizeLog>::UnlockAll() {
for (int i = 0; i < kTabSize; ++i) {
atomic_uint32_t *p = &tab[i];
uptr s = atomic_load(p, memory_order_relaxed);
- unlock(p, s & kUnlockMask);
+ if (s & kLockMask)
+ unlock(p, s & kUnlockMask);
}
}