From 4a4c52dbdfd6bfff211c90922ed66f8fa65d2588 Mon Sep 17 00:00:00 2001 From: Konstantin Baladurin Date: Fri, 26 Jan 2018 01:19:19 +0300 Subject: [PATCH] Fix asan false-positive errors: (dotnet/coreclr#15563) - Call __asan_handle_no_return in RtlRestoreContext if it doesn't return and in ThrowExceptionFromContextInternal function; - Increase alternate signal stack size and use it also for asan. Commit migrated from https://github.com/dotnet/coreclr/commit/9639f454de21775ab1031471c2acb64738b77c95 --- src/coreclr/CMakeLists.txt | 1 + src/coreclr/enablesanitizers.sh | 7 ++++--- src/coreclr/src/pal/src/arch/amd64/context2.S | 13 ++++++++++++- src/coreclr/src/pal/src/arch/amd64/exceptionhelper.S | 6 ++++++ src/coreclr/src/pal/src/arch/arm/context2.S | 13 ++++++++++++- src/coreclr/src/pal/src/arch/arm/exceptionhelper.S | 6 ++++++ src/coreclr/src/pal/src/arch/arm64/context2.S | 11 +++++++++++ src/coreclr/src/pal/src/arch/arm64/exceptionhelper.S | 6 ++++++ src/coreclr/src/pal/src/arch/i386/context2.S | 5 ++++- src/coreclr/src/pal/src/arch/i386/exceptionhelper.S | 6 ++++++ src/coreclr/src/pal/src/exception/signal.cpp | 5 +++++ 11 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/coreclr/CMakeLists.txt b/src/coreclr/CMakeLists.txt index 0d42deb..8057d67 100644 --- a/src/coreclr/CMakeLists.txt +++ b/src/coreclr/CMakeLists.txt @@ -424,6 +424,7 @@ elseif (CLR_CMAKE_PLATFORM_UNIX) if (${__ASAN_POS} GREATER -1) set(CLR_SANITIZE_CXX_FLAGS "${CLR_SANITIZE_CXX_FLAGS}address,") set(CLR_SANITIZE_LINK_FLAGS "${CLR_SANITIZE_LINK_FLAGS}address,") + add_definitions(-DHAS_ASAN) message("Address Sanitizer (asan) enabled") endif () if (${__UBSAN_POS} GREATER -1) diff --git a/src/coreclr/enablesanitizers.sh b/src/coreclr/enablesanitizers.sh index 2937b0b..aedb95d 100755 --- a/src/coreclr/enablesanitizers.sh +++ b/src/coreclr/enablesanitizers.sh @@ -83,8 +83,9 @@ else unset DEBUG_SANITIZERS echo "Setting DEBUG_SANITIZERS=" else - # for now, specify alloc_dealloc_mismatch=0 as there are too many error reports that are not an issue - ASAN_OPTIONS="symbolize=1 alloc_dealloc_mismatch=0" + # for now, specify alloc_dealloc_mismatch=0 as there are too many error reports that are not an issue. + # Also specify use_sigaltstack=0 as coreclr uses own alternate stack for signal handlers + ASAN_OPTIONS="symbolize=1 alloc_dealloc_mismatch=0 use_sigaltstack=0" # when Clang 3.8 available, add: suppressions=$(readlink -f sanitizersuppressions.txt) UBSAN_OPTIONS="print_stacktrace=1" @@ -132,4 +133,4 @@ else unset __EnableLSan unset __TurnOff unset __Options -fi \ No newline at end of file +fi diff --git a/src/coreclr/src/pal/src/arch/amd64/context2.S b/src/coreclr/src/pal/src/arch/amd64/context2.S index 0e93e81..46c941f 100644 --- a/src/coreclr/src/pal/src/arch/amd64/context2.S +++ b/src/coreclr/src/pal/src/arch/amd64/context2.S @@ -104,7 +104,18 @@ LEAF_END RtlCaptureContext, _TEXT LEAF_ENTRY RtlRestoreContext, _TEXT push_nonvol_reg rbp alloc_stack (IRetFrameLengthAligned) - + +#ifdef HAS_ASAN + test BYTE PTR [rdi + CONTEXT_ContextFlags], CONTEXT_CONTROL + je LOCAL_LABEL(Restore_CONTEXT_DEBUG_REGISTERS) + + push_nonvol_reg rdi + push_nonvol_reg rsi + call EXTERNAL_C_FUNC(__asan_handle_no_return) + pop_nonvol_reg rsi + pop_nonvol_reg rdi +LOCAL_LABEL(Restore_CONTEXT_DEBUG_REGISTERS): +#endif test BYTE PTR [rdi + CONTEXT_ContextFlags], CONTEXT_DEBUG_REGISTERS je LOCAL_LABEL(Done_Restore_CONTEXT_DEBUG_REGISTERS) mov rdx, [rdi + CONTEXT_Dr0] diff --git a/src/coreclr/src/pal/src/arch/amd64/exceptionhelper.S b/src/coreclr/src/pal/src/arch/amd64/exceptionhelper.S index b7b34ac..72a1393 100644 --- a/src/coreclr/src/pal/src/arch/amd64/exceptionhelper.S +++ b/src/coreclr/src/pal/src/arch/amd64/exceptionhelper.S @@ -14,6 +14,12 @@ // Then it uses the ThrowExceptionHelper to throw the passed in exception from that context. // EXTERN_C void ThrowExceptionFromContextInternal(CONTEXT* context, PAL_SEHException* ex); LEAF_ENTRY ThrowExceptionFromContextInternal, _TEXT +#ifdef HAS_ASAN + // Need to call __asan_handle_no_return explicitly here because we re-intialize RSP before + // throwing exception in ThrowExceptionHelper + call EXTERNAL_C_FUNC(__asan_handle_no_return) +#endif + // Save the RBP to the stack so that the unwind can work at the instruction after // loading the RBP from the context, but before loading the RSP from the context. push_nonvol_reg rbp diff --git a/src/coreclr/src/pal/src/arch/arm/context2.S b/src/coreclr/src/pal/src/arch/arm/context2.S index 61e9ab8..42f50c9 100644 --- a/src/coreclr/src/pal/src/arch/arm/context2.S +++ b/src/coreclr/src/pal/src/arch/arm/context2.S @@ -112,7 +112,18 @@ LEAF_END RtlCaptureContext, _TEXT // LEAF_ENTRY RtlRestoreContext, _TEXT END_PROLOGUE - + +#ifdef HAS_ASAN + ldr r2, [r0, #(CONTEXT_ContextFlags)] + tst r2, #(CONTEXT_CONTROL) + beq LOCAL_LABEL(Restore_CONTEXT_FLOATING_POINT) + + push {r0, r1} + bl EXTERNAL_C_FUNC(__asan_handle_no_return) + pop {r0, r1} + +LOCAL_LABEL(Restore_CONTEXT_FLOATING_POINT): +#endif ldr r2, [r0, #(CONTEXT_ContextFlags)] tst r2, #(CONTEXT_FLOATING_POINT) diff --git a/src/coreclr/src/pal/src/arch/arm/exceptionhelper.S b/src/coreclr/src/pal/src/arch/arm/exceptionhelper.S index 76cdcba..dad48de 100644 --- a/src/coreclr/src/pal/src/arch/arm/exceptionhelper.S +++ b/src/coreclr/src/pal/src/arch/arm/exceptionhelper.S @@ -11,6 +11,12 @@ // EXTERN_C void ThrowExceptionFromContextInternal(CONTEXT* context, PAL_SEHException* ex); LEAF_ENTRY ThrowExceptionFromContextInternal, _TEXT // Ported from src/pal/src/arch/amd64/exceptionhelper.S +#ifdef HAS_ASAN + // Need to call __asan_handle_no_return explicitly here because we re-intialize SP before + // throwing exception in ThrowExceptionHelper + bl EXTERNAL_C_FUNC(__asan_handle_no_return) +#endif + push_nonvol_reg {r7} /* FP. x64-RBP */ ldr r4, [r0, #(CONTEXT_R4)] diff --git a/src/coreclr/src/pal/src/arch/arm64/context2.S b/src/coreclr/src/pal/src/arch/arm64/context2.S index e62a9ac..ac3661a 100644 --- a/src/coreclr/src/pal/src/arch/arm64/context2.S +++ b/src/coreclr/src/pal/src/arch/arm64/context2.S @@ -133,6 +133,17 @@ LEAF_END RtlCaptureContext, _TEXT // x1: Exception* // LEAF_ENTRY RtlRestoreContext, _TEXT + +#ifdef HAS_ASAN + ldr w17, [x0, #(CONTEXT_ContextFlags)] + tbz w17, #CONTEXT_CONTROL_BIT, LOCAL_LABEL(Restore_CONTEXT_FLOATING_POINT) + + stp x0, x1, [sp] + bl EXTERNAL_C_FUNC(__asan_handle_no_return) + ldp x0, x1, [sp] + +LOCAL_LABEL(Restore_CONTEXT_FLOATING_POINT): +#endif // aarch64 specifies: // IP0 and IP1, the Intra-Procedure Call temporary registers, // are available for use by e.g. veneers or branch islands during a procedure call. diff --git a/src/coreclr/src/pal/src/arch/arm64/exceptionhelper.S b/src/coreclr/src/pal/src/arch/arm64/exceptionhelper.S index 480846e..7deeee6 100644 --- a/src/coreclr/src/pal/src/arch/arm64/exceptionhelper.S +++ b/src/coreclr/src/pal/src/arch/arm64/exceptionhelper.S @@ -12,6 +12,12 @@ // Then it uses the ThrowExceptionHelper to throw the passed in exception from that context. // EXTERN_C void ThrowExceptionFromContextInternal(CONTEXT* context, PAL_SEHException* ex); LEAF_ENTRY ThrowExceptionFromContextInternal, _TEXT +#ifdef HAS_ASAN + // Need to call __asan_handle_no_return explicitly here because we re-intialize SP before + // throwing exception in ThrowExceptionHelper + bl EXTERNAL_C_FUNC(__asan_handle_no_return) +#endif + // Save the FP & LR to the stack so that the unwind can work at the instruction after // loading the FP from the context, but before loading the SP from the context. stp fp, lr, [sp, -16]! diff --git a/src/coreclr/src/pal/src/arch/i386/context2.S b/src/coreclr/src/pal/src/arch/i386/context2.S index cf7581d..8c5db20 100644 --- a/src/coreclr/src/pal/src/arch/i386/context2.S +++ b/src/coreclr/src/pal/src/arch/i386/context2.S @@ -94,8 +94,11 @@ LEAF_ENTRY RtlCaptureContext, _TEXT LEAF_END RtlCaptureContext, _TEXT LEAF_ENTRY RtlRestoreContext, _TEXT - mov eax, [esp + 4] +#ifdef HAS_ASAN + call EXTERNAL_C_FUNC(__asan_handle_no_return) +#endif + mov eax, [esp + 4] test BYTE PTR [eax + CONTEXT_ContextFlags], CONTEXT_FLOATING_POINT je LOCAL_LABEL(Done_Restore_CONTEXT_FLOATING_POINT) frstor [eax + CONTEXT_FloatSave] diff --git a/src/coreclr/src/pal/src/arch/i386/exceptionhelper.S b/src/coreclr/src/pal/src/arch/i386/exceptionhelper.S index b9ceffc..609efcf 100644 --- a/src/coreclr/src/pal/src/arch/i386/exceptionhelper.S +++ b/src/coreclr/src/pal/src/arch/i386/exceptionhelper.S @@ -18,6 +18,12 @@ ////////////////////////////////////////////////////////////////////////// LEAF_ENTRY ThrowExceptionFromContextInternal, _TEXT +#ifdef HAS_ASAN + // Need to call __asan_handle_no_return explicitly here because we re-intialize ESP before + // throwing exception in ThrowExceptionHelper + call EXTERNAL_C_FUNC(__asan_handle_no_return) +#endif + push ebp mov ecx, [esp + 12] // ecx: PAL_SEHException * (first argument for ThrowExceptionHelper) mov eax, [esp + 8] // ebx: CONTEXT * diff --git a/src/coreclr/src/pal/src/exception/signal.cpp b/src/coreclr/src/pal/src/exception/signal.cpp index 012e320..a273823 100644 --- a/src/coreclr/src/pal/src/exception/signal.cpp +++ b/src/coreclr/src/pal/src/exception/signal.cpp @@ -153,6 +153,11 @@ BOOL EnsureSignalAlternateStack() // We include the size of the SignalHandlerWorkerReturnPoint in the alternate stack size since the // context contained in it is large and the SIGSTKSZ was not sufficient on ARM64 during testing. int altStackSize = SIGSTKSZ + ALIGN_UP(sizeof(SignalHandlerWorkerReturnPoint), 16) + GetVirtualPageSize(); +#ifdef HAS_ASAN + // Asan also uses alternate stack so we increase its size on the SIGSTKSZ * 4 that enough for asan + // (see kAltStackSize in compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cc) + altStackSize += SIGSTKSZ * 4; +#endif void* altStack; int st = posix_memalign(&altStack, GetVirtualPageSize(), altStackSize); if (st == 0) -- 2.7.4