[compiler-rt] 39db491 - [LeakSanitizer] Capture calling thread SP early to avoid false negatives.
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 12 16:46:54 PDT 2022
Author: Wiktor Garbacz
Date: 2022-10-12T16:46:32-07:00
New Revision: 39db491957dcf095936d81bed89c2b4edae2a1e7
URL: https://github.com/llvm/llvm-project/commit/39db491957dcf095936d81bed89c2b4edae2a1e7
DIFF: https://github.com/llvm/llvm-project/commit/39db491957dcf095936d81bed89c2b4edae2a1e7.diff
LOG: [LeakSanitizer] Capture calling thread SP early to avoid false negatives.
As shown in https://github.com/llvm/llvm-project/issues/42932 dead
pointers might be overlapped by a new stack frame inside CheckForLeaks,
which does not use bytes with pointers. This leads to false negatives.
It's not a full solution for the problem as it does not solve
"overlapping" new/old frames for frames below the CheckForLeaks and in
other threads. It should improve leaks found in direct callers of
__lsan_do_leak_check.
Differential Revision: https://reviews.llvm.org/D130237
Added:
Modified:
compiler-rt/lib/lsan/lsan_common.cpp
compiler-rt/lib/lsan/lsan_common.h
Removed:
################################################################################
diff --git a/compiler-rt/lib/lsan/lsan_common.cpp b/compiler-rt/lib/lsan/lsan_common.cpp
index ade0b8081f276..51218770d6dcc 100644
--- a/compiler-rt/lib/lsan/lsan_common.cpp
+++ b/compiler-rt/lib/lsan/lsan_common.cpp
@@ -391,7 +391,8 @@ static void ProcessThreadRegistry(Frontier *frontier) {
// Scans thread data (stacks and TLS) for heap pointers.
static void ProcessThreads(SuspendedThreadsList const &suspended_threads,
- Frontier *frontier) {
+ Frontier *frontier, tid_t caller_tid,
+ uptr caller_sp) {
InternalMmapVector<uptr> registers;
for (uptr i = 0; i < suspended_threads.ThreadCount(); i++) {
tid_t os_id = static_cast<tid_t>(suspended_threads.GetThreadID(i));
@@ -418,6 +419,9 @@ static void ProcessThreads(SuspendedThreadsList const &suspended_threads,
continue;
sp = stack_begin;
}
+ if (suspended_threads.GetThreadID(i) == caller_tid) {
+ sp = caller_sp;
+ }
if (flags()->use_registers && have_registers) {
uptr registers_begin = reinterpret_cast<uptr>(registers.data());
@@ -598,7 +602,8 @@ static void CollectIgnoredCb(uptr chunk, void *arg) {
// Sets the appropriate tag on each chunk.
static void ClassifyAllChunks(SuspendedThreadsList const &suspended_threads,
- Frontier *frontier) {
+ Frontier *frontier, tid_t caller_tid,
+ uptr caller_sp) {
const InternalMmapVector<u32> &suppressed_stacks =
GetSuppressionContext()->GetSortedSuppressedStacks();
if (!suppressed_stacks.empty()) {
@@ -607,7 +612,7 @@ static void ClassifyAllChunks(SuspendedThreadsList const &suspended_threads,
}
ForEachChunk(CollectIgnoredCb, frontier);
ProcessGlobalRegions(frontier);
- ProcessThreads(suspended_threads, frontier);
+ ProcessThreads(suspended_threads, frontier, caller_tid, caller_sp);
ProcessRootRegions(frontier);
FloodFillTag(frontier, kReachable);
@@ -703,7 +708,8 @@ static void CheckForLeaksCallback(const SuspendedThreadsList &suspended_threads,
CHECK(param);
CHECK(!param->success);
ReportUnsuspendedThreads(suspended_threads);
- ClassifyAllChunks(suspended_threads, ¶m->frontier);
+ ClassifyAllChunks(suspended_threads, ¶m->frontier, param->caller_tid,
+ param->caller_sp);
ForEachChunk(CollectLeaksCb, ¶m->leaks);
// Clean up for subsequent leak checks. This assumes we did not overwrite any
// kIgnored tags.
@@ -742,6 +748,12 @@ static bool CheckForLeaks() {
for (int i = 0;; ++i) {
EnsureMainThreadIDIsCorrect();
CheckForLeaksParam param;
+ // Capture calling thread's stack pointer early, to avoid false negatives.
+ // Old frame with dead pointers might be overlapped by new frame inside
+ // CheckForLeaks which does not use bytes with pointers before the
+ // threads are suspended and stack pointers captured.
+ param.caller_tid = GetTid();
+ param.caller_sp = reinterpret_cast<uptr>(__builtin_frame_address(0));
LockStuffAndStopTheWorld(CheckForLeaksCallback, ¶m);
if (!param.success) {
Report("LeakSanitizer has encountered a fatal error.\n");
diff --git a/compiler-rt/lib/lsan/lsan_common.h b/compiler-rt/lib/lsan/lsan_common.h
index d7153751faee0..20ef7c458b42a 100644
--- a/compiler-rt/lib/lsan/lsan_common.h
+++ b/compiler-rt/lib/lsan/lsan_common.h
@@ -145,6 +145,8 @@ struct RootRegion {
struct CheckForLeaksParam {
Frontier frontier;
LeakedChunks leaks;
+ tid_t caller_tid;
+ uptr caller_sp;
bool success = false;
};
More information about the llvm-commits
mailing list