[PATCH] D20884: This patch attempts to primitive support for Win64 asan

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/interception/interception_win.cc:68-71
@@ +67,6 @@
+  *(unsigned int*)(jmp_from + 1) = to_addr_lower;
+  *(jmp_from + 5) = '\xc7';
+  *(jmp_from + 6) = '\x44';
+  *(jmp_from + 7) = '\x24';
+  *(jmp_from + 8) = '\x04';
+  // Higher 4 bytes of the full 64-bit address.
----------------
etienneb wrote:
> *(jmp_from + 5)   -> jmp_from[5]
> and below
Would this be cleaner with array indexing?
  jmp_from[5] = '\xc7';
  jmp_from[6] = '\x44;
  jmp_from[7] = '\x24;
  ...
Obviously doesn't apply to the store of the immediate.

================
Comment at: lib/interception/interception_win.cc:85
@@ +84,3 @@
+  *(jmp_from + 4) = 0xFF;
+  *(jmp_from + 5) = 0xFF;
+  // FIXME(wwchrome): We are betting on 0xCC paddings, but in practice
----------------
ditto

================
Comment at: lib/interception/interception_win.cc:177-178
@@ +176,4 @@
+    switch (0xFFFFFFFFFFull & *(unsigned long long*)(code + cursor)) {
+      case 0x08245c8948:    // 48 89 5c 24 08 : mov QWORD PTR [rsp+0x8], rbx
+      case 0x1024748948:    // 48 89 74 24 10 : mov QWORD PTR [rsp+0x10], rsi
+        cursor += 5;
----------------
These can be 4 byte checks, the displacement is in the fifth byte and doesn't need to be checked.

================
Comment at: lib/interception/interception_win.cc:183
@@ +182,3 @@
+
+    // Unknown instructions!!!
+    __debugbreak();
----------------
We don't need three exclamation points, we know the table above isn't complete, it's just good enough for what we're trying to do.

================
Comment at: lib/interception/interception_win.cc:275
@@ +274,3 @@
+
+  // TODO(wwchrome): Merge with the 32-bit version maybe.
+  size_t kHeadMin = 6; // The minimum size of the head.
----------------
Can we do this now? There really isn't much difference between the 32-bit and 64-bit code, just different trampolines, which I think you could abstract pretty easily.

================
Comment at: lib/interception/interception_win.cc:293
@@ +292,3 @@
+    _memcpy(trampoline, old_bytes, head);
+    WriteJumpInstruction_14I(trampoline + head, old_bytes + head);
+    *orig_old_func = (uptr)trampoline;
----------------
It's probably simpler to use this code here:
  jmp [rip+5] # not sure about precise offset
  .quad target_address

To help with the 32/64 code merge, I'd create two entry points, WriteTrampolineJumpInstruction and WriteInterceptorJumpInstruction which behave differently on 32 and 64bit. On 64-bit, they would both delegate to WriteIndirectJumpInstruction, which would take two pointers, a code pointer a pointer to the address to jump through. A prototype:

  void WriteTrampolineJumpInstruction(char *jmp_from, char *to) {
    // Emit an indirect jump through immediately following bytes:
    // jmp_from:
    //   jmp [rip + 5]
    //   .quad to
    // Store the address.
    uptr *indirect_target = (uptr *)(jmp_from + 6);
    *indirect_target = (uptr)to;
    // Write the indirect jump.
    WriteIndirectJumpInstruction(jmp_from, indirect_target);
  }

  void WriteInterceptorJumpInstruction(char *jmp_from, char *to) {
    // Emit an indirect jump through immediately preceding bytes:
    //   .quad to
    // jmp_from:
    //   jmp [rip - 8]
    // Store the address.
    uptr *indirect_target = (uptr *)(jmp_from - 8);
    *indirect_target = (uptr)to;
    // Write the indirect jump.
    WriteIndirectJumpInstruction(jmp_from, indirect_target);
  }

For 32-bit they would both write a normal 5 byte jump


http://reviews.llvm.org/D20884