[ASan][Windows] Fix rip-relative instruction replacement

[strega-nil]

The old code incorrectly checked what relative offsets were allowed.

The correct check is that the offset from the target to the instruction pointer should be within $[-2^{31}, 2^{31})$; however, the check that was originally written was that the offset was within $[0, 2^{31})$. Negative offsets are certainly allowable (as long as they fit in 32 bits), and this change fixes that.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Changes

The old code incorrectly checked what relative offsets were allowed.

The correct check is that the offset from the target to the instruction pointer should be within [-231, 231); however, the check that was originally written was that the offset was within [0, 2**31). Negative offsets are certainly allowable (as long as they fit in 32 bits), and this change fixes that.

---

Full diff: https://github.com/llvm/llvm-project/pull/68432.diff

1 Files Affected:

• (modified) compiler-rt/lib/interception/interception_win.cpp (+11-5)

diff --git a/compiler-rt/lib/interception/interception_win.cpp b/compiler-rt/lib/interception/interception_win.cpp
index 23d26ea94b51a66..4f2a51e1adbc6e0 100644
--- a/compiler-rt/lib/interception/interception_win.cpp
+++ b/compiler-rt/lib/interception/interception_win.cpp
@@ -726,16 +726,22 @@ static bool CopyInstructions(uptr to, uptr from, size_t size) {
     size_t instruction_size = GetInstructionSize(from + cursor, &rel_offset);
     if (!instruction_size)
       return false;
-    _memcpy((void*)(to + cursor), (void*)(from + cursor),
+    _memcpy((void *)(to + cursor), (void *)(from + cursor),
             (size_t)instruction_size);
     if (rel_offset) {
-      uptr delta = to - from;
-      uptr relocated_offset = *(u32*)(to + cursor + rel_offset) - delta;
 #if SANITIZER_WINDOWS64
-      if (relocated_offset + 0x80000000U >= 0xFFFFFFFFU)
+      // we want to make sure that the new relative offset still fits in 32-bits
+      // this will be untrue if relocated_offset \notin [-2**31, 2**31)
+      s64 delta = to - from;
+      s64 relocated_offset = *(s32 *)(to + cursor + rel_offset) - delta;
+      if (-0x8000'0000ll > relocated_offset || relocated_offset > 0x7FFF'FFFFll)
         return false;
+#else
+      // on 32-bit, the relative offset will always be correct
+      s32 delta = to - from;
+      s32 relocated_offset = *(s32 *)(to + cursor + rel_offset) - delta;
 #endif
-      *(u32*)(to + cursor + rel_offset) = relocated_offset;
+      *(s32 *)(to + cursor + rel_offset) = relocated_offset;
     }
     cursor += instruction_size;
   }

[vitalybuka]

Is this clang-formated?

[strega-nil]

Yeah.

[vitalybuka]

maybe if constexpr (SANITIZER_WINDOWS64) to compile it always

[vitalybuka]

I guess you can use same code if use s64/s32 -> sptr