Skip to content

Commit eb2db81

Browse files
committed
Revert "[LSAN] More LSAN interface tweaking."
Breaks bots. Also it's missing changes we discussed on review. This reverts commit f001e50. This reverts commit 2924189.
1 parent 2532aa5 commit eb2db81

File tree

8 files changed

+45
-49
lines changed

8 files changed

+45
-49
lines changed

compiler-rt/lib/asan/asan_allocator.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,6 +1153,33 @@ IgnoreObjectResult IgnoreObjectLocked(const void *p) {
11531153
return kIgnoreObjectSuccess;
11541154
}
11551155

1156+
void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs) {
1157+
// Look for the arg pointer of threads that have been created or are running.
1158+
// This is necessary to prevent false positive leaks due to the AsanThread
1159+
// holding the only live reference to a heap object. This can happen because
1160+
// the `pthread_create()` interceptor doesn't wait for the child thread to
1161+
// start before returning and thus loosing the the only live reference to the
1162+
// heap object on the stack.
1163+
1164+
__asan::AsanThreadContext *atctx =
1165+
reinterpret_cast<__asan::AsanThreadContext *>(tctx);
1166+
__asan::AsanThread *asan_thread = atctx->thread;
1167+
1168+
// Note ThreadStatusRunning is required because there is a small window where
1169+
// the thread status switches to `ThreadStatusRunning` but the `arg` pointer
1170+
// still isn't on the stack yet.
1171+
if (atctx->status != ThreadStatusCreated &&
1172+
atctx->status != ThreadStatusRunning)
1173+
return;
1174+
1175+
uptr thread_arg = reinterpret_cast<uptr>(asan_thread->get_arg());
1176+
if (!thread_arg)
1177+
return;
1178+
1179+
auto ptrsVec = reinterpret_cast<InternalMmapVector<uptr> *>(ptrs);
1180+
ptrsVec->push_back(thread_arg);
1181+
}
1182+
11561183
} // namespace __lsan
11571184

11581185
// ---------------------- Interface ---------------- {{{1

compiler-rt/lib/asan/asan_thread.cpp

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -518,41 +518,9 @@ void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
518518
fake_stack->ForEachFakeFrame(callback, arg);
519519
}
520520

521-
static void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs) {
522-
// Look for the arg pointer of threads that have been created or are running.
523-
// This is necessary to prevent false positive leaks due to the AsanThread
524-
// holding the only live reference to a heap object. This can happen because
525-
// the `pthread_create()` interceptor doesn't wait for the child thread to
526-
// start before returning and thus loosing the the only live reference to the
527-
// heap object on the stack.
528-
529-
__asan::AsanThreadContext *atctx =
530-
reinterpret_cast<__asan::AsanThreadContext *>(tctx);
531-
__asan::AsanThread *asan_thread = atctx->thread;
532-
533-
// Note ThreadStatusRunning is required because there is a small window where
534-
// the thread status switches to `ThreadStatusRunning` but the `arg` pointer
535-
// still isn't on the stack yet.
536-
if (atctx->status != ThreadStatusCreated &&
537-
atctx->status != ThreadStatusRunning)
538-
return;
539-
540-
uptr thread_arg = reinterpret_cast<uptr>(asan_thread->get_arg());
541-
if (!thread_arg)
542-
return;
543-
544-
auto ptrsVec = reinterpret_cast<InternalMmapVector<uptr> *>(ptrs);
545-
ptrsVec->push_back(thread_arg);
546-
}
547-
548-
void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {
549-
GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
550-
GetAdditionalThreadContextPtrs, ptrs);
551-
}
552-
553-
void ReportUnsuspendedThreadsLocked(InternalMmapVector<tid_t> *threads) {
554-
GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
555-
&ReportIfNotSuspended, threads);
521+
void RunCallbackForEachThreadLocked(__sanitizer::ThreadRegistry::ThreadCallback cb,
522+
void *arg) {
523+
GetAsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(cb, arg);
556524
}
557525

558526
void FinishThreadLocked(u32 tid) {

compiler-rt/lib/lsan/lsan_allocator.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ IgnoreObjectResult IgnoreObjectLocked(const void *p) {
319319
}
320320
}
321321

322-
void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs) {
322+
void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs) {
323323
// This function can be used to treat memory reachable from `tctx` as live.
324324
// This is useful for threads that have been created but not yet started.
325325

compiler-rt/lib/lsan/lsan_common.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ extern "C" SANITIZER_WEAK_ATTRIBUTE void __libc_iterate_dynamic_tls(
371371

372372
static void ProcessThreadRegistry(Frontier *frontier) {
373373
InternalMmapVector<uptr> ptrs;
374-
GetAdditionalThreadContextPtrsLocked(&ptrs);
374+
RunCallbackForEachThreadLocked(GetAdditionalThreadContextPtrs, &ptrs);
375375

376376
for (uptr i = 0; i < ptrs.size(); ++i) {
377377
void *ptr = reinterpret_cast<void *>(ptrs[i]);
@@ -668,7 +668,7 @@ void LeakSuppressionContext::PrintMatchedSuppressions() {
668668
Printf("%s\n\n", line);
669669
}
670670

671-
void ReportIfNotSuspended(ThreadContextBase *tctx, void *arg) {
671+
static void ReportIfNotSuspended(ThreadContextBase *tctx, void *arg) {
672672
const InternalMmapVector<tid_t> &suspended_threads =
673673
*(const InternalMmapVector<tid_t> *)arg;
674674
if (tctx->status == ThreadStatusRunning) {
@@ -695,7 +695,8 @@ static void ReportUnsuspendedThreads(
695695
threads[i] = suspended_threads.GetThreadID(i);
696696

697697
Sort(threads.data(), threads.size());
698-
ReportUnsuspendedThreadsLocked(&threads);
698+
699+
RunCallbackForEachThreadLocked(&ReportIfNotSuspended, &threads);
699700
}
700701

701702
# endif // !SANITIZER_FUCHSIA

compiler-rt/lib/lsan/lsan_common.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ bool GetThreadRangesLocked(tid_t os_id, uptr *stack_begin, uptr *stack_end,
105105
void GetAllThreadAllocatorCachesLocked(InternalMmapVector<uptr> *caches);
106106
void ForEachExtraStackRange(tid_t os_id, RangeIteratorCallback callback,
107107
void *arg);
108-
void GetAdditionalThreadContextPtrsLocked(InternalMmapVector<uptr> *ptrs);
109-
void ReportUnsuspendedThreadsLocked(InternalMmapVector<tid_t> *threads);
110-
void FinishThreadLocked(u32 tid);
108+
109+
void RunCallbackForEachThreadLocked(__sanitizer::ThreadRegistry::ThreadCallback cb,
110+
void *arg);
111111

112112
//// --------------------------------------------------------------------------
113113
//// Allocator prototypes.
@@ -146,6 +146,8 @@ void ForEachChunk(ForEachChunkCallback callback, void *arg);
146146
// Helper for __lsan_ignore_object().
147147
IgnoreObjectResult IgnoreObjectLocked(const void *p);
148148

149+
void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs);
150+
149151
// The rest of the LSan interface which is implemented by library.
150152

151153
struct ScopedStopTheWorldLock {
@@ -267,7 +269,6 @@ void DoLeakCheck();
267269
void DoRecoverableLeakCheckVoid();
268270
void DisableCounterUnderflow();
269271
bool DisabledInThisThread();
270-
void ReportIfNotSuspended(ThreadContextBase *tctx, void *arg);
271272

272273
// Used to implement __lsan::ScopedDisabler.
273274
void DisableInThisThread();

compiler-rt/lib/lsan/lsan_common_fuchsia.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
//===---------------------------------------------------------------------===//
1313

1414
#include "lsan_common.h"
15-
#include "lsan_thread.h"
1615
#include "sanitizer_common/sanitizer_platform.h"
1716

1817
#if CAN_SANITIZE_LEAKS && SANITIZER_FUCHSIA
@@ -147,7 +146,7 @@ void LockStuffAndStopTheWorld(StopTheWorldCallback callback,
147146
// just for the allocator cache, and to call ForEachExtraStackRange,
148147
// which ASan needs.
149148
if (flags()->use_stacks) {
150-
GetLsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
149+
RunCallbackForEachThreadLocked(
151150
[](ThreadContextBase *tctx, void *arg) {
152151
ForEachExtraStackRange(tctx->os_id, ForEachExtraStackRangeCb,
153152
arg);

compiler-rt/lib/lsan/lsan_fuchsia.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ void InitializeMainThread() {
6868
}
6969

7070
void GetAllThreadAllocatorCachesLocked(InternalMmapVector<uptr> *caches) {
71-
GetLsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
71+
RunCallbackForEachThreadLocked(
7272
[](ThreadContextBase *tctx, void *arg) {
7373
auto ctx = static_cast<ThreadContext *>(tctx);
7474
static_cast<decltype(caches)>(arg)->push_back(ctx->cache_begin());

compiler-rt/lib/lsan/lsan_thread.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ ThreadRegistry *GetLsanThreadRegistryLocked() {
8787
return thread_registry;
8888
}
8989

90-
void ReportUnsuspendedThreadsLocked(InternalMmapVector<tid_t> *threads) {
91-
GetLsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(
92-
&ReportIfNotSuspended, threads);
90+
void RunCallbackForEachThreadLocked(
91+
__sanitizer::ThreadRegistry::ThreadCallback cb, void *arg) {
92+
GetLsanThreadRegistryLocked()->RunCallbackForEachThreadLocked(cb, arg);
9393
}
9494

9595
} // namespace __lsan

0 commit comments

Comments
 (0)