diff --git a/compiler-rt/lib/asan/asan_interceptors.h b/compiler-rt/lib/asan/asan_interceptors.h --- a/compiler-rt/lib/asan/asan_interceptors.h +++ b/compiler-rt/lib/asan/asan_interceptors.h @@ -22,6 +22,7 @@ namespace __asan { void InitializeAsanInterceptors(); +void PlatformBeforeInitializeAsanInterceptors(); void InitializePlatformInterceptors(); #define ENSURE_ASAN_INITED() \ diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp --- a/compiler-rt/lib/asan/asan_interceptors.cpp +++ b/compiler-rt/lib/asan/asan_interceptors.cpp @@ -656,6 +656,7 @@ static bool was_called_once; CHECK(!was_called_once); was_called_once = true; + PlatformBeforeInitializeAsanInterceptors(); InitializeCommonInterceptors(); InitializeSignalInterceptors(); diff --git a/compiler-rt/lib/asan/asan_linux.cpp b/compiler-rt/lib/asan/asan_linux.cpp --- a/compiler-rt/lib/asan/asan_linux.cpp +++ b/compiler-rt/lib/asan/asan_linux.cpp @@ -80,6 +80,7 @@ namespace __asan { +void PlatformBeforeInitializeAsanInterceptors() {} void InitializePlatformInterceptors() {} void InitializePlatformExceptionHandlers() {} bool IsSystemHeapAddress(uptr addr) { return false; } diff --git a/compiler-rt/lib/asan/asan_mac.cpp b/compiler-rt/lib/asan/asan_mac.cpp --- a/compiler-rt/lib/asan/asan_mac.cpp +++ b/compiler-rt/lib/asan/asan_mac.cpp @@ -45,6 +45,7 @@ namespace __asan { +void PlatformBeforeInitializeAsanInterceptors() {} void InitializePlatformInterceptors() {} void InitializePlatformExceptionHandlers() {} bool IsSystemHeapAddress (uptr addr) { return false; } diff --git a/compiler-rt/lib/asan/asan_win.cpp b/compiler-rt/lib/asan/asan_win.cpp --- a/compiler-rt/lib/asan/asan_win.cpp +++ b/compiler-rt/lib/asan/asan_win.cpp @@ -158,6 +158,10 @@ namespace __asan { +void PlatformBeforeInitializeAsanInterceptors() { + __interception::SetErrorReportCallback(Report); +} + void InitializePlatformInterceptors() { // The interceptors were not designed to be removable, so we have to keep this // module alive for the life of the process. diff --git a/compiler-rt/lib/interception/interception_win.h b/compiler-rt/lib/interception/interception_win.h --- a/compiler-rt/lib/interception/interception_win.h +++ b/compiler-rt/lib/interception/interception_win.h @@ -41,6 +41,11 @@ const char *function_name, uptr new_function, uptr *orig_old_func); +// Sets a callback to be used for reporting errors by interception_win. The +// callback will be called with printf-like arguments. Intended to be used with +// __sanitizer::Report. Pass nullptr to disable error reporting (default). +void SetErrorReportCallback(void (*callback)(const char *format, ...)); + #if !SANITIZER_WINDOWS64 // Exposed for unittests bool OverrideFunctionWithDetour( diff --git a/compiler-rt/lib/interception/interception_win.cpp b/compiler-rt/lib/interception/interception_win.cpp --- a/compiler-rt/lib/interception/interception_win.cpp +++ b/compiler-rt/lib/interception/interception_win.cpp @@ -141,8 +141,27 @@ FIRST_32_SECOND_64(kJumpInstructionLength, kIndirectJumpInstructionLength); static const int kDirectBranchLength = kBranchLength + kAddressLength; +# if defined(_MSC_VER) +# define INTERCEPTION_FORMAT(f, a) +# else +# define INTERCEPTION_FORMAT(f, a) __attribute__((format(printf, f, a))) +# endif + +static void (*ErrorReportCallback)(const char *format, ...) + INTERCEPTION_FORMAT(1, 2); + +void SetErrorReportCallback(void (*callback)(const char *format, ...)) { + ErrorReportCallback = callback; +} + +# define Report(...) \ + do { \ + if (ErrorReportCallback) \ + ErrorReportCallback(__VA_ARGS__); \ + } while (0) + static void InterceptionFailed() { - // Do we have a good way to abort with an error message here? + Report("interception_win: failed due to an unrecoverable error.\n"); // This acts like an abort when no debugger is attached. According to an old // comment, calling abort() leads to an infinite recursion in CheckFailed. __debugbreak(); @@ -657,10 +676,18 @@ } #endif - // Unknown instruction! - // FIXME: Unknown instruction failures might happen when we add a new - // interceptor or a new compiler version. In either case, they should result - // in visible and readable error messages. + // Unknown instruction! This might happen when we add a new interceptor, use + // a new compiler version, or if Windows changed how some functions are + // compiled. In either case, we print the address and 8 bytes of instructions + // to notify the user about the error and to help identify the unknown + // instruction. Don't treat this as a fatal error, though we can break the + // debugger if one has been attached. + u8 *bytes = (u8 *)address; + Report( + "interception_win: unhandled instruction at %p: %02X %02X %02X %02X %02X " + "%02X %02X %02X\n", + (void *)address, bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], + bytes[5], bytes[6], bytes[7]); if (::IsDebuggerPresent()) __debugbreak(); return 0; diff --git a/compiler-rt/lib/interception/tests/interception_win_test.cpp b/compiler-rt/lib/interception/tests/interception_win_test.cpp --- a/compiler-rt/lib/interception/tests/interception_win_test.cpp +++ b/compiler-rt/lib/interception/tests/interception_win_test.cpp @@ -18,6 +18,8 @@ #if !SANITIZER_DEBUG #if SANITIZER_WINDOWS +#include + #define WIN32_LEAN_AND_MEAN #include @@ -728,7 +730,37 @@ TestOverrideFunction override = OverrideFunctionWithTrampoline; FunctionPrefixKind prefix = FunctionPrefixPadding; + static bool reportCalled; + reportCalled = false; + + struct Local { + static void Report(const char *format, ...) { + if (reportCalled) + FAIL() << "Report called more times than expected"; + reportCalled = true; + ASSERT_STREQ( + "interception_win: unhandled instruction at %p: %02X %02X %02X %02X " + "%02X %02X %02X %02X\n", + format); + va_list args; + va_start(args, format); + u8 *ptr = va_arg(args, u8 *); + for (int i = 0; i < 8; i++) EXPECT_EQ(kUnsupportedCode1[i], ptr[i]); + int bytes[8]; + for (int i = 0; i < 8; i++) { + bytes[i] = va_arg(args, int); + EXPECT_EQ(kUnsupportedCode1[i], bytes[i]); + } + va_end(args); + } + }; + + SetErrorReportCallback(Local::Report); EXPECT_FALSE(TestFunctionPatching(kUnsupportedCode1, override, prefix)); + SetErrorReportCallback(nullptr); + + if (!reportCalled) + ADD_FAILURE() << "Report not called"; } TEST(Interception, PatchableFunctionPadding) {