[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
krytarowski added a comment. This looks like broke ASan on NetBSD: $ sh ./projects/compiler-rt/test/sanitizer_common/asan-i386-NetBSD/NetBSD/Output/ttyent.cc.script /usr/lib/i386/libgcc.a(unwind-dw2.o): In function `_Unwind_RaiseException': unwind-dw2.c:(.text+0x1b41): multiple definition of `_Unwind_RaiseException' /public/llvm-build/lib/clang/7.0.0/lib/netbsd/libclang_rt.asan-i386.a(asan_interceptors.cc.o):/public/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:337: first defined here clang-7.0: error: linker command failed with exit code 1 (use -v to see invocation) Investigating. Repository: rCRT Compiler Runtime https://reviews.llvm.org/D42644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCRT326132: [asan] Intercept std::rethrow_exception indirectly
(authored by vitalybuka, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D42644?vs=132990&id=135965#toc
Repository:
rCRT Compiler Runtime
https://reviews.llvm.org/D42644
Files:
lib/asan/asan_interceptors.cc
lib/asan/asan_interceptors.h
test/asan/TestCases/intercept-rethrow-exception.cc
Index: test/asan/TestCases/intercept-rethrow-exception.cc
===
--- test/asan/TestCases/intercept-rethrow-exception.cc
+++ test/asan/TestCases/intercept-rethrow-exception.cc
@@ -0,0 +1,64 @@
+// Regression test for
+// https://bugs.llvm.org/show_bug.cgi?id=32434
+
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %run %t
+
+#include
+#include
+#include
+
+namespace {
+
+// Not instrumented because std::rethrow_exception is a [[noreturn]] function,
+// for which the compiler would emit a call to __asan_handle_no_return which
+// unpoisons the stack.
+// We emulate here some code not compiled with asan. This function is not
+// [[noreturn]] because the scenario we're emulating doesn't always throw. If it
+// were [[noreturn]], the calling code would emit a call to
+// __asan_handle_no_return.
+void __attribute__((no_sanitize("address")))
+uninstrumented_rethrow_exception(std::exception_ptr const &exc_ptr) {
+ std::rethrow_exception(exc_ptr);
+}
+
+char *poisoned1;
+char *poisoned2;
+
+// Create redzones for stack variables in shadow memory and call
+// std::rethrow_exception which should unpoison the entire stack.
+void create_redzones_and_throw(std::exception_ptr const &exc_ptr) {
+ char a[100];
+ poisoned1 = a - 1;
+ poisoned2 = a + sizeof(a);
+ assert(__asan_address_is_poisoned(poisoned1));
+ assert(__asan_address_is_poisoned(poisoned2));
+ uninstrumented_rethrow_exception(exc_ptr);
+}
+
+} // namespace
+
+// Check that std::rethrow_exception is intercepted by asan and the interception
+// unpoisons the stack.
+// If std::rethrow_exception is NOT intercepted, then calls to this function
+// from instrumented code will still unpoison the stack because
+// std::rethrow_exception is a [[noreturn]] function and any [[noreturn]]
+// function call will be instrumented with __asan_handle_no_return.
+// However, calls to std::rethrow_exception from UNinstrumented code will not
+// unpoison the stack, so we need to intercept std::rethrow_exception to
+// unpoison the stack.
+int main() {
+ // In some implementations of std::make_exception_ptr, e.g. libstdc++ prior to
+ // gcc 7, this function calls __cxa_throw. The __cxa_throw is intercepted by
+ // asan to unpoison the entire stack; since this test essentially tests that
+ // the stack is unpoisoned by a call to std::rethrow_exception, we need to
+ // generate the exception_ptr BEFORE we have the local variables poison the
+ // stack.
+ std::exception_ptr my_exception_ptr = std::make_exception_ptr("up");
+
+ try {
+create_redzones_and_throw(my_exception_ptr);
+ } catch(char const *) {
+assert(!__asan_region_is_poisoned(poisoned1, poisoned2 - poisoned1 + 1));
+ }
+}
Index: lib/asan/asan_interceptors.cc
===
--- lib/asan/asan_interceptors.cc
+++ lib/asan/asan_interceptors.cc
@@ -33,6 +33,11 @@
#include "sanitizer_common/sanitizer_posix.h"
#endif
+#if ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION || \
+ASAN_INTERCEPT__SJLJ_UNWIND_RAISEEXCEPTION
+#include
+#endif
+
#if defined(__i386) && SANITIZER_LINUX
#define ASAN_PTHREAD_CREATE_VERSION "GLIBC_2.1"
#elif defined(__mips__) && SANITIZER_LINUX
@@ -319,6 +324,32 @@
}
#endif
+#if ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION
+INTERCEPTOR(void, __cxa_rethrow_primary_exception, void *a) {
+ CHECK(REAL(__cxa_rethrow_primary_exception));
+ __asan_handle_no_return();
+ REAL(__cxa_rethrow_primary_exception)(a);
+}
+#endif
+
+#if ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION
+INTERCEPTOR(_Unwind_Reason_Code, _Unwind_RaiseException,
+struct _Unwind_Exception *object) {
+ CHECK(REAL(_Unwind_RaiseException));
+ __asan_handle_no_return();
+ return REAL(_Unwind_RaiseException)(object);
+}
+#endif
+
+#if ASAN_INTERCEPT__SJLJ_UNWIND_RAISEEXCEPTION
+INTERCEPTOR(_Unwind_Reason_Code, _Unwind_SjLj_RaiseException,
+struct _Unwind_Exception *object) {
+ CHECK(REAL(_Unwind_SjLj_RaiseException));
+ __asan_handle_no_return();
+ return REAL(_Unwind_SjLj_RaiseException)(object);
+}
+#endif
+
#if ASAN_INTERCEPT_INDEX
# if ASAN_USE_ALIAS_ATTRIBUTE_FOR_INDEX
INTERCEPTOR(char*, index, const char *string, int c)
@@ -599,6 +630,17 @@
#if ASAN_INTERCEPT___CXA_THROW
ASAN_INTERCEPT_FUNC(__cxa_throw);
#endif
+#if ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION
+ ASAN_INTERCEPT_FUNC(__cxa_rethrow_primary_exception);
+#endif
+ // Indirectly intercept std::rethrow_exception.
+#if ASAN_INTERCEPT__UNWIND_R
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
vitalybuka added inline comments.
Comment at: test/asan/TestCases/intercept-rethrow-exception.cc:48
+ // memcpy is intercepted by asan which performs checks on src and dst
+ using T = int[1000];
+ T x {};
robot wrote:
> vitalybuka wrote:
> > You can include #include
> > and use __asan_region_is_poisoned
> We don't have commit rights, can you commit it? Should we first change to
> `__asan_region_is_poisoned` and kill the program if it is indeed poisoined?
Sure, I'll update and commit this for you.
Repository:
rCRT Compiler Runtime
https://reviews.llvm.org/D42644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
robot added inline comments.
Comment at: test/asan/TestCases/intercept-rethrow-exception.cc:48
+ // memcpy is intercepted by asan which performs checks on src and dst
+ using T = int[1000];
+ T x {};
vitalybuka wrote:
> You can include #include
> and use __asan_region_is_poisoned
We don't have commit rights, can you commit it? Should we first change to
`__asan_region_is_poisoned` and kill the program if it is indeed poisoined?
Repository:
rCRT Compiler Runtime
https://reviews.llvm.org/D42644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.
Comment at: test/asan/TestCases/intercept-rethrow-exception.cc:48
+ // memcpy is intercepted by asan which performs checks on src and dst
+ using T = int[1000];
+ T x {};
You can include #include
and use __asan_region_is_poisoned
Repository:
rCRT Compiler Runtime
https://reviews.llvm.org/D42644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
robot marked an inline comment as done. robot added a comment. Have you had the time to review the second revision? Repository: rCRT Compiler Runtime https://reviews.llvm.org/D42644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
robot updated this revision to Diff 132990.
robot added a comment.
- Reformatted using clang-format
- Fixed copy&waste error CHECK(REAL(__cxa_throw))
- moved test from asan unit test to lit tests
Repository:
rCRT Compiler Runtime
https://reviews.llvm.org/D42644
Files:
lib/asan/asan_interceptors.cc
lib/asan/asan_interceptors.h
test/asan/TestCases/intercept-rethrow-exception.cc
Index: test/asan/TestCases/intercept-rethrow-exception.cc
===
--- /dev/null
+++ test/asan/TestCases/intercept-rethrow-exception.cc
@@ -0,0 +1,90 @@
+// Regression test for
+// https://bugs.llvm.org/show_bug.cgi?id=32434
+
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %run %t
+
+#include
+#include
+
+#define ATTRIBUTE_NO_SANITIZE_ADDRESS __attribute__((no_sanitize("address")))
+
+// Since we compile with -O0, an empty function is sufficient to mark a variable
+// as "used", which forces creation of redzones.
+void use(void *arg) {}
+
+namespace {
+
+// Not instrumented because std::rethrow_exception is a [[noreturn]] function,
+// for which the compiler would emit a call to __asan_handle_no_return which
+// unpoisons the stack.
+// We emulate here some code not compiled with asan. This function is not
+// [[noreturn]] because the scenario we're emulating doesn't always throw. If it
+// were [[noreturn]], the calling code would emit a call to
+// __asan_handle_no_return.
+void ATTRIBUTE_NO_SANITIZE_ADDRESS
+uninstrumented_rethrow_exception(std::exception_ptr const &exc_ptr) {
+ std::rethrow_exception(exc_ptr);
+}
+
+// access stack, instrumented by asan
+// Asan checks pass if either the stack is entirely unpoisoned around dst and
+// src, or redzones exist but don't overlap with src nor dst.
+// We want this function to emit asan checks (checking the shadow memory). This
+// works either by calling std::memcpy which is intercepted by asan, or by
+// generating instrumented code (if memcpy is resolved via a builtin). To make
+// sure it works in either case, we place the memcpy outside of
+// uninstrumented_fill_and_use_stack and into this separate function, which is
+// instrumented.
+void instrumented_use_stack(void *dst, void const *src, int size) {
+ std::memcpy(dst, src, size);
+}
+
+// Create variables on the stack without touching the shadow memory, and use
+// them to check if there are redzones remaining (from prior stack operations)
+// on the stack.
+void ATTRIBUTE_NO_SANITIZE_ADDRESS uninstrumented_fill_and_use_stack() {
+ // memcpy is intercepted by asan which performs checks on src and dst
+ using T = int[1000];
+ T x {};
+ T y {};
+ instrumented_use_stack(&x, &y, sizeof(T));
+}
+
+// Create redzones for stack variables in shadow memory and call
+// std::rethrow_exception which should unpoison the entire stack.
+void create_redzones_and_throw(std::exception_ptr const &exc_ptr) {
+ int i[100];
+ // force creation of redzones even though variable is unused
+ use(i);
+
+ uninstrumented_rethrow_exception(exc_ptr);
+}
+
+} // namespace
+
+// Check that std::rethrow_exception is intercepted by asan and the interception
+// unpoisons the stack.
+// If std::rethrow_exception is NOT intercepted, then calls to this function
+// from instrumented code will still unpoison the stack because
+// std::rethrow_exception is a [[noreturn]] function and any [[noreturn]]
+// function call will be instrumented with __asan_handle_no_return.
+// However, calls to std::rethrow_exception from UNinstrumented code will not
+// unpoison the stack, so we need to intercept std::rethrow_exception to
+// unpoison the stack.
+int main() {
+ // In some implementations of std::make_exception_ptr, e.g. libstdc++ prior to
+ // gcc 7, this function calls __cxa_throw. The __cxa_throw is intercepted by
+ // asan to unpoison the entire stack; since this test essentially tests that
+ // the stack is unpoisoned by a call to std::rethrow_exception, we need to
+ // generate the exception_ptr BEFORE we have the local variables poison the
+ // stack.
+ std::exception_ptr my_exception_ptr = std::make_exception_ptr("up");
+
+ try {
+create_redzones_and_throw(my_exception_ptr);
+ } catch(char const *) {
+uninstrumented_fill_and_use_stack();
+// ignore exception such that test is successful unless asan reports above
+ }
+}
Index: lib/asan/asan_interceptors.h
===
--- lib/asan/asan_interceptors.h
+++ lib/asan/asan_interceptors.h
@@ -85,8 +85,17 @@
!(SANITIZER_ANDROID && defined(__i386)) && \
!SANITIZER_SOLARIS
# define ASAN_INTERCEPT___CXA_THROW 1
+# define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 1
+# ifdef _GLIBCXX_SJLJ_EXCEPTIONS
+# define ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION 1
+# else
+# define ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION 1
+# endif
#else
# define ASAN_INTERCEPT___CXA_THROW 0
+# define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 0
+# define ASAN_INTERCEPT__UN
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
vitalybuka added a comment.
I would not worry about cross platform here. You can patch just Linux and
whoever have access (and issues) on other platforms can send a patch with
similar changes.
Mac should use libc++.
I'd put the test outside of Posix, and mark failing platforms as "// XFAIL:",
temporarily, as we suppose to handle all of them.
Comment at: lib/asan/asan_interceptors.cc:327
+INTERCEPTOR(void, __cxa_rethrow_primary_exception, void *a) {
+ CHECK(REAL(__cxa_throw));
+ __asan_handle_no_return();
Should this be: CHECK(REAL(__cxa_rethrow_primary_exception)); ?
Repository:
rCRT Compiler Runtime
https://reviews.llvm.org/D42644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
vitalybuka added a comment. Could you please reformat it? With git I usually use: git clang-format -f --style=file HEAD^ Repository: rCRT Compiler Runtime https://reviews.llvm.org/D42644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
kubamracek added a comment. > This is our first patch so we're unfamiliar with the LLVM testing > infrastructure. Could you please tell us what kind of test you'd like? An > example would also be great. Thank you for your first contribution! I'm going to comment on the testing infrastructure only, as I don't know the answer to the portability/mangling/ABI question. Most user-visible features of ASan can usually be demonstrated in a single-file test that is completely standalone test program. E.g. ASan's ability to detect heap buffer overflows is demonstrated in `test/asan/TestCases/heap-overflow.cc` file. Notice that these tests don't include or reference any sanitizer internal functions. They are meant to be written in a way that user's code is written. Can you try adding such a test, which would be a standalone .cc file that would demonstrate the problematic scenario (C++ exception rethrow) without including ASan headers? The test's expectation should be that ASan doesn't report any issues, whereas without your patch, the test fails (or might fail). For an example of a negative test (that checks that ASan *doesn't* report any issues), see `test/asan/TestCases/Darwin/nil-return-struct.mm`. If the test should work on all ASan supported platforms, put it directly into `test/asan/TestCases`. If it's POSIX-only, place it under `Posix/` subdirectory. Repository: rCRT Compiler Runtime https://reviews.llvm.org/D42644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
robot added a comment. In https://reviews.llvm.org/D42644#993506, @kubamracek wrote: > Cool. Can we get a lit test as well? This is our first patch so we're unfamiliar with the LLVM testing infrastructure. Could you please tell us what kind of test you'd like? An example would also be great. We still have some concerns regarding the portability of this patch: we don’t know how to portably intercept a C++ standard library function (name mangling, layout of types, …). In libc++abi, there’s `__cxa_rethrow_primary_exception` but it is not used by libstdc++. Instead, libstdc++’s implementation calls either `_Unwind_RaiseException` or `_Unwind_SjLj_RaiseException` directly – so we’ve intercepted those two for libstdc++. We didn’t check if this helps on other platforms (Mac and Windows come to mind). Additionally, it might be possible there are some other interceptor functions missing such as `__cxa_rethrow` (for throw; statements in catch blocks) which might lead to analogous false positives (especially if you don’t intercept `_Unwind_RaiseException` in the libc++ case). Arnaud & Robot Repository: rCRT Compiler Runtime https://reviews.llvm.org/D42644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
kubamracek added a comment. Cool. Can we get a lit test as well? Repository: rCRT Compiler Runtime https://reviews.llvm.org/D42644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
robot created this revision.
robot added a reviewer: cryptoad.
robot added a project: Sanitizers.
Herald added subscribers: Sanitizers, llvm-commits, hintonda, mgorny,
kubamracek.
Fixes Bug 32434
See https://bugs.llvm.org/show_bug.cgi?id=32434
Short summary:
std::rethrow_exception does not use __cxa_throw to rethrow the exception, so if
it is called from uninstrumented code, it will leave the stack poisoned. This
can lead to false positives.
Long description:
For functions which don't return normally (e.g. via exceptions), asan needs to
unpoison the entire stack. It is not known before a call to such a function
where execution will continue, some function which don't contain cleanup code
like destructors might be skipped. After stack unwinding, execution might
continue in uninstrumented code.
If the stack has been poisoned before such a function is called, but the stack
is unwound during the unconventional return, then zombie redzones (entries) for
no longer existing stack variables can remain in the shadow memory. Normally,
this is avoided by asan generating a call to __asan_handle_no_return before all
functions marked as [[noreturn]]. This __asan_handle_no_return unpoisons the
entire stack. Since these [[noreturn]] functions can be called from
uninstrumented code, asan also introduces interceptor functions which call
__asan_handle_no_return before running the original [[noreturn]] function;
for example, __cxa_throw is intercepted.
If a [[noreturn]] function is called from uninstrumented code (so the stack is
left poisoned) and additionally, execution continues in uninstrumented code, new
stack variables might be introduced and overlap with the stack variables
which have been removed during stack unwinding. Since the redzones are not
cleared nor overwritten by uninstrumented code, they remain but now contain
invalid data.
Now, if the redzones are checked against the new stack variables, false
positive reports can occur. This can happen for example by the uninstrumented
code calling an intercepted function such as memcpy, or an instrumented
function.
Intercepting std::rethrow_exception directly is not easily possible since it
depends on the C++ standard library implementation (e.g. libcxx vs libstdc++)
and the mangled name it produces for this function. As a rather simple
workaround, we're intercepting _Unwind_RaiseException for libstdc++. For
libcxxabi, we can intercept the ABI function __cxa_rethrow_primary_exception.
Repository:
rCRT Compiler Runtime
https://reviews.llvm.org/D42644
Files:
lib/asan/asan_interceptors.cc
lib/asan/asan_interceptors.h
lib/asan/tests/CMakeLists.txt
lib/asan/tests/asan_rethrow_exception_test.cc
Index: lib/asan/tests/asan_rethrow_exception_test.cc
===
--- /dev/null
+++ lib/asan/tests/asan_rethrow_exception_test.cc
@@ -0,0 +1,94 @@
+#if ASAN_HAS_EXCEPTIONS
+
+
+#include "asan_test_utils.h"
+
+#include
+#include
+
+
+namespace {
+
+
+// Not instrumented because std::rethrow_exception is a [[noreturn]] function, for which the
+// compiler would emit a call to __asan_handle_no_return which unpoisons the stack.
+// We emulate here some code not compiled with asan. This function is not [[noreturn]] because the
+// scenario we're emulating doesn't always throw. If it were [[noreturn]], the calling code would
+// emit a call to __asan_handle_no_return.
+void NOINLINE ATTRIBUTE_NO_SANITIZE_ADDRESS
+uninstrumented_rethrow_exception(std::exception_ptr const& exc_ptr)
+{
+ // if we just call rethrow_exception, the compiler knows this function is noreturn, even
+ // though it's not inlined
+ if(Ident(true))
+std::rethrow_exception(exc_ptr);
+}
+
+// access stack, instrumented by asan
+// Asan checks pass if either the stack is entirely unpoisoned around dst and src, or redzones
+// exist but don't overlap with src nor dst.
+// We want this function to emit asan checks (checking the shadow memory). This works either by
+// calling std::memcpy which is intercepted by asan, or by generating instrumented code (if memcpy
+// is resolved via a builtin). To make sure it works in either case, we place the memcpy outside of
+// uninstrumented_fill_and_use_stack and into this separate function, which is instrumented.
+void NOINLINE
+instrumented_use_stack(void* dst, void const* src, int size)
+{
+ std::memcpy(dst, src, size);
+}
+
+// Create variables on the stack without touching the shadow memory, and use them to check if there
+// are redzones remaining (from prior stack operations) on the stack.
+void ATTRIBUTE_NO_SANITIZE_ADDRESS
+uninstrumented_fill_and_use_stack()
+{
+ // memcpy is intercepted by asan which performs checks on src and dst
+ using T = int[1000];
+ T x{};
+ T y{};
+ instrumented_use_stack(&x, &y, sizeof(T));
+}
+
+
+// Create redzones for stack variables in shadow memory and call std::rethrow_exception which
+// should unpoison the entire stack.
+void NOINLINE
+create_redzones_a
