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

[Alvin Wong via Phabricator via llvm-commits]

alvinhochun created this revision.
alvinhochun added a reviewer: vitalybuka.
Herald added subscribers: Enna1, dberris.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149549

Files:
  compiler-rt/lib/interception/interception_win.cpp
  compiler-rt/lib/interception/tests/interception_win_test.cpp


Index: compiler-rt/lib/interception/tests/interception_win_test.cpp
===================================================================
--- compiler-rt/lib/interception/tests/interception_win_test.cpp
+++ compiler-rt/lib/interception/tests/interception_win_test.cpp
@@ -307,6 +307,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 +724,13 @@
   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;
Index: compiler-rt/lib/interception/interception_win.cpp
===================================================================
--- compiler-rt/lib/interception/interception_win.cpp
+++ compiler-rt/lib/interception/interception_win.cpp
@@ -143,6 +143,8 @@
 
 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 @@
   // 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 @@
   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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149549.518304.patch
Type: text/x-patch
Size: 2537 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230430/ec3e661f/attachment.bin>