Skip to content

Conversation

@MaskRay
Copy link
Member

@MaskRay MaskRay commented Nov 17, 2024

Certain tests (many are from lld/test) run ... '2>&1 | count 0 to
ensure that there is no stderr message.

GetRegistersAndSP may rarely fail, leading to
a spurious failure like (with a local hack to make count dump the input):

+ /home/ray/llvm/out/asan/bin/ld.lld func1-gcs.o func2-gcs.o func3-gcs.o -o /dev/null -z gcs-report=warning -z gcs=never
+ /home/ray/llvm/out/asan/bin/count 0
Expected 0 lines, got 1.
==2403039==Unable to get registers from thread 2403018.

The failure can reliably be reproduced by running ninja check-lld a
few times under asan+lsan (see the bot
sanitizer-x86_64-linux-bootstrap-asan).

After #112609 we started to print errors for threads not in the registry.
Message is unnecessary as we skip such threads anyway.

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Fangrui Song (MaskRay)

Changes

Certain tests (many are from lld/test) run ... '2>&1 | count 0 to
ensure that there is no stderr message.

GetRegistersAndSP may rarely fail, leading to
a spurious failure like (with a local hack to make count dump the input):

+ /home/ray/llvm/out/asan/bin/ld.lld func1-gcs.o func2-gcs.o func3-gcs.o -o /dev/null -z gcs-report=warning -z gcs=never
+ /home/ray/llvm/out/asan/bin/count 0
Expected 0 lines, got 1.
==2403039==Unable to get registers from thread 2403018.

The failure can reliably be reproduced by running ninja check-lld a
few times under asan+lsan (see the bot
sanitizer-x86_64-linux-bootstrap-asan).
Since registers as the root set aren't very useful anyway, just switch
to VReport instead.


Full diff: https://github.com/llvm/llvm-project/pull/116555.diff

1 Files Affected:

  • (modified) compiler-rt/lib/lsan/lsan_common.cpp (+1-1)
diff --git a/compiler-rt/lib/lsan/lsan_common.cpp b/compiler-rt/lib/lsan/lsan_common.cpp
index 5c44c000ae577b..7ab9e4ff2ac9fd 100644
--- a/compiler-rt/lib/lsan/lsan_common.cpp
+++ b/compiler-rt/lib/lsan/lsan_common.cpp
@@ -569,7 +569,7 @@ static void ProcessThreads(SuspendedThreadsList const &suspended_threads,
     PtraceRegistersStatus have_registers =
         suspended_threads.GetRegistersAndSP(i, &registers, &sp);
     if (have_registers != REGISTERS_AVAILABLE) {
-      Report("Unable to get registers from thread %llu.\n", os_id);
+      VReport(1, "Unable to get registers from thread %llu.\n", os_id);
       // If unable to get SP, consider the entire stack to be reachable unless
       // GetRegistersAndSP failed with ESRCH.
       if (have_registers == REGISTERS_UNAVAILABLE_FATAL)

@vitalybuka
Copy link
Collaborator

vitalybuka commented Nov 19, 2024

Since registers as the root set aren't very useful anyway, just switch
to VReport instead.

They are critical to avoid false leak reports, and this condition means LSAN is broken (or thread is somehow special).
Thread can be suspended in any moment. A pointer to just allocated memory can be in register only.
It would be nice to understand how we get there.

@vitalybuka
Copy link
Collaborator

I made changes to this function recently. I will try to reproduce and understand the cause.

@vitalybuka
Copy link
Collaborator

I made changes to this function recently. I will try to reproduce and understand the cause.

This is from #112609

Other than Report #112609 is NFC. Before the patch we returned earlier after failed registry search.
So I confirm that this is not caused by LSAN regression, and safe to land.

@vitalybuka vitalybuka merged commit ac38ab5 into main Nov 20, 2024
11 checks passed
@vitalybuka vitalybuka deleted the users/MaskRay/spr/lsan-use-vreport-if-not-registers_available branch November 20, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants