[PATCH] D137788: [sanitizers] [windows] Correctly override functions with backward jmps

[Markus Böck via Phabricator via llvm-commits]

zero9178 updated this revision to Diff 480262.
zero9178 added a comment.

Address review comments:

- Make use of `s32` and `sptr`
- Adjust test x86 code to correctly jump to the beginning of the code
- Fix test to correctly override the relative jump


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137788/new/

https://reviews.llvm.org/D137788

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
@@ -85,7 +85,16 @@
     0xC3,                   // ret
 };
 
-#else
+const u8 kIdentityCodeWithJumpBackwards[] = {
+    0x89, 0xC8,  // mov         eax, ecx
+    0xC3,        // ret
+    0xE9, 0xF8, 0xFF, 0xFF,
+    0xFF,  // jmp - 8
+    0xCC, 0xCC, 0xCC, 0xCC,
+};
+const u8 kIdentityCodeWithJumpBackwardsOffset = 3;
+
+#    else
 
 const u8 kIdentityCodeWithPrologue[] = {
     0x55,                   // push        ebp
@@ -134,7 +143,16 @@
     0xC3,                   // ret
 };
 
-#endif
+const u8 kIdentityCodeWithJumpBackwards[] = {
+    0x8B, 0x44, 0x24, 0x04,  // mov         eax,dword ptr [esp + 4]
+    0xC3,                    // ret
+    0xE9, 0xF6, 0xFF, 0xFF,
+    0xFF,  // jmp - 10
+    0xCC, 0xCC, 0xCC, 0xCC,
+};
+const u8 kIdentityCodeWithJumpBackwardsOffset = 5;
+
+#    endif
 
 const u8 kPatchableCode1[] = {
     0xB8, 0x4B, 0x00, 0x00, 0x00,   // mov eax,4B
@@ -366,13 +384,14 @@
   EXPECT_NE(DbgPrint_adddress, isdigit_address);
 }
 
-template<class T>
+template <class T>
 static void TestIdentityFunctionPatching(
-    const T &code,
-    TestOverrideFunction override,
-    FunctionPrefixKind prefix_kind = FunctionPrefixNone) {
+    const T &code, TestOverrideFunction override,
+    FunctionPrefixKind prefix_kind = FunctionPrefixNone,
+    int function_start_offset = 0) {
   uptr identity_address;
   LoadActiveCode(code, &identity_address, prefix_kind);
+  identity_address += function_start_offset;
   IdentityFunction identity = (IdentityFunction)identity_address;
 
   // Validate behavior before dynamic patching.
@@ -410,7 +429,7 @@
   TestOnlyReleaseTrampolineRegions();
 }
 
-#if !SANITIZER_WINDOWS64
+#    if !SANITIZER_WINDOWS64
 TEST(Interception, OverrideFunctionWithDetour) {
   TestOverrideFunction override = OverrideFunctionWithDetour;
   FunctionPrefixKind prefix = FunctionPrefixDetour;
@@ -424,6 +443,9 @@
 TEST(Interception, OverrideFunctionWithRedirectJump) {
   TestOverrideFunction override = OverrideFunctionWithRedirectJump;
   TestIdentityFunctionPatching(kIdentityCodeWithJump, override);
+  TestIdentityFunctionPatching(kIdentityCodeWithJumpBackwards, override,
+                               FunctionPrefixNone,
+                               kIdentityCodeWithJumpBackwardsOffset);
 }
 
 TEST(Interception, OverrideFunctionWithHotPatch) {
Index: compiler-rt/lib/interception/interception_win.cpp
===================================================================
--- compiler-rt/lib/interception/interception_win.cpp
+++ compiler-rt/lib/interception/interception_win.cpp
@@ -738,7 +738,7 @@
     return false;
 
   if (orig_old_func) {
-    uptr relative_offset = *(u32*)(old_func + 1);
+    sptr relative_offset = *(s32 *)(old_func + 1);
     uptr absolute_target = old_func + relative_offset + kJumpInstructionLength;
     *orig_old_func = absolute_target;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137788.480262.patch
Type: text/x-patch
Size: 3097 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221205/ae969538/attachment.bin>