[asan] Intercept std::rethrow_exception indirectly.

[robot]

Summary

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.

[kubamracek]

Cool. Can we get a lit test as well?

[robot]

In 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

[kubamracek]

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.

[vitalybuka]

Could you please reformat it?
With git I usually use: git clang-format -f --style=file HEAD^

[vitalybuka]

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.

lib/asan/asan_interceptors.cc

329

Should this be: CHECK(REAL(__cxa_rethrow_primary_exception)); ?

[robot]

• Reformatted using clang-format
• Fixed copy&waste error CHECK(REAL(__cxa_throw))
• moved test from asan unit test to lit tests

[robot]

Have you had the time to review the second revision?

[vitalybuka]

test/asan/TestCases/intercept-rethrow-exception.cc

49

You can include #include <sanitizer/asan_interface.h>
and use __asan_region_is_poisoned

[robot]

test/asan/TestCases/intercept-rethrow-exception.cc

49

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?

[vitalybuka]

test/asan/TestCases/intercept-rethrow-exception.cc

49

Sure, I'll update and commit this for you.

[krytarowski]

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.

lib/asan/asan_interceptors.h

 # define ASAN_INTERCEPT___LONGJMP_CHK 0
 #endif
 
 // Android bug: https://code.google.com/p/android/issues/detail?id=61799
 #if ASAN_HAS_EXCEPTIONS && !SANITIZER_WINDOWS && \
     !(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__UNWIND_RAISEEXCEPTION 0
+# define ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION 0
 #endif
 
 #if !SANITIZER_WINDOWS
 # define ASAN_INTERCEPT___CXA_ATEXIT 1
 #else
 # define ASAN_INTERCEPT___CXA_ATEXIT 0
 #endif
 

lib/asan/asan_interceptors.cc

 // Only the functions in asan_interceptors_memintrinsics.cc are
 // really defined to replace libc functions.
 #if !SANITIZER_FUCHSIA
 
 #if SANITIZER_POSIX
 #include "sanitizer_common/sanitizer_posix.h"
 #endif
 
+#if ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION || \
+    ASAN_INTERCEPT__SJLJ_UNWIND_RAISEEXCEPTION
+#include <unwind.h>
+#endif
+
 #if defined(__i386) && SANITIZER_LINUX
 #define ASAN_PTHREAD_CREATE_VERSION "GLIBC_2.1"
 #elif defined(__mips__) && SANITIZER_LINUX
 #define ASAN_PTHREAD_CREATE_VERSION "GLIBC_2.2"
 #endif
 
 namespace __asan {
 

 #if ASAN_INTERCEPT___CXA_THROW
 INTERCEPTOR(void, __cxa_throw, void *a, void *b, void *c) {
   CHECK(REAL(__cxa_throw));
   __asan_handle_no_return();
   REAL(__cxa_throw)(a, b, c);
 }
 #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)
   ALIAS(WRAPPER_NAME(strchr));
 # else
 #  if SANITIZER_MAC
 DECLARE_REAL(char*, index, const char *string, int c)
 OVERRIDE_FUNCTION(index, strchr);

 #if ASAN_INTERCEPT_SIGLONGJMP
   ASAN_INTERCEPT_FUNC(siglongjmp);
 #endif
 
   // Intercept exception handling functions.
 #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_RAISEEXCEPTION
+  INTERCEPT_FUNCTION(_Unwind_RaiseException);
+#endif
+  // Indirectly intercept std::rethrow_exception.
+#if ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION
+  INTERCEPT_FUNCTION(_Unwind_SjLj_RaiseException);
+#endif
 
   // Intercept threading-related functions
 #if ASAN_INTERCEPT_PTHREAD_CREATE
 #if defined(ASAN_PTHREAD_CREATE_VERSION)
   ASAN_INTERCEPT_FUNC_VER(pthread_create, ASAN_PTHREAD_CREATE_VERSION);
 #else
   ASAN_INTERCEPT_FUNC(pthread_create);
 #endif

test/asan/TestCases/intercept-rethrow-exception.cc

+// Regression test for
+// https://bugs.llvm.org/show_bug.cgi?id=32434
+
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %run %t
+
+#include <assert.h>
+#include <exception>
+#include <sanitizer/asan_interface.h>
+
+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));
+  }
+}