[compiler-rt][interception][win] Don't crash on unknown instructions

Do not treat unknown instructions as a fatal error. In most cases, failure to intercept a function is reported by the caller, though requires setting verbosity to 1 or higher to be visible. Better error message reporting for asan will be added in a separate patch. Differential Revision: https://reviews.llvm.org/D149549
intel · May 4, 2023 · 7b5571f · 7b5571f
1 parent 814a75d
commit 7b5571f
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 3 deletions.
diff --git a/compiler-rt/lib/interception/interception_win.cpp b/compiler-rt/lib/interception/interception_win.cpp
@@ -143,6 +143,8 @@ static const int kDirectBranchLength = kBranchLength + kAddressLength;
 
 static void InterceptionFailed() {
   // Do we have a good way to abort with an error message here?
+  // 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();
 }
 
@@ -658,9 +660,9 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
   // 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();
+  // in visible and readable error messages.
+  if (::IsDebuggerPresent())
+    __debugbreak();
   return 0;
 }
 
@@ -681,6 +683,8 @@ static bool CopyInstructions(uptr to, uptr from, size_t size) {
   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/interception_win_test.cpp b/compiler-rt/lib/interception/tests/interception_win_test.cpp
@@ -307,6 +307,13 @@ const u8 kPatchableCode14[] = {
     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 +724,13 @@ TEST(Interception, PatchableFunctionWithTrampoline) {
   EXPECT_FALSE(TestFunctionPatching(kUnpatchableCode6, override, prefix));
 }
 
+TEST(Interception, UnsupportedInstructionWithTrampoline) {
+  TestOverrideFunction override = OverrideFunctionWithTrampoline;
+  FunctionPrefixKind prefix = FunctionPrefixPadding;
+
+  EXPECT_FALSE(TestFunctionPatching(kUnsupportedCode1, override, prefix));
+}
+
 TEST(Interception, PatchableFunctionPadding) {
   TestOverrideFunction override = OverrideFunction;
   FunctionPrefixKind prefix = FunctionPrefixPadding;