[Libunwind] Try to fix msan failures #120013
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-libunwind Author: Dmitry Chestnykh (chestnykh) ChangesFull diff: https://github.com/llvm/llvm-project/pull/120013.diff 7 Files Affected:
diff --git a/libunwind/test/forceunwind.pass.cpp b/libunwind/test/forceunwind.pass.cpp
index 344034e1ea5f5e..e8333eb74a979a 100644
--- a/libunwind/test/forceunwind.pass.cpp
+++ b/libunwind/test/forceunwind.pass.cpp
@@ -9,9 +9,6 @@
// REQUIRES: linux
-// TODO: Figure out why this fails with Memory Sanitizer.
-// XFAIL: msan
-
// Basic test for _Unwind_ForcedUnwind.
// See libcxxabi/test/forced_unwind* tests too.
diff --git a/libunwind/test/libunwind_01.pass.cpp b/libunwind/test/libunwind_01.pass.cpp
index 838df6b5897204..82fb66d665c259 100644
--- a/libunwind/test/libunwind_01.pass.cpp
+++ b/libunwind/test/libunwind_01.pass.cpp
@@ -10,9 +10,6 @@
// TODO: Investigate this failure on x86_64 macOS back deployment
// XFAIL: stdlib=system && target=x86_64-apple-macosx{{10.9|10.10|10.11|10.12|10.13|10.14|10.15|11.0|12.0}}
-// TODO: Figure out why this fails with Memory Sanitizer.
-// XFAIL: msan
-
#include <libunwind.h>
#include <stdlib.h>
#include <stdio.h>
diff --git a/libunwind/test/libunwind_02.pass.cpp b/libunwind/test/libunwind_02.pass.cpp
index 9fd8e5d7159c96..5f2d2b43bddc1c 100644
--- a/libunwind/test/libunwind_02.pass.cpp
+++ b/libunwind/test/libunwind_02.pass.cpp
@@ -7,9 +7,6 @@
//
//===----------------------------------------------------------------------===//
-// TODO: Figure out why this fails with Memory Sanitizer.
-// XFAIL: msan
-
// This test fails on older llvm, when built with picolibc.
// XFAIL: clang-16 && LIBCXX-PICOLIBC-FIXME
diff --git a/libunwind/test/signal_frame.pass.cpp b/libunwind/test/signal_frame.pass.cpp
index 004029cfe1e90b..67b862c98fbfc7 100644
--- a/libunwind/test/signal_frame.pass.cpp
+++ b/libunwind/test/signal_frame.pass.cpp
@@ -12,9 +12,6 @@
// TODO: Investigate this failure on Apple
// XFAIL: target={{.+}}-apple-{{.+}}
-// TODO: Figure out why this fails with Memory Sanitizer.
-// XFAIL: msan
-
// UNSUPPORTED: libunwind-arm-ehabi
// The AIX assembler does not support CFI directives, which
diff --git a/libunwind/test/signal_unwind.pass.cpp b/libunwind/test/signal_unwind.pass.cpp
index 1c1566415a4d4b..8ba0c8b2859ac4 100644
--- a/libunwind/test/signal_unwind.pass.cpp
+++ b/libunwind/test/signal_unwind.pass.cpp
@@ -10,9 +10,6 @@
// Ensure that the unwinder can cope with the signal handler.
// REQUIRES: target={{(aarch64|riscv64|s390x|x86_64)-.+linux.*}}
-// TODO: Figure out why this fails with Memory Sanitizer.
-// XFAIL: msan
-
// Note: this test fails on musl because:
//
// (a) musl disables emission of unwind information for its build, and
diff --git a/libunwind/test/unw_resume.pass.cpp b/libunwind/test/unw_resume.pass.cpp
index 2b7470b5cad0eb..ca6068a828e0ac 100644
--- a/libunwind/test/unw_resume.pass.cpp
+++ b/libunwind/test/unw_resume.pass.cpp
@@ -10,9 +10,6 @@
// Ensure that unw_resume() resumes execution at the stack frame identified by
// cursor.
-// TODO: Figure out why this fails with Memory Sanitizer.
-// XFAIL: msan
-
#include <libunwind.h>
__attribute__((noinline)) void test_unw_resume() {
diff --git a/libunwind/test/unwind_leaffunction.pass.cpp b/libunwind/test/unwind_leaffunction.pass.cpp
index 98de7dc43260c2..4259406cc493d1 100644
--- a/libunwind/test/unwind_leaffunction.pass.cpp
+++ b/libunwind/test/unwind_leaffunction.pass.cpp
@@ -10,9 +10,6 @@
// Ensure that leaf function can be unwund.
// REQUIRES: target={{(aarch64|riscv64|s390x|x86_64)-.+linux.*}}
-// TODO: Figure out why this fails with Memory Sanitizer.
-// XFAIL: msan
-
// Note: this test fails on musl because:
//
// (a) musl disables emission of unwind information for its build, and
|
Contributor
Author
|
@ldionne i run these tests locally with msan (x86-64) and have no errors. Could you please tell on what architecture(s) the tests were failed? |
Contributor
Author
|
All CI failures look unrelated |
ldionne
force-pushed
the
msan-dontxfail
branch
from
95d35c8 to
51c5f79
Compare
Member
|
Rebased onto I don't remember why these |
chestnykh
force-pushed
the
msan-dontxfail
branch
from
51c5f79 to
9422b09
Compare
chestnykh
changed the title
Contributor
Author
|
Maybe load order breaks something: With only |
The possible reason why tests fail is that MAYBE there are another libunwind on the testing host which is loaded(?) instead of libunwind from llvm. clang passes 'right' libunwind (-lunwind) to the linker if -unwindlib=libunwind is passed to the driver
This reverts commit 0a4a0e1.
Contributor
Author
|
I've debugged msan errors. On CI there were stack overflows caused by 'wrong' msan reports and circular calls libunwind -> libmsan -> libunwind. For example: There were also regular msan reports fixed by modifying tests srcs to add explicit initialization of unw_cursor_t and unw_context_t variables: |
Sometimes msan seems to fail proving that variable is (un)initialized and emits false-positives. Such false positives can lead to stack overflow (libc++ CI error) So add explicit initialization of all variables that msan thinks are used without initialization
arichardson
requested changes
Dec 17, 2024
Member
arichardson
left a comment
arichardson
left a comment
There was a problem hiding this comment.
We shouldn't need to initialize these structures to silence sanitizer errors. I had a draft pr last year to fix this properly but forgot about it since other things came up: #67860
I'd prefer something like that. Alternatively I can update my change once I'm back in the office in January.
| unw_cursor_t cursor; | ||
| unw_context_t uc; | ||
| #if __has_feature(memory_sanitizer) | ||
| __builtin_memset(&cursor, 0, sizeof(cursor)); |
Member
There was a problem hiding this comment.
I'm not keen on working around the missing msan annotations on every call. This seems fragile.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.