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 @@ -127,6 +127,8 @@ #include "interception.h" #if SANITIZER_WINDOWS +#include "sanitizer_common/sanitizer_common.h" +#include "sanitizer_common/sanitizer_libc.h" #include "sanitizer_common/sanitizer_platform.h" #define WIN32_LEAN_AND_MEAN #include @@ -142,7 +144,9 @@ static const int kDirectBranchLength = kBranchLength + kAddressLength; 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(); } @@ -655,12 +659,20 @@ } #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. However, merely calling abort() - // leads to an infinite recursion in CheckFailed. - InterceptionFailed(); + // 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. + char instruction[8 * 3 + 1] = {0}; + for (int i = 0; i < 8; i++) { + internal_snprintf(instruction + (i * 3), 4, " %02X", *(u8 *)(address + i)); + } + Report("interception_win: unhandled instruction at %p:%s\n", (void *)address, + instruction); + if (IsDebuggerPresent()) + __debugbreak(); return 0; } @@ -681,6 +693,8 @@ while (cursor != size) { size_t rel_offset = 0; size_t instruction_size = GetInstructionSize(from + cursor, &rel_offset); + if (!instruction_size) + return false; _memcpy((void*)(to + cursor), (void*)(from + cursor), (size_t)instruction_size); if (rel_offset) { diff --git a/compiler-rt/lib/interception/tests/CMakeLists.txt b/compiler-rt/lib/interception/tests/CMakeLists.txt --- a/compiler-rt/lib/interception/tests/CMakeLists.txt +++ b/compiler-rt/lib/interception/tests/CMakeLists.txt @@ -32,6 +32,11 @@ else() list(APPEND INTERCEPTION_TEST_CFLAGS_COMMON -g) endif() +set(INTERCEPTION_TEST_GMOCK_SOURCE "") +if(WIN32) + list(APPEND INTERCEPTION_TEST_CFLAGS_COMMON ${COMPILER_RT_GMOCK_CFLAGS}) + set(INTERCEPTION_TEST_GMOCK_SOURCE ${COMPILER_RT_GMOCK_SOURCE}) +endif() if(MSVC) list(APPEND INTERCEPTION_TEST_CFLAGS_COMMON -gcodeview) list(APPEND INTERCEPTION_TEST_LINK_FLAGS_COMMON @@ -101,7 +106,7 @@ generate_compiler_rt_tests(INTERCEPTION_TEST_OBJECTS InterceptionUnitTests "Interception-${arch}-Test" ${arch} RUNTIME ${INTERCEPTION_COMMON_LIB} - SOURCES ${INTERCEPTION_UNITTESTS} ${COMPILER_RT_GTEST_SOURCE} + SOURCES ${INTERCEPTION_UNITTESTS} ${COMPILER_RT_GTEST_SOURCE} ${INTERCEPTION_TEST_GMOCK_SOURCE} COMPILE_DEPS ${INTERCEPTION_TEST_HEADERS} DEPS llvm_gtest CFLAGS ${INTERCEPTION_TEST_CFLAGS_COMMON} @@ -118,6 +123,10 @@ foreach(arch ${INTERCEPTION_UNITTEST_SUPPORTED_ARCH}) add_interceptor_lib("RTInterception.test.${arch}" $) + if(WIN32) + target_sources("RTInterception.test.${arch}" PUBLIC + $) + endif() endforeach() endif() foreach(arch ${INTERCEPTION_UNITTEST_SUPPORTED_ARCH}) 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,10 @@ #if !SANITIZER_DEBUG #if SANITIZER_WINDOWS +#include + +#include "gmock/gmock.h" +#include "sanitizer_common/sanitizer_common.h" #define WIN32_LEAN_AND_MEAN #include @@ -307,6 +311,13 @@ 0x56, // push esi }; +const u8 kUnsupportedCode1[] = { + 0x0f, 0x0b, // ud2 + 0x0f, 0x0b, // ud2 + 0x0f, 0x0b, // ud2 + 0x0f, 0x0b, // ud2 +}; + // A buffer holding the dynamically generated code under test. u8* ActiveCode; const size_t ActiveCodeLength = 4096; @@ -717,6 +728,24 @@ EXPECT_FALSE(TestFunctionPatching(kUnpatchableCode6, override, prefix)); } +TEST(Interception, UnsupportedInstructionWithTrampoline) { + using ::testing::MatchesRegex; + TestOverrideFunction override = OverrideFunctionWithTrampoline; + FunctionPrefixKind prefix = FunctionPrefixPadding; + + static std::string reportString; + reportString.clear(); + + SetPrintfAndReportCallback( + [](const char *message) { reportString.append(message); }); + EXPECT_FALSE(TestFunctionPatching(kUnsupportedCode1, override, prefix)); + SetPrintfAndReportCallback(nullptr); + + EXPECT_THAT(reportString.c_str(), + MatchesRegex("^==\\d+==interception_win: unhandled instruction " + "at 0x\\w+: 0F 0B 0F 0B 0F 0B 0F 0B\n$")); +} + TEST(Interception, PatchableFunctionPadding) { TestOverrideFunction override = OverrideFunction; FunctionPrefixKind prefix = FunctionPrefixPadding;