[libunwind] [Libunwind] Try to fix msan failures (PR #120013)

Dmitry Chestnykh via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 17 00:53:48 PST 2024


https://github.com/chestnykh updated https://github.com/llvm/llvm-project/pull/120013

>From 9422b097ff80f5501dcf4fa3d41176b9439ab3f6 Mon Sep 17 00:00:00 2001
From: Dmitry Chestnykh <dm.chestnykh at gmail.com>
Date: Sun, 15 Dec 2024 21:18:23 +0300
Subject: [PATCH 1/4] [Libunwind] Don't XFAIL tests with msan

---
 libunwind/test/forceunwind.pass.cpp         | 3 ---
 libunwind/test/libunwind_01.pass.cpp        | 3 ---
 libunwind/test/libunwind_02.pass.cpp        | 3 ---
 libunwind/test/signal_frame.pass.cpp        | 3 ---
 libunwind/test/signal_unwind.pass.cpp       | 3 ---
 libunwind/test/unw_resume.pass.cpp          | 3 ---
 libunwind/test/unwind_leaffunction.pass.cpp | 3 ---
 7 files changed, 21 deletions(-)

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

>From 0a4a0e12d7b4daf72dd4461962fbf6cc5114347f Mon Sep 17 00:00:00 2001
From: Dmitry Chestnykh <dm.chestnykh at gmail.com>
Date: Mon, 16 Dec 2024 21:40:46 +0300
Subject: [PATCH 2/4] Try to remove -lunwind from link flags

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
---
 libunwind/test/configs/llvm-libunwind-shared.cfg.in | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libunwind/test/configs/llvm-libunwind-shared.cfg.in b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
index f3e40928b525da..256f8111948bd3 100644
--- a/libunwind/test/configs/llvm-libunwind-shared.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
@@ -23,6 +23,10 @@ if '@CMAKE_DL_LIBS@':
 # Stack unwinding tests need unwinding tables and these are not generated by default on all targets.
 compile_flags.append('-funwind-tables')
 
+# Use libunwind from llvm
+compile_flags.append('-rtlib=compiler-rt')
+compile_flags.append('-unwindlib=libunwind')
+
 local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format(local_sysroot) if local_sysroot else ''
@@ -31,7 +35,7 @@ config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{{include}} {}'.format(' '.join(compile_flags))
 ))
 config.substitutions.append(('%{link_flags}',
-    '-L %{{lib}} -Wl,-rpath,%{{lib}} -lunwind {}'.format(' '.join(link_flags))
+    '-L %{{lib}} -Wl,-rpath,%{{lib}} {}'.format(' '.join(link_flags))
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T -- '

>From f90ca87ece39c536d6f09e9ee249a487d5daa049 Mon Sep 17 00:00:00 2001
From: Dmitry Chestnykh <dm.chestnykh at gmail.com>
Date: Tue, 17 Dec 2024 10:32:11 +0300
Subject: [PATCH 3/4] Revert "Try to remove -lunwind from link flags"

This reverts commit 0a4a0e12d7b4daf72dd4461962fbf6cc5114347f.
---
 libunwind/test/configs/llvm-libunwind-shared.cfg.in | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/libunwind/test/configs/llvm-libunwind-shared.cfg.in b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
index 256f8111948bd3..f3e40928b525da 100644
--- a/libunwind/test/configs/llvm-libunwind-shared.cfg.in
+++ b/libunwind/test/configs/llvm-libunwind-shared.cfg.in
@@ -23,10 +23,6 @@ if '@CMAKE_DL_LIBS@':
 # Stack unwinding tests need unwinding tables and these are not generated by default on all targets.
 compile_flags.append('-funwind-tables')
 
-# Use libunwind from llvm
-compile_flags.append('-rtlib=compiler-rt')
-compile_flags.append('-unwindlib=libunwind')
-
 local_sysroot = '@CMAKE_OSX_SYSROOT@' or '@CMAKE_SYSROOT@'
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format(local_sysroot) if local_sysroot else ''
@@ -35,7 +31,7 @@ config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -I %{{include}} {}'.format(' '.join(compile_flags))
 ))
 config.substitutions.append(('%{link_flags}',
-    '-L %{{lib}} -Wl,-rpath,%{{lib}} {}'.format(' '.join(link_flags))
+    '-L %{{lib}} -Wl,-rpath,%{{lib}} -lunwind {}'.format(' '.join(link_flags))
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T -- '

>From a94ecd3b7fd9631b04ee3f1fad816abd08e18ba0 Mon Sep 17 00:00:00 2001
From: Dmitry Chestnykh <dm.chestnykh at gmail.com>
Date: Tue, 17 Dec 2024 11:53:24 +0300
Subject: [PATCH 4/4] [Libunwind] Fix msan-related errors

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
---
 libunwind/src/AddressSpace.hpp       |  3 +++
 libunwind/src/UnwindLevel1-gcc-ext.c |  4 ++++
 libunwind/src/UnwindLevel1.c         |  4 ++++
 libunwind/test/libunwind_01.pass.cpp | 18 +++++++++---------
 libunwind/test/signal_frame.pass.cpp |  4 ++--
 libunwind/test/unw_resume.pass.cpp   |  4 ++--
 6 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp
index 5551c7d4bef1c5..8a337dac9f4f58 100644
--- a/libunwind/src/AddressSpace.hpp
+++ b/libunwind/src/AddressSpace.hpp
@@ -621,6 +621,9 @@ inline bool LocalAddressSpace::findUnwindSections(pint_t targetAddr,
   }
   // Try to find the unwind info using `dl_find_object`
   dl_find_object findResult;
+#if __has_feature(memory_sanitizer)
+  __builtin_memset(&findResult, 0, sizeof(dl_find_object));
+#endif
   if (dlFindObject && dlFindObject((void *)targetAddr, &findResult) == 0) {
     if (findResult.dlfo_eh_frame == nullptr) {
       // Found an entry for `targetAddr`, but there is no unwind info.
diff --git a/libunwind/src/UnwindLevel1-gcc-ext.c b/libunwind/src/UnwindLevel1-gcc-ext.c
index 32c872ffade1fd..c906ce263965c9 100644
--- a/libunwind/src/UnwindLevel1-gcc-ext.c
+++ b/libunwind/src/UnwindLevel1-gcc-ext.c
@@ -133,6 +133,10 @@ _LIBUNWIND_EXPORT _Unwind_Reason_Code
 _Unwind_Backtrace(_Unwind_Trace_Fn callback, void *ref) {
   unw_cursor_t cursor;
   unw_context_t uc;
+#if __has_feature(memory_sanitizer)
+  __builtin_memset(&cursor, 0, sizeof(cursor));
+  __builtin_memset(&uc, 0, sizeof(uc));
+#endif
   __unw_getcontext(&uc);
   __unw_init_local(&cursor, &uc);
 
diff --git a/libunwind/src/UnwindLevel1.c b/libunwind/src/UnwindLevel1.c
index 7e785f4d31e716..e63f22906f5d6f 100644
--- a/libunwind/src/UnwindLevel1.c
+++ b/libunwind/src/UnwindLevel1.c
@@ -507,6 +507,10 @@ _Unwind_ForcedUnwind(_Unwind_Exception *exception_object,
                        (void *)exception_object, (void *)(uintptr_t)stop);
   unw_context_t uc;
   unw_cursor_t cursor;
+#if __has_feature(memory_sanitizer)
+  __builtin_memset(&uc, 0, sizeof(uc));
+  __builtin_memset(&cursor, 0, sizeof(cursor));
+#endif
   __unw_getcontext(&uc);
 
   // Mark that this is a forced unwind, so _Unwind_Resume() can do
diff --git a/libunwind/test/libunwind_01.pass.cpp b/libunwind/test/libunwind_01.pass.cpp
index 82fb66d665c259..d5fd7fbbf05f7b 100644
--- a/libunwind/test/libunwind_01.pass.cpp
+++ b/libunwind/test/libunwind_01.pass.cpp
@@ -16,10 +16,10 @@
 #include <string.h>
 
 void backtrace(int lower_bound) {
-  unw_context_t context;
+  unw_context_t context = {0};
   unw_getcontext(&context);
 
-  unw_cursor_t cursor;
+  unw_cursor_t cursor = {0};
   unw_init_local(&cursor, &context);
 
   char buffer[1024];
@@ -64,10 +64,10 @@ __attribute__((noinline)) void test3(int i, int j, int k) {
 }
 
 void test_no_info() {
-  unw_context_t context;
+  unw_context_t context = {0};
   unw_getcontext(&context);
 
-  unw_cursor_t cursor;
+  unw_cursor_t cursor = {0};
   unw_init_local(&cursor, &context);
 
   unw_proc_info_t info;
@@ -84,10 +84,10 @@ void test_no_info() {
 }
 
 void test_reg_names() {
-  unw_context_t context;
+  unw_context_t context = {0};
   unw_getcontext(&context);
 
-  unw_cursor_t cursor;
+  unw_cursor_t cursor = {0};
   unw_init_local(&cursor, &context);
 
   int max_reg_num = -100;
@@ -110,7 +110,7 @@ void test_reg_names() {
 
 #if defined(__x86_64__)
 void test_reg_get_set() {
-  unw_context_t context;
+  unw_context_t context = {0};
   unw_getcontext(&context);
 
   unw_cursor_t cursor;
@@ -131,10 +131,10 @@ void test_reg_get_set() {
 }
 
 void test_fpreg_get_set() {
-  unw_context_t context;
+  unw_context_t context = {0};
   unw_getcontext(&context);
 
-  unw_cursor_t cursor;
+  unw_cursor_t cursor = {0};
   unw_init_local(&cursor, &context);
 
   // get/set is not implemented for x86_64 fpregs.
diff --git a/libunwind/test/signal_frame.pass.cpp b/libunwind/test/signal_frame.pass.cpp
index 67b862c98fbfc7..2882b3fbf264b3 100644
--- a/libunwind/test/signal_frame.pass.cpp
+++ b/libunwind/test/signal_frame.pass.cpp
@@ -29,8 +29,8 @@
 
 void test() {
   asm(".cfi_signal_frame");
-  unw_cursor_t cursor;
-  unw_context_t uc;
+  unw_cursor_t cursor = {0};
+  unw_context_t uc = {0};
   unw_getcontext(&uc);
   unw_init_local(&cursor, &uc);
   assert(unw_step(&cursor) > 0);
diff --git a/libunwind/test/unw_resume.pass.cpp b/libunwind/test/unw_resume.pass.cpp
index ca6068a828e0ac..45f34ac69ca096 100644
--- a/libunwind/test/unw_resume.pass.cpp
+++ b/libunwind/test/unw_resume.pass.cpp
@@ -13,8 +13,8 @@
 #include <libunwind.h>
 
 __attribute__((noinline)) void test_unw_resume() {
-  unw_context_t context;
-  unw_cursor_t cursor;
+  unw_context_t context = {0};
+  unw_cursor_t cursor = {0};
 
   unw_getcontext(&context);
   unw_init_local(&cursor, &context);



More information about the cfe-commits mailing list