Refactor rel32 searching process for x64 to make it more similar to x86.

courgette/rel32_finder_x64.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "courgette/rel32_finder_x64.h"
+
+namespace courgette {
+
+Rel32FinderX64::Rel32FinderX64(RVA relocs_start_rva,
+                               RVA relocs_end_rva,
+                               RVA size_of_image)
+    : Rel32Finder(relocs_start_rva, relocs_end_rva),
+      size_of_image_(size_of_image) {}
+
+// Scan for opcodes matching the following instructions :
+//  rel32 JMP/CALL
+//  rip mov/lea

[huangs, 2016/05/30 05:48:34]

NIT: Capitalize MOV/LEA for consistency (Jcc is fine though).  Also apply to
comments elsewhere in the file.

[etiennep, 2016/05/30 17:07:30]

Done.

+//  Jcc (excluding JPO/JPE)
+// Falsely detected rel32 that collide with known abs32 or that point outside
+// valid regions are discarded.
+void Rel32FinderX64::Find(const uint8_t* start_pointer,
+                          const uint8_t* end_pointer,
+                          RVA start_rva,
+                          RVA end_rva,
+                          const std::vector<RVA>& abs32_locations) {
+  // Quick way to convert from Pointer to RVA within a single Section is to
+  // subtract 'adjust_pointer_to_rva'.

[huangs, 2016/05/30 05:48:34]

NIT: |adjust_pointer_to_rva|, now that it's a variable.

[etiennep, 2016/05/30 17:07:30]

Done.

+  const uint8_t* const adjust_pointer_to_rva = start_pointer - start_rva;
+
+  std::vector<RVA>::const_iterator abs32_pos = abs32_locations.begin();
+
+  // Find the rel32 relocations.
+  const uint8_t* p = start_pointer;
+  while (p < end_pointer) {
+    RVA current_rva = static_cast<RVA>(p - adjust_pointer_to_rva);
+
+    // Skip the base reloation table if we encounter it.
+    // Note: We're not bothering to handle the edge case where a Rel32 pointer
+    // collides with |relocs_start_rva_| by being {1, 2, 3}-bytes before it.
+    if (current_rva >= relocs_start_rva_ && current_rva < relocs_end_rva_) {
+      p += relocs_end_rva_ - current_rva;
+      continue;
+    }
+
+    // Heuristic discovery of rel32 locations in instruction stream: are the
+    // next few bytes the start of an instruction containing a rel32
+    // addressing mode?
+    const uint8_t* rel32 = nullptr;
+    bool is_rip_relative = false;
+
+    if (p + 5 <= end_pointer) {
+      if (p[0] == 0xE8 || p[0] == 0xE9)  // jmp rel32 and call rel32
+        rel32 = p + 1;
+    }
+    if (p + 6 <= end_pointer) {
+      if (p[0] == 0x0F && (p[1] & 0xF0) == 0x80) {  // Jcc long form
+        if (p[1] != 0x8A && p[1] != 0x8B)           // JPE/JPO unlikely
+          rel32 = p + 2;
+      } else if (p[0] == 0xFF && (p[1] == 0x15 || p[1] == 0x25)) {
+        // rip relative call/jmp
+        rel32 = p + 2;
+        is_rip_relative = true;
+      }
+    }
+    // TODO(etiennep): Many rip mov/lea variants are not detected.

[huangs, 2016/05/30 05:48:34]

NIT: ... Experiment and fix.

[etiennep, 2016/05/30 17:07:30]

Done.

+    if (p + 7 <= end_pointer) {
+      if ((p[0] & 0xFB) == 0x48 &&  // Dst reg : 48/4C [rax-rdi]/[r8-r15]

[huangs, 2016/05/30 05:48:34]

Src or Dst?

[etiennep, 2016/05/30 17:07:30]

Dst

+          p[1] == 0x8D &&           // lea
+          (p[2] & 0xC7) == 0x05) {  // Dst reg : [05-3D]

[huangs, 2016/05/30 05:48:34]

NIT:
Dst reg: [05,0D,...,3D] = [r8-r15]

[etiennep, 2016/05/30 17:07:30]

Done.

+        // rip relative lea QWORD

[huangs, 2016/05/30 05:48:34]

QWORD is 64-bits, but we only read 32-bits for this?

[etiennep, 2016/05/30 17:07:30]

Done.

+        rel32 = p + 3;
+        is_rip_relative = true;
+      } else if ((p[0] & 0xFB) == 0x48 &&  // Dst reg : 48/4C [rax-rdi]/[r8-r15]

[huangs, 2016/05/30 05:48:34]

Note that the two logic can be combined:

if ((p[1] == 0x8D || p[1] == 0x8B) &&  // LEA or MOV
    (p[0] & 0xFB) == 0x48 &&  // ...

Since you're refactoring, leaving as-is is a good idea.  Can add TODO though.

[etiennep, 2016/05/30 17:07:30]

Done.

+                 p[1] == 0x8B &&           // mov
+                 (p[2] & 0xC7) == 0x05) {  // Dst reg : [05-3D]
+        // rip relative mov QWORD
+        rel32 = p + 3;
+        is_rip_relative = true;
+      }
+    }
+
+    if (rel32) {
+      RVA rel32_rva = static_cast<RVA>(rel32 - adjust_pointer_to_rva);
+
+      // Is there an abs32 reloc overlapping the candidate?
+      while (abs32_pos != abs32_locations.end() && *abs32_pos < rel32_rva - 3)
+        ++abs32_pos;
+      // Now: (*abs32_pos > rel32_rva - 4) i.e. the lowest addressed 4-byte
+      // region that could overlap rel32_rva.
+      if (abs32_pos != abs32_locations.end()) {
+        if (*abs32_pos < rel32_rva + 4) {
+          // Beginning of abs32 reloc is before end of rel32 reloc so they
+          // overlap. Skip four bytes past the abs32 reloc.
+          p += (*abs32_pos + 4) - current_rva;
+          continue;
+        }
+      }
+
+      // + 4 since offset is relative to start of next instruction.
+      RVA target_rva = rel32_rva + 4 + Read32LittleEndian(rel32);
+      // To be valid, rel32 target must be within image, and within this
+      // section.
+      if (target_rva < size_of_image_ &&  // Subsumes rva != kUnassignedRVA.

[huangs, 2016/05/30 05:48:34]

Re. your Patch Set #2 comment on moving |abs32_locations|: This is still needed
if we do some disassembly.  Disassembly is not fool-proof (e.g. switch
statements may have look up tables inlined in code).  Having overlapping abs32
and rel32 would break Courgette.  However, we can do this as a post-processing
step?  Anyway, leaving as-is for now is fine.

[etiennep, 2016/05/30 17:07:30]

Will talk about in next cl!

+          (is_rip_relative ||
+           (start_rva <= target_rva && target_rva < end_rva))) {
+        rel32_locations_.push_back(rel32_rva);
+#if COURGETTE_HISTOGRAM_TARGETS
+        ++rel32_target_rvas_[target_rva];
+#endif
+        p = rel32 + 4;
+        continue;
+      }
+    }
+    p += 1;
+  }
+}
+
+}  // namespace courgette