Skip to content

Commit f78a742

Browse files
authored
[NFC][sanitizer] Rename Lock{Before,After}Fork suffixes locking StackDepotBase (#76279)
This is preparation for performance optimization. We need to highlight that this is very specific lock, and should not be used for other purposes. Add `fork_child` parameter to distinguish processes after fork.
1 parent 8097a5d commit f78a742

11 files changed

+109
-86
lines changed

compiler-rt/lib/asan/asan_posix.cpp

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -146,33 +146,37 @@ void PlatformTSDDtor(void *tsd) {
146146
# endif
147147
AsanThread::TSDDtor(tsd);
148148
}
149-
#endif
149+
# endif
150+
151+
static void BeforeFork() {
152+
if (CAN_SANITIZE_LEAKS) {
153+
__lsan::LockGlobal();
154+
}
155+
// `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and lock the
156+
// stuff we need.
157+
__lsan::LockThreads();
158+
__lsan::LockAllocator();
159+
StackDepotLockBeforeFork();
160+
}
161+
162+
static void AfterFork(bool fork_child) {
163+
StackDepotUnlockAfterFork(fork_child);
164+
// `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and unlock
165+
// the stuff we need.
166+
__lsan::UnlockAllocator();
167+
__lsan::UnlockThreads();
168+
if (CAN_SANITIZE_LEAKS) {
169+
__lsan::UnlockGlobal();
170+
}
171+
}
150172

151173
void InstallAtForkHandler() {
152174
# if SANITIZER_SOLARIS || SANITIZER_NETBSD || SANITIZER_APPLE
153175
return; // FIXME: Implement FutexWait.
154176
# endif
155-
auto before = []() {
156-
if (CAN_SANITIZE_LEAKS) {
157-
__lsan::LockGlobal();
158-
}
159-
// `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and lock the
160-
// stuff we need.
161-
__lsan::LockThreads();
162-
__lsan::LockAllocator();
163-
StackDepotLockAll();
164-
};
165-
auto after = []() {
166-
StackDepotUnlockAll();
167-
// `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and unlock
168-
// the stuff we need.
169-
__lsan::UnlockAllocator();
170-
__lsan::UnlockThreads();
171-
if (CAN_SANITIZE_LEAKS) {
172-
__lsan::UnlockGlobal();
173-
}
174-
};
175-
pthread_atfork(before, after, after);
177+
pthread_atfork(
178+
&BeforeFork, []() { AfterFork(/* fork_child= */ false); },
179+
[]() { AfterFork(/* fork_child= */ true); });
176180
}
177181

178182
void InstallAtExitCheckLeaks() {

compiler-rt/lib/dfsan/dfsan_chained_origin_depot.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,10 @@ static ChainedOriginDepot chainedOriginDepot;
1919

2020
ChainedOriginDepot* GetChainedOriginDepot() { return &chainedOriginDepot; }
2121

22+
void ChainedOriginDepotLockBeforeFork() { chainedOriginDepot.LockAll(); }
23+
24+
void ChainedOriginDepotUnlockAfterFork(bool fork_child) {
25+
chainedOriginDepot.UnlockAll();
26+
}
27+
2228
} // namespace __dfsan

compiler-rt/lib/dfsan/dfsan_chained_origin_depot.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ namespace __dfsan {
2121

2222
ChainedOriginDepot* GetChainedOriginDepot();
2323

24+
void ChainedOriginDepotLockBeforeFork();
25+
void ChainedOriginDepotUnlockAfterFork(bool fork_child);
26+
2427
} // namespace __dfsan
2528

2629
#endif // DFSAN_CHAINED_ORIGIN_DEPOT_H

compiler-rt/lib/dfsan/dfsan_custom.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2893,13 +2893,13 @@ int __dfso___isoc99_sscanf(char *str, const char *format, dfsan_label str_label,
28932893
}
28942894

28952895
static void BeforeFork() {
2896-
StackDepotLockAll();
2897-
GetChainedOriginDepot()->LockAll();
2896+
StackDepotLockBeforeFork();
2897+
ChainedOriginDepotLockBeforeFork();
28982898
}
28992899

2900-
static void AfterFork() {
2901-
GetChainedOriginDepot()->UnlockAll();
2902-
StackDepotUnlockAll();
2900+
static void AfterFork(bool fork_child) {
2901+
ChainedOriginDepotUnlockAfterFork(fork_child);
2902+
StackDepotUnlockAfterFork(fork_child);
29032903
}
29042904

29052905
SANITIZER_INTERFACE_ATTRIBUTE
@@ -2913,7 +2913,7 @@ SANITIZER_INTERFACE_ATTRIBUTE
29132913
pid_t __dfso_fork(dfsan_label *ret_label, dfsan_origin *ret_origin) {
29142914
BeforeFork();
29152915
pid_t pid = __dfsw_fork(ret_label);
2916-
AfterFork();
2916+
AfterFork(/* fork_child= */ pid == 0);
29172917
return pid;
29182918
}
29192919

compiler-rt/lib/hwasan/hwasan_linux.cpp

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -521,28 +521,32 @@ uptr TagMemoryAligned(uptr p, uptr size, tag_t tag) {
521521
return AddTagToPointer(p, tag);
522522
}
523523

524+
static void BeforeFork() {
525+
if (CAN_SANITIZE_LEAKS) {
526+
__lsan::LockGlobal();
527+
}
528+
// `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and lock the
529+
// stuff we need.
530+
__lsan::LockThreads();
531+
__lsan::LockAllocator();
532+
StackDepotLockBeforeFork();
533+
}
534+
535+
static void AfterFork(bool fork_child) {
536+
StackDepotUnlockAfterFork(fork_child);
537+
// `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and unlock
538+
// the stuff we need.
539+
__lsan::UnlockAllocator();
540+
__lsan::UnlockThreads();
541+
if (CAN_SANITIZE_LEAKS) {
542+
__lsan::UnlockGlobal();
543+
}
544+
}
545+
524546
void HwasanInstallAtForkHandler() {
525-
auto before = []() {
526-
if (CAN_SANITIZE_LEAKS) {
527-
__lsan::LockGlobal();
528-
}
529-
// `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and lock the
530-
// stuff we need.
531-
__lsan::LockThreads();
532-
__lsan::LockAllocator();
533-
StackDepotLockAll();
534-
};
535-
auto after = []() {
536-
StackDepotUnlockAll();
537-
// `_lsan` functions defined regardless of `CAN_SANITIZE_LEAKS` and unlock
538-
// the stuff we need.
539-
__lsan::UnlockAllocator();
540-
__lsan::UnlockThreads();
541-
if (CAN_SANITIZE_LEAKS) {
542-
__lsan::UnlockGlobal();
543-
}
544-
};
545-
pthread_atfork(before, after, after);
547+
pthread_atfork(
548+
&BeforeFork, []() { AfterFork(/* fork_child= */ false); },
549+
[]() { AfterFork(/* fork_child= */ true); });
546550
}
547551

548552
void InstallAtExitCheckLeaks() {

compiler-rt/lib/lsan/lsan_posix.cpp

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,23 +100,27 @@ void InstallAtExitCheckLeaks() {
100100
Atexit(DoLeakCheck);
101101
}
102102

103+
static void BeforeFork() {
104+
LockGlobal();
105+
LockThreads();
106+
LockAllocator();
107+
StackDepotLockBeforeFork();
108+
}
109+
110+
static void AfterFork(bool fork_child) {
111+
StackDepotUnlockAfterFork(fork_child);
112+
UnlockAllocator();
113+
UnlockThreads();
114+
UnlockGlobal();
115+
}
116+
103117
void InstallAtForkHandler() {
104118
# if SANITIZER_SOLARIS || SANITIZER_NETBSD || SANITIZER_APPLE
105119
return; // FIXME: Implement FutexWait.
106120
# endif
107-
auto before = []() {
108-
LockGlobal();
109-
LockThreads();
110-
LockAllocator();
111-
StackDepotLockAll();
112-
};
113-
auto after = []() {
114-
StackDepotUnlockAll();
115-
UnlockAllocator();
116-
UnlockThreads();
117-
UnlockGlobal();
118-
};
119-
pthread_atfork(before, after, after);
121+
pthread_atfork(
122+
&BeforeFork, []() { AfterFork(/* fork_child= */ false); },
123+
[]() { AfterFork(/* fork_child= */ true); });
120124
}
121125

122126
} // namespace __lsan

compiler-rt/lib/msan/msan_chained_origin_depot.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,9 @@ u32 ChainedOriginDepotGet(u32 id, u32 *other) {
3131
return chainedOriginDepot.Get(id, other);
3232
}
3333

34-
void ChainedOriginDepotLockAll() {
35-
chainedOriginDepot.LockAll();
36-
}
34+
void ChainedOriginDepotBeforeFork() { chainedOriginDepot.LockAll(); }
3735

38-
void ChainedOriginDepotUnlockAll() {
36+
void ChainedOriginDepotAfterFork(bool fork_child) {
3937
chainedOriginDepot.UnlockAll();
4038
}
4139

compiler-rt/lib/msan/msan_chained_origin_depot.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ bool ChainedOriginDepotPut(u32 here_id, u32 prev_id, u32 *new_id);
3030
// Retrieves the stored StackDepot ID for the given origin ID.
3131
u32 ChainedOriginDepotGet(u32 id, u32 *other);
3232

33-
void ChainedOriginDepotLockAll();
34-
void ChainedOriginDepotUnlockAll();
33+
void ChainedOriginDepotBeforeFork();
34+
void ChainedOriginDepotAfterFork(bool fork_child);
3535

3636
} // namespace __msan
3737

compiler-rt/lib/msan/msan_linux.cpp

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -256,22 +256,26 @@ void MsanTSDDtor(void *tsd) {
256256
atomic_signal_fence(memory_order_seq_cst);
257257
MsanThread::TSDDtor(tsd);
258258
}
259-
#endif
259+
# endif
260+
261+
static void BeforeFork() {
262+
// Usually we lock ThreadRegistry, but msan does not have one.
263+
LockAllocator();
264+
StackDepotLockBeforeFork();
265+
ChainedOriginDepotBeforeFork();
266+
}
267+
268+
static void AfterFork(bool fork_child) {
269+
ChainedOriginDepotAfterFork(fork_child);
270+
StackDepotUnlockAfterFork(fork_child);
271+
UnlockAllocator();
272+
// Usually we unlock ThreadRegistry, but msan does not have one.
273+
}
260274

261275
void InstallAtForkHandler() {
262-
auto before = []() {
263-
// Usually we lock ThreadRegistry, but msan does not have one.
264-
LockAllocator();
265-
StackDepotLockAll();
266-
ChainedOriginDepotLockAll();
267-
};
268-
auto after = []() {
269-
ChainedOriginDepotUnlockAll();
270-
StackDepotUnlockAll();
271-
UnlockAllocator();
272-
// Usually we unlock ThreadRegistry, but msan does not have one.
273-
};
274-
pthread_atfork(before, after, after);
276+
pthread_atfork(
277+
&BeforeFork, []() { AfterFork(/* fork_child= */ false); },
278+
[]() { AfterFork(/* fork_child= */ true); });
275279
}
276280

277281
} // namespace __msan

compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,13 @@ StackTrace StackDepotGet(u32 id) {
215215
return theDepot.Get(id);
216216
}
217217

218-
void StackDepotLockAll() {
218+
void StackDepotLockBeforeFork() {
219219
theDepot.LockAll();
220220
compress_thread.LockAndStop();
221221
stackStore.LockAll();
222222
}
223223

224-
void StackDepotUnlockAll() {
224+
void StackDepotUnlockAfterFork(bool fork_child) {
225225
stackStore.UnlockAll();
226226
compress_thread.Unlock();
227227
theDepot.UnlockAll();

compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ StackDepotHandle StackDepotPut_WithHandle(StackTrace stack);
3939
// Retrieves a stored stack trace by the id.
4040
StackTrace StackDepotGet(u32 id);
4141

42-
void StackDepotLockAll();
43-
void StackDepotUnlockAll();
42+
void StackDepotLockBeforeFork();
43+
void StackDepotUnlockAfterFork(bool fork_child);
4444
void StackDepotPrintAll();
4545
void StackDepotStopBackgroundThread();
4646

0 commit comments

Comments
 (0)