[libunwind] [libunwind] Fix running tests with MSan (PR #67860)
Alexander Richardson via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 2 09:03:14 PDT 2023
https://github.com/arichardson updated https://github.com/llvm/llvm-project/pull/67860
>From e39c568ba557fbca5d3ef17f3d4f4d74f15e8205 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Fri, 29 Sep 2023 13:25:39 -0700
Subject: [PATCH] [libunwind] Fix running tests with MSan
In order to run tests with MSan we have silence two false-positives:
First, we have to tell the runtime that __unw_getcontext actually populates
the buffer with initialized data.
Ideally we would call __msan_unpoison directly from __unw_getcontext, but
this would require adding assembly calls for all possible configurations
which is rather error prone. We also can't implement __unw_getcontext as
a C function that calls into the assembly code since that would result
in potentially incorrect register values being saved. Instead, use a
wrapper macro that calls __msan_unpoison directly after __unw_getcontext.
And second, MSan does not intercept _dl_find_object, so we initialize
the struct to zero before calling _dl_find_object.
These two changes are sufficient to make the whole test suite pass my
x86_64 Linux system.
---
libunwind/include/libunwind.h | 18 ++++++++++++++++++
libunwind/src/AddressSpace.hpp | 3 +++
libunwind/src/libunwind_ext.h | 10 ++++++++++
libunwind/test/bad_unwind_info.pass.cpp | 3 ---
libunwind/test/forceunwind.pass.cpp | 3 ---
libunwind/test/libunwind_01.pass.cpp | 3 ---
libunwind/test/libunwind_02.pass.cpp | 3 ---
libunwind/test/remember_state_leak.pass.sh.s | 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 ---
12 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/libunwind/include/libunwind.h b/libunwind/include/libunwind.h
index b2dae8feed9a3b5..a1b02c07a2c79e3 100644
--- a/libunwind/include/libunwind.h
+++ b/libunwind/include/libunwind.h
@@ -43,6 +43,12 @@
#define LIBUNWIND_AVAIL
#endif
+#if defined(__SANITIZE_MEMORY__) || \
+ (defined(__has_feature) && __has_feature(memory_sanitizer))
+#define LIBUNWIND_HAVE_MSAN
+#include <sanitizer/msan_interface.h>
+#endif
+
#if defined(_WIN32) && defined(__SEH__)
#define LIBUNWIND_CURSOR_ALIGNMENT_ATTR __attribute__((__aligned__(16)))
#else
@@ -115,6 +121,18 @@ extern int unw_set_reg(unw_cursor_t *, unw_regnum_t, unw_word_t) LIBUNWIND_AVAIL
extern int unw_set_fpreg(unw_cursor_t *, unw_regnum_t, unw_fpreg_t) LIBUNWIND_AVAIL;
extern int unw_resume(unw_cursor_t *) LIBUNWIND_AVAIL;
+#ifdef LIBUNWIND_HAVE_MSAN
+// unw_getcontext is implemented in assembly so it is rather difficult to
+// mark the MSan shadow as initialized from within the function. Instead we
+// use a macro wrapper when compiling with MSan to avoid false-positives.
+#define unw_getcontext(context) \
+ ({ \
+ int _result = (unw_getcontext)(context); \
+ __msan_unpoison((context), sizeof(unw_context_t)); \
+ _result; \
+ })
+#endif
+
#ifdef __arm__
/* Save VFP registers in FSTMX format (instead of FSTMD). */
extern void unw_save_vfp_as_X(unw_cursor_t *) LIBUNWIND_AVAIL;
diff --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp
index 1abbc822546878d..f3e625037a64c57 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;
+ // _dl_find_object should fully initialize this struct, but we zero it to
+ // be safe in case this is not true (and to keep MSan quite).
+ memset(&findResult, 0, sizeof(findResult));
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/libunwind_ext.h b/libunwind/src/libunwind_ext.h
index 28db43a4f6eef20..d55d906ea1a066c 100644
--- a/libunwind/src/libunwind_ext.h
+++ b/libunwind/src/libunwind_ext.h
@@ -32,6 +32,16 @@ extern int __unw_set_reg(unw_cursor_t *, unw_regnum_t, unw_word_t);
extern int __unw_set_fpreg(unw_cursor_t *, unw_regnum_t, unw_fpreg_t);
extern int __unw_resume(unw_cursor_t *);
+#ifdef LIBUNWIND_HAVE_MSAN
+// See comment above unw_getcontext (libunwind.h) for why this is needed.
+#define __unw_getcontext(context) \
+ ({ \
+ int _result = (__unw_getcontext)(context); \
+ __msan_unpoison((context), sizeof(unw_context_t)); \
+ _result; \
+ })
+#endif
+
#ifdef __arm__
/* Save VFP registers in FSTMX format (instead of FSTMD). */
extern void __unw_save_vfp_as_X(unw_cursor_t *);
diff --git a/libunwind/test/bad_unwind_info.pass.cpp b/libunwind/test/bad_unwind_info.pass.cpp
index b3284e8daed71f3..b2c6b03aa25dd9b 100644
--- a/libunwind/test/bad_unwind_info.pass.cpp
+++ b/libunwind/test/bad_unwind_info.pass.cpp
@@ -15,9 +15,6 @@
// GCC doesn't support __attribute__((naked)) on AArch64.
// UNSUPPORTED: gcc
-// Inline assembly is incompatible with MSAN.
-// UNSUPPORTED: msan
-
#undef NDEBUG
#include <assert.h>
#include <libunwind.h>
diff --git a/libunwind/test/forceunwind.pass.cpp b/libunwind/test/forceunwind.pass.cpp
index 8c26551b6d0b678..319d49011bfdb95 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 96f12d1fc393743..f6617d0beb8bbb7 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=apple-libc++ && 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 fc034378781a2fa..f91432990c7b4b2 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
-
#undef NDEBUG
#include <assert.h>
#include <stdlib.h>
diff --git a/libunwind/test/remember_state_leak.pass.sh.s b/libunwind/test/remember_state_leak.pass.sh.s
index 63beb7e4701ec0b..47566ba0d3ea906 100644
--- a/libunwind/test/remember_state_leak.pass.sh.s
+++ b/libunwind/test/remember_state_leak.pass.sh.s
@@ -8,9 +8,6 @@
# REQUIRES: target={{x86_64-.+-linux-gnu}}
-# Inline assembly isn't supported by Memory Sanitizer
-# UNSUPPORTED: msan
-
# RUN: %{build} -no-pie
# RUN: %{run}
diff --git a/libunwind/test/signal_frame.pass.cpp b/libunwind/test/signal_frame.pass.cpp
index e5409f6ce3d9973..d60f932207482dd 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 954a5d4ba3db10d..fd2180415c743f2 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
-
#undef NDEBUG
#include <assert.h>
#include <dlfcn.h>
diff --git a/libunwind/test/unw_resume.pass.cpp b/libunwind/test/unw_resume.pass.cpp
index 08e8d4edeaf2927..0bf264baf6762e6 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>
void test_unw_resume() {
diff --git a/libunwind/test/unwind_leaffunction.pass.cpp b/libunwind/test/unwind_leaffunction.pass.cpp
index 8c9912e3c38680b..330dc2fd4e627b8 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
-
#undef NDEBUG
#include <assert.h>
#include <dlfcn.h>
More information about the cfe-commits
mailing list