[Zucchini] Simplify Abs32GapFinder and Rel32Finder.

Previously, using Abs32GapFinder / Rel32Finder to visit gaps / rel32
references involves calling a getter that returns an optional<> value
whose emptiness indicates end of iteration. The code to use this looks
like:

  for (auto value = finder.GetNext(); value; value = finder.GetNext()) {
    ...
  }

This CL abandons optional<> usage and caches the results in Finders,
thereby removing repetition in iteration code:

  while (finder.FindNext() {
    auto value = finder.GetValue();
    ...
  }

Additional changes:
* Incorporate AddressTranslator into Rel32Finder to offload translation
  work from callers.
* Add tests to integrate Abs32GapFinder with Rel32Finder.
  * Stylize test data to better show overlap between abs32 references
    with disassembled test data.

Bug: 918867
Change-Id: Id044b67550f81c3f46ab383c5b6200906f56ca4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2918113
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#888049}
NOKEYCHECK=True
GitOrigin-RevId: 89023e1c511e599e6aeaf0b8d80e3efa2e730b5b

7 files changed, 344 insertions, 219 deletions

diff --git a/address_translator.cc b/address_translator.cc
index 20f19b2..a329d1a 100644
--- a/address_translator.cc
+++ b/address_translator.cc
@@ -63,6 +63,8 @@ offset_t AddressTranslator::RvaToOffsetCache::Convert(rva_t rva) const {
 
 AddressTranslator::AddressTranslator() = default;
 
+AddressTranslator::AddressTranslator(AddressTranslator&&) = default;
+
 AddressTranslator::~AddressTranslator() = default;
 
 AddressTranslator::Status AddressTranslator::Initialize(
diff --git a/address_translator.h b/address_translator.h
index 5666b99..a517a2c 100644
--- a/address_translator.h
+++ b/address_translator.h
@@ -10,7 +10,6 @@
 #include <tuple>
 #include <vector>
 
-#include "base/macros.h"
 #include "components/zucchini/algorithm.h"
 #include "components/zucchini/image_utils.h"
 
@@ -114,14 +113,14 @@ class AddressTranslator {
     // Embeds |translator| for use. Now object lifetime is tied to |translator|
     // lifetime.
     explicit OffsetToRvaCache(const AddressTranslator& translator);
+    OffsetToRvaCache(const OffsetToRvaCache&) = delete;
+    const OffsetToRvaCache& operator=(const OffsetToRvaCache&) = delete;
 
     rva_t Convert(offset_t offset) const;
 
    private:
     const AddressTranslator& translator_;
     mutable const AddressTranslator::Unit* cached_unit_ = nullptr;
-
-    DISALLOW_COPY_AND_ASSIGN(OffsetToRvaCache);
   };
 
   // An adaptor for AddressTranslator::RvaToOffset() that caches the last Unit
@@ -131,6 +130,8 @@ class AddressTranslator {
     // Embeds |translator| for use. Now object lifetime is tied to |translator|
     // lifetime.
     explicit RvaToOffsetCache(const AddressTranslator& translator);
+    RvaToOffsetCache(const RvaToOffsetCache&) = delete;
+    const RvaToOffsetCache& operator=(const RvaToOffsetCache&) = delete;
 
     bool IsValid(rva_t rva) const;
 
@@ -139,8 +140,6 @@ class AddressTranslator {
    private:
     const AddressTranslator& translator_;
     mutable const AddressTranslator::Unit* cached_unit_ = nullptr;
-
-    DISALLOW_COPY_AND_ASSIGN(RvaToOffsetCache);
   };
 
   enum Status {
@@ -152,6 +151,9 @@ class AddressTranslator {
   };
 
   AddressTranslator();
+  AddressTranslator(AddressTranslator&&);
+  AddressTranslator(const AddressTranslator&) = delete;
+  const AddressTranslator& operator=(const AddressTranslator&) = delete;
   ~AddressTranslator();
 
   // Consumes |units| to populate data in this class. Performs consistency
@@ -190,8 +192,6 @@ class AddressTranslator {
 
   // Conversion factor to translate between dangling RVAs and fake offsets.
   offset_t fake_offset_begin_;
-
-  DISALLOW_COPY_AND_ASSIGN(AddressTranslator);
 };
 
 }  // namespace zucchini
diff --git a/disassembler_elf.cc b/disassembler_elf.cc
index 4727d1c..37cff0b 100644
--- a/disassembler_elf.cc
+++ b/disassembler_elf.cc
@@ -417,35 +417,31 @@ std::vector<ReferenceGroup> DisassemblerElfIntel<Traits>::MakeReferenceGroups()
 template <class Traits>
 void DisassemblerElfIntel<Traits>::ParseExecSection(
     const typename Traits::Elf_Shdr& section) {
+  // |this->| is needed to access protected members of templated base class. To
+  // reduce noise, use local references for these.
   ConstBufferView& image_ = this->image_;
+  const AddressTranslator& translator_ = this->translator_;
   auto& abs32_locations_ = this->abs32_locations_;
 
-  std::ptrdiff_t from_offset_to_rva = section.sh_addr - section.sh_offset;
-
   // Range of values was ensured in ParseHeader().
   rva_t start_rva = base::checked_cast<rva_t>(section.sh_addr);
   rva_t end_rva = base::checked_cast<rva_t>(start_rva + section.sh_size);
 
-  AddressTranslator::RvaToOffsetCache target_rva_checker(this->translator_);
+  AddressTranslator::RvaToOffsetCache target_rva_checker(translator_);
 
   ConstBufferView region(image_.begin() + section.sh_offset, section.sh_size);
   Abs32GapFinder gap_finder(image_, region, abs32_locations_, 4);
-  typename Traits::Rel32FinderUse finder;
-  for (auto gap = gap_finder.GetNext(); gap.has_value();
-       gap = gap_finder.GetNext()) {
-    finder.SetRegion(gap.value());
-    for (auto rel32 = finder.GetNext(); rel32.has_value();
-         rel32 = finder.GetNext()) {
-      offset_t rel32_offset =
-          base::checked_cast<offset_t>(rel32->location - image_.begin());
-      rva_t rel32_rva = rva_t(rel32_offset + from_offset_to_rva);
-      DCHECK_NE(rel32_rva, kInvalidRva);
-      rva_t target_rva = rel32_rva + 4 + image_.read<uint32_t>(rel32_offset);
-      if (target_rva_checker.IsValid(target_rva) &&
-          (rel32->can_point_outside_section ||
-           (start_rva <= target_rva && target_rva < end_rva))) {
-        finder.Accept();
-        rel32_locations_.push_back(rel32_offset);
+  typename Traits::Rel32FinderUse rel_finder(image_, translator_);
+  // Iterate over gaps between abs32 references, to avoid collision.
+  while (gap_finder.FindNext()) {
+    rel_finder.SetRegion(gap_finder.GetGap());
+    while (rel_finder.FindNext()) {
+      auto rel32 = rel_finder.GetRel32();
+      if (target_rva_checker.IsValid(rel32.target_rva) &&
+          (rel32.can_point_outside_section ||
+           (start_rva <= rel32.target_rva && rel32.target_rva < end_rva))) {
+        rel_finder.Accept();
+        rel32_locations_.push_back(rel32.location);
       }
     }
   }
diff --git a/disassembler_win32.cc b/disassembler_win32.cc
index da830bf..a23201b 100644
--- a/disassembler_win32.cc
+++ b/disassembler_win32.cc
@@ -363,7 +363,6 @@ bool DisassemblerWin32<Traits>::ParseAndStoreRel32() {
 
   ParseAndStoreAbs32();
 
-  AddressTranslator::OffsetToRvaCache location_offset_to_rva(translator_);
   AddressTranslator::RvaToOffsetCache target_rva_checker(translator_);
 
   for (const pe::ImageSectionHeader& section : sections_) {
@@ -382,24 +381,18 @@ bool DisassemblerWin32<Traits>::ParseAndStoreRel32() {
         image_[{section.file_offset_of_raw_data, size_to_use}];
     Abs32GapFinder gap_finder(image_, region, abs32_locations_,
                               Traits::kVAWidth);
-    typename Traits::RelFinder finder;
+    typename Traits::RelFinder rel_finder(image_, translator_);
     // Iterate over gaps between abs32 references, to avoid collision.
-    for (auto gap = gap_finder.GetNext(); gap.has_value();
-         gap = gap_finder.GetNext()) {
-      finder.SetRegion(gap.value());
-      // Iterate over heuristically detected rel32 references, validate, and add
-      // to |rel32_locations_|.
-      for (auto rel32 = finder.GetNext(); rel32.has_value();
-           rel32 = finder.GetNext()) {
-        offset_t rel32_offset = offset_t(rel32->location - image_.begin());
-        rva_t rel32_rva = location_offset_to_rva.Convert(rel32_offset);
-        DCHECK_NE(rel32_rva, kInvalidRva);
-        rva_t target_rva = rel32_rva + 4 + image_.read<uint32_t>(rel32_offset);
-        if (target_rva_checker.IsValid(target_rva) &&
-            (rel32->can_point_outside_section ||
-             (start_rva <= target_rva && target_rva < end_rva))) {
-          finder.Accept();
-          rel32_locations_.push_back(rel32_offset);
+    while (gap_finder.FindNext()) {
+      rel_finder.SetRegion(gap_finder.GetGap());
+      // Heuristically detect rel32 references, store if valid.
+      while (rel_finder.FindNext()) {
+        auto rel32 = rel_finder.GetRel32();
+        if (target_rva_checker.IsValid(rel32.target_rva) &&
+            (rel32.can_point_outside_section ||
+             (start_rva <= rel32.target_rva && rel32.target_rva < end_rva))) {
+          rel_finder.Accept();
+          rel32_locations_.push_back(rel32.location);
         }
       }
     }
diff --git a/rel32_finder.cc b/rel32_finder.cc
index 45810d6..ccb26f4 100644
--- a/rel32_finder.cc
+++ b/rel32_finder.cc
@@ -26,44 +26,43 @@ Abs32GapFinder::Abs32GapFinder(ConstBufferView image,
 
   const offset_t begin_offset =
       base::checked_cast<offset_t>(region.begin() - image.begin());
-  // Find the first |abs32_current_| with |*abs32_current_ >= begin_offset|.
-  abs32_current_ = std::lower_bound(abs32_locations.begin(),
-                                    abs32_locations.end(), begin_offset);
+  // Find the first |abs32_cur_| with |*abs32_cur_ >= begin_offset|.
+  abs32_cur_ = std::lower_bound(abs32_locations.begin(), abs32_locations.end(),
+                                begin_offset);
 
-  // Find lower boundary, accounting for possibility that |abs32_current_[-1]|
+  // Find lower boundary, accounting for the possibility that |abs32_cur_[-1]|
   // may straddle across |region.begin()|.
-  current_lo_ = region.begin();
-  if (abs32_current_ > abs32_locations.begin()) {
-    current_lo_ = std::max(current_lo_,
-                           image.begin() + abs32_current_[-1] + abs32_width_);
-  }
+  cur_lo_ = region.begin();
+  if (abs32_cur_ > abs32_locations.begin())
+    cur_lo_ = std::max(cur_lo_, image.begin() + abs32_cur_[-1] + abs32_width_);
 }
 
 Abs32GapFinder::~Abs32GapFinder() = default;
 
-absl::optional<ConstBufferView> Abs32GapFinder::GetNext() {
-  // Iterate over |[abs32_current_, abs32_end_)| and emit segments.
-  while (abs32_current_ != abs32_end_ &&
-         base_ + *abs32_current_ < region_end_) {
-    ConstBufferView::const_iterator hi = base_ + *abs32_current_;
-    ConstBufferView gap = ConstBufferView::FromRange(current_lo_, hi);
-    current_lo_ = hi + abs32_width_;
-    ++abs32_current_;
-    if (!gap.empty())
-      return gap;
+bool Abs32GapFinder::FindNext() {
+  // Iterate over |[abs32_cur_, abs32_end_)| and emit segments.
+  while (abs32_cur_ != abs32_end_ && base_ + *abs32_cur_ < region_end_) {
+    ConstBufferView::const_iterator hi = base_ + *abs32_cur_;
+    gap_ = ConstBufferView::FromRange(cur_lo_, hi);
+    cur_lo_ = hi + abs32_width_;
+    ++abs32_cur_;
+    if (!gap_.empty())
+      return true;
   }
   // Emit final segment.
-  if (current_lo_ < region_end_) {
-    ConstBufferView gap = ConstBufferView::FromRange(current_lo_, region_end_);
-    current_lo_ = region_end_;
-    return gap;
+  if (cur_lo_ < region_end_) {
+    gap_ = ConstBufferView::FromRange(cur_lo_, region_end_);
+    cur_lo_ = region_end_;
+    return true;
   }
-  return absl::nullopt;
+  return false;
 }
 
 /******** Rel32Finder ********/
 
-Rel32Finder::Rel32Finder() {}
+Rel32Finder::Rel32Finder(ConstBufferView image,
+                         const AddressTranslator& translator)
+    : image_(image), offset_to_rva_(translator) {}
 
 Rel32Finder::~Rel32Finder() = default;
 
@@ -95,7 +94,12 @@ Rel32Finder::NextIterators Rel32FinderIntel::SetResult(
     ConstBufferView::const_iterator cursor,
     uint32_t opcode_size,
     bool can_point_outside_section) {
-  rel32_ = {cursor + opcode_size, can_point_outside_section};
+  offset_t location =
+      base::checked_cast<offset_t>((cursor + opcode_size) - image_.begin());
+  rva_t location_rva = offset_to_rva_.Convert(location);
+  DCHECK_NE(location_rva, kInvalidRva);
+  rva_t target_rva = location_rva + 4 + image_.read<uint32_t>(location);
+  rel32_ = {location, target_rva, can_point_outside_section};
   return {cursor + 1, cursor + (opcode_size + 4)};
 }
 
diff --git a/rel32_finder.h b/rel32_finder.h
index 5953755..96a23b9 100644
--- a/rel32_finder.h
+++ b/rel32_finder.h
@@ -9,10 +9,9 @@
 
 #include <vector>
 
-#include "base/macros.h"
+#include "components/zucchini/address_translator.h"
 #include "components/zucchini/buffer_view.h"
 #include "components/zucchini/image_utils.h"
-#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace zucchini {
 
@@ -49,34 +48,58 @@ class Abs32GapFinder {
                  ConstBufferView region,
                  const std::vector<offset_t>& abs32_locations,
                  size_t abs32_width);
+  Abs32GapFinder(const Abs32GapFinder&) = delete;
+  const Abs32GapFinder& operator=(const Abs32GapFinder&) = delete;
   ~Abs32GapFinder();
 
-  // Returns the next available gap, or nullopt if exhausted.
-  absl::optional<ConstBufferView> GetNext();
+  // Searches for the next available gap, and returns successfulness.
+  bool FindNext();
+
+  // Returns the cached result from the last successful FindNext().
+  ConstBufferView GetGap() const { return gap_; }
 
  private:
   const ConstBufferView::const_iterator base_;
   const ConstBufferView::const_iterator region_end_;
-  ConstBufferView::const_iterator current_lo_;
-  std::vector<offset_t>::const_iterator abs32_current_;
-  std::vector<offset_t>::const_iterator abs32_end_;
-  size_t abs32_width_;
-
-  DISALLOW_COPY_AND_ASSIGN(Abs32GapFinder);
+  ConstBufferView::const_iterator cur_lo_;
+  const std::vector<offset_t>::const_iterator abs32_end_;
+  std::vector<offset_t>::const_iterator abs32_cur_;
+  const size_t abs32_width_;
+  ConstBufferView gap_;
 };
 
 // A class to scan regions within an image to find successive rel32 references.
 // Architecture-specific parsing and result extraction are delegated to
-// inherited classes. This is typically used along with Abs32GapFinder to find
-// search regions.
+// inherited classes (say, Rel32Finder_Impl). Sample extraction loop, combined
+// with Abs32GapFinder usage:
+//
+//   Abs32GapFinder gap_finder(...);
+//   Rel32Finder_Impl finder(...);
+//   while (gap_finder.FindNext()) {
+//     rel_finder.SetRegion(gap_finder.GetGap());
+//     while (rel_finder.FindNext()) {
+//       auto rel32 = rel_finder.GetRel32();  // In Rel32Finder_Impl.
+//       if (architecture_specific_validation(rel32)) {
+//         rel_finder.Accept();
+//         // Store rel32.
+//       }
+//     }
+//   }
 class Rel32Finder {
  public:
-  Rel32Finder();
+  Rel32Finder(ConstBufferView image, const AddressTranslator& translator);
+  Rel32Finder(const Rel32Finder&) = delete;
+  const Rel32Finder& operator=(const Rel32Finder&) = delete;
   virtual ~Rel32Finder();
 
   // Assigns the scan |region| for rel32 references to enable FindNext() use.
   void SetRegion(ConstBufferView region);
 
+  // Scans for the next rel32 reference, and returns whether any is found, so a
+  // "while" loop can be used for iterative rel32 extraction. The results are
+  // cached in Rel32Finder_Impl and obtained by Rel32Finder_Impl::GetRel32().
+  bool FindNext();
+
   // When a rel32 reference is found, the caller needs to decide whether to keep
   // the result (perhaps following more validation). If it decides to keep the
   // result, then it must call Accept(), so the next call to FindNext() can skip
@@ -98,11 +121,6 @@ class Rel32Finder {
     ConstBufferView::const_iterator accept;
   };
 
-  // Scans for the next rel32 reference, and returns whether any is found, so a
-  // "while" loop can be used for iterative rel32 extraction. The results are
-  // cached in Rel32Finder_Impl and obtained by Rel32Finder_Impl::GetRel32().
-  bool FindNext();
-
   // Detects and extracts architecture-specific rel32 reference. For each one
   // found, the implementation should cache the necessary data to be retrieved
   // via accessors. Returns a NextIterators that stores alternatives for where
@@ -110,20 +128,25 @@ class Rel32Finder {
   // NextIterators are nulls.
   virtual NextIterators Scan(ConstBufferView region) = 0;
 
+  const ConstBufferView image_;
+  AddressTranslator::OffsetToRvaCache offset_to_rva_;
+
  private:
   ConstBufferView region_;
   ConstBufferView::const_iterator accept_it_ = nullptr;
-
-  DISALLOW_COPY_AND_ASSIGN(Rel32Finder);
 };
 
 // Parsing for X86 or X64: we perform naive scan for opcodes that have rel32 as
 // an argument, and disregard instruction alignment.
 class Rel32FinderIntel : public Rel32Finder {
  public:
-  // Struct to store GetNext() results.
+  Rel32FinderIntel(const Rel32FinderIntel&) = delete;
+  const Rel32FinderIntel& operator=(const Rel32FinderIntel&) = delete;
+
+  // Struct to store GetRel32() results.
   struct Result {
-    ConstBufferView::const_iterator location;
+    offset_t location;
+    rva_t target_rva;
 
     // Some references must have their target in the same section as location,
     // which we use this to heuristically reject rel32 reference candidates.
@@ -133,12 +156,8 @@ class Rel32FinderIntel : public Rel32Finder {
 
   using Rel32Finder::Rel32Finder;
 
-  // Returns the next available Result, or nullopt if exhausted.
-  absl::optional<Result> GetNext() {
-    if (FindNext())
-      return rel32_;
-    return absl::nullopt;
-  }
+  // Returns the cached result from the last successful FindNext().
+  const Result& GetRel32() { return rel32_; }
 
  protected:
   // Helper for Scan() that also assigns |rel32_|.
@@ -151,9 +170,6 @@ class Rel32FinderIntel : public Rel32Finder {
 
   // Rel32Finder:
   NextIterators Scan(ConstBufferView region) override = 0;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(Rel32FinderIntel);
 };
 
 // X86 instructions.
@@ -161,11 +177,12 @@ class Rel32FinderX86 : public Rel32FinderIntel {
  public:
   using Rel32FinderIntel::Rel32FinderIntel;
 
+  Rel32FinderX86(const Rel32FinderX86&) = delete;
+  const Rel32FinderX86& operator=(const Rel32FinderX86&) = delete;
+
  private:
   // Rel32Finder:
   NextIterators Scan(ConstBufferView region) override;
-
-  DISALLOW_COPY_AND_ASSIGN(Rel32FinderX86);
 };
 
 // X64 instructions.
@@ -173,11 +190,12 @@ class Rel32FinderX64 : public Rel32FinderIntel {
  public:
   using Rel32FinderIntel::Rel32FinderIntel;
 
+  Rel32FinderX64(const Rel32FinderX64&) = delete;
+  const Rel32FinderX64& operator=(const Rel32FinderX64&) = delete;
+
  private:
   // Rel32Finder:
   NextIterators Scan(ConstBufferView region) override;
-
-  DISALLOW_COPY_AND_ASSIGN(Rel32FinderX64);
 };
 
 }  // namespace zucchini
diff --git a/rel32_finder_unittest.cc b/rel32_finder_unittest.cc
index 42a442c..8f274d5 100644
--- a/rel32_finder_unittest.cc
+++ b/rel32_finder_unittest.cc
@@ -40,9 +40,10 @@ TEST(Abs32GapFinderTest, All) {
     Abs32GapFinder gap_finder(image, region, abs32_locations, abs32_width);
 
     std::string out_str;
-    for (auto gap = gap_finder.GetNext(); gap; gap = gap_finder.GetNext()) {
-      size_t lo = static_cast<size_t>(gap->begin() - image.begin());
-      size_t hi = static_cast<size_t>(gap->end() - image.begin());
+    while (gap_finder.FindNext()) {
+      ConstBufferView gap = gap_finder.GetGap();
+      size_t lo = static_cast<size_t>(gap.begin() - image.begin());
+      size_t hi = static_cast<size_t>(gap.end() - image.begin());
       out_str.append(base::StringPrintf("[%" PRIuS ",%" PRIuS ")", lo, hi));
     }
     return out_str;
@@ -125,19 +126,24 @@ class TestRel32Finder : public Rel32Finder {
   // Rel32Finder:
   NextIterators Scan(ConstBufferView region) override { return next_result; }
 
-  bool GetNext() { return FindNext(); }
-
   NextIterators next_result;
 };
 
+AddressTranslator GetTrivialTranslator(size_t size) {
+  AddressTranslator translator;
+  EXPECT_EQ(AddressTranslator::kSuccess,
+            translator.Initialize({{0, size, 0U, size}}));
+  return translator;
+}
+
 }  // namespace
 
 TEST(Rel32FinderTest, Scan) {
   const size_t kRegionTotal = 99;
   std::vector<uint8_t> buffer(kRegionTotal);
   ConstBufferView image(buffer.data(), buffer.size());
-
-  TestRel32Finder finder;
+  AddressTranslator translator(GetTrivialTranslator(image.size()));
+  TestRel32Finder finder(image, translator);
   finder.SetRegion(image);
 
   auto check_finder_state = [&](const TestRel32Finder& finder,
@@ -153,88 +159,135 @@ TEST(Rel32FinderTest, Scan) {
   check_finder_state(finder, 0, 0);
 
   finder.next_result = {image.begin() + 1, image.begin() + 1};
-  EXPECT_TRUE(finder.GetNext());
+  EXPECT_TRUE(finder.FindNext());
   check_finder_state(finder, 1, 1);
 
   finder.next_result = {image.begin() + 2, image.begin() + 2};
-  EXPECT_TRUE(finder.GetNext());
+  EXPECT_TRUE(finder.FindNext());
   check_finder_state(finder, 2, 2);
 
   finder.next_result = {image.begin() + 5, image.begin() + 6};
-  EXPECT_TRUE(finder.GetNext());
+  EXPECT_TRUE(finder.FindNext());
   check_finder_state(finder, 5, 6);
   finder.Accept();
   check_finder_state(finder, 6, 6);
 
   finder.next_result = {image.begin() + 7, image.begin() + 7};
-  EXPECT_TRUE(finder.GetNext());
+  EXPECT_TRUE(finder.FindNext());
   check_finder_state(finder, 7, 7);
 
   finder.next_result = {image.begin() + 8, image.begin() + 8};
-  EXPECT_TRUE(finder.GetNext());
+  EXPECT_TRUE(finder.FindNext());
   check_finder_state(finder, 8, 8);
 
   finder.next_result = {image.begin() + 99, image.begin() + 99};
-  EXPECT_TRUE(finder.GetNext());
+  EXPECT_TRUE(finder.FindNext());
   check_finder_state(finder, 99, 99);
 
   finder.next_result = {nullptr, nullptr};
-  EXPECT_FALSE(finder.GetNext());
+  EXPECT_FALSE(finder.FindNext());
   check_finder_state(finder, 99, 99);
 }
 
-TEST(Rel32FinderX86Test, FindNext) {
-  constexpr uint8_t data[] = {
-      0x55,                                // 00: push  ebp
-      0x8B, 0xEC,                          // 01: mov   ebp,esp
-      0xE8, 0x00, 0x00, 0x00, 0x00,        // 03: call  08
-      0xE9, 0x00, 0x00, 0x00, 0x00,        // 08: jmp   0D
-      0x0F, 0x80, 0x00, 0x00, 0x00, 0x00,  // 0D: jo    13
-      0x0F, 0x81, 0x00, 0x00, 0x00, 0x00,  // 13: jno   19
-      0x0F, 0x82, 0x00, 0x00, 0x00, 0x00,  // 19: jb    1F
-      0x0F, 0x83, 0x00, 0x00, 0x00, 0x00,  // 1F: jae   25
-      0x0F, 0x84, 0x00, 0x00, 0x00, 0x00,  // 25: je    2B
-      0x0F, 0x85, 0x00, 0x00, 0x00, 0x00,  // 2B: jne   31
-      0x0F, 0x86, 0x00, 0x00, 0x00, 0x00,  // 31: jbe   37
-      0x0F, 0x87, 0x00, 0x00, 0x00, 0x00,  // 37: ja    3D
-      0x0F, 0x88, 0x00, 0x00, 0x00, 0x00,  // 3D: js    43
-      0x0F, 0x89, 0x00, 0x00, 0x00, 0x00,  // 43: jns   49
-      0x0F, 0x8A, 0x00, 0x00, 0x00, 0x00,  // 49: jp    4F
-      0x0F, 0x8B, 0x00, 0x00, 0x00, 0x00,  // 4F: jnp   55
-      0x0F, 0x8C, 0x00, 0x00, 0x00, 0x00,  // 55: jl    5B
-      0x0F, 0x8D, 0x00, 0x00, 0x00, 0x00,  // 5B: jge   61
-      0x0F, 0x8E, 0x00, 0x00, 0x00, 0x00,  // 61: jle   67
-      0x0F, 0x8F, 0x00, 0x00, 0x00, 0x00,  // 67: jg    6D
-      0x5D,                                // 6D: pop   ebp
-      0xC3,                                // C3: ret
-  };
+namespace {
 
-  ConstBufferView image =
-      ConstBufferView::FromRange(std::begin(data), std::end(data));
+// X86 test data. (x) and +x entries are covered by abs32 references, which have
+// width = 4.
+constexpr uint8_t kDataX86[] = {
+    0x55,                                // 00: push  ebp
+    0x8B,   0xEC,                        // 01: mov   ebp,esp
+    0xE8,   0,      0,   0,   0,         // 03: call  08
+    (0xE9), +0,     +0,  +0,  0,         // 08: jmp   0D
+    0x0F,   0x80,   0,   0,   0,   0,    // 0D: jo    13
+    0x0F,   0x81,   0,   0,   (0), +0,   // 13: jno   19
+    +0x0F,  +0x82,  0,   0,   0,   0,    // 19: jb    1F
+    0x0F,   0x83,   0,   0,   0,   0,    // 1F: jae   25
+    0x0F,   (0x84), +0,  +0,  +0,  (0),  // 25: je    2B
+    +0x0F,  +0x85,  +0,  0,   0,   0,    // 2B: jne   31
+    0x0F,   0x86,   0,   0,   0,   0,    // 31: jbe   37
+    0x0F,   0x87,   0,   0,   0,   0,    // 37: ja    3D
+    0x0F,   0x88,   0,   (0), +0,  +0,   // 3D: js    43
+    +0x0F,  0x89,   0,   0,   0,   0,    // 43: jns   49
+    0x0F,   0x8A,   0,   0,   0,   0,    // 49: jp    4F
+    0x0F,   0x8B,   (0), +0,  +0,  +0,   // 4F: jnp   55
+    0x0F,   0x8C,   0,   0,   0,   0,    // 55: jl    5B
+    0x0F,   0x8D,   0,   0,   (0), +0,   // 5B: jge   61
+    +0x0F,  +0x8E,  (0), +0,  +0,  +0,   // 61: jle   67
+    0x0F,   0x8F,   0,   0,   0,   0,    // 67: jg    6D
+    0x5D,                                // 6D: pop   ebp
+    0xC3,                                // C3: ret
+};
 
-  Rel32FinderX86 rel_finder;
+// Abs32 locations corresponding to |kDataX86|, with width = 4.
+constexpr uint8_t kAbs32X86[] = {0x08, 0x17, 0x26, 0x2A,
+                                 0x40, 0x51, 0x5F, 0x63};
+
+}  // namespace
+
+TEST(Rel32FinderX86Test, FindNext) {
+  ConstBufferView image =
+      ConstBufferView::FromRange(std::begin(kDataX86), std::end(kDataX86));
+  AddressTranslator translator(GetTrivialTranslator(image.size()));
+  Rel32FinderX86 rel_finder(image, translator);
   rel_finder.SetRegion(image);
 
-  // List of expected locations as pairs of {cursor offset, rel32 offset}.
+  // List of expected locations as pairs of {cursor offset, rel32 offset},
+  // ignoring |kAbs32X86|.
   std::vector<std::pair<size_t, size_t>> expected_locations = {
       {0x04, 0x04}, {0x09, 0x09}, {0x0E, 0x0F}, {0x14, 0x15}, {0x1A, 0x1B},
       {0x20, 0x21}, {0x26, 0x27}, {0x2C, 0x2D}, {0x32, 0x33}, {0x38, 0x39},
       {0x3E, 0x3F}, {0x44, 0x45}, {0x4A, 0x4B}, {0x50, 0x51}, {0x56, 0x57},
       {0x5C, 0x5D}, {0x62, 0x63}, {0x68, 0x69},
   };
-
   for (auto location : expected_locations) {
-    auto result = rel_finder.GetNext();
-    EXPECT_TRUE(result.has_value());
+    EXPECT_TRUE(rel_finder.FindNext());
+    auto rel32 = rel_finder.GetRel32();
 
     EXPECT_EQ(location.first,
               size_t(rel_finder.region().begin() - image.begin()));
-    EXPECT_EQ(location.second, size_t(result->location - image.begin()));
-    EXPECT_EQ(result->location + 4, rel_finder.accept_it());
-    EXPECT_FALSE(result->can_point_outside_section);
+    EXPECT_EQ(location.second, rel32.location);
+    EXPECT_EQ(image.begin() + (rel32.location + 4), rel_finder.accept_it());
+    EXPECT_FALSE(rel32.can_point_outside_section);
     rel_finder.Accept();
   }
-  EXPECT_EQ(absl::nullopt, rel_finder.GetNext());
+  EXPECT_FALSE(rel_finder.FindNext());
+}
+
+TEST(Rel32FinderX86Test, Integrated) {
+  // Truncated form of Rel32FinderIntel::Result.
+  typedef std::pair<offset_t, rva_t> TruncatedResults;
+
+  ConstBufferView image =
+      ConstBufferView::FromRange(std::begin(kDataX86), std::end(kDataX86));
+  std::vector<offset_t> abs32_locations(std::begin(kAbs32X86),
+                                        std::end(kAbs32X86));
+  std::vector<TruncatedResults> rel32_results;
+
+  Abs32GapFinder gap_finder(image, image, abs32_locations, 4U);
+  AddressTranslator translator(GetTrivialTranslator(image.size()));
+  Rel32FinderX86 rel_finder(image, translator);
+  while (gap_finder.FindNext()) {
+    auto gap = gap_finder.GetGap();
+    rel_finder.SetRegion(gap_finder.GetGap());
+    while (rel_finder.FindNext()) {
+      auto rel32 = rel_finder.GetRel32();
+      rel32_results.emplace_back(
+          TruncatedResults{rel32.location, rel32.target_rva});
+    }
+  }
+
+  std::vector<TruncatedResults> expected_rel32_results = {
+      {0x04, 0x08},
+      /* {0x09, 0x0D}, */ {0x0F, 0x13},
+      /* {0x15, 0x19}, */ /*{0x1B, 0x1F}, */
+      {0x21, 0x25},
+      /* {0x27, 0x2B}, */ /* {0x2D, 0x31}, */ {0x33, 0x37},
+      {0x39, 0x3D},
+      /* {0x3F, 0x43}, */ /* {0x45, 0x49}, */ {0x4B, 0x4F},
+      /* {0x51, 0x55}, */ {0x57, 0x5B},
+      /* {0x5D, 0x61}, */ /* {0x63, 0x67}, */ {0x69, 0x6D},
+  };
+  EXPECT_EQ(expected_rel32_results, rel32_results);
 }
 
 TEST(Rel32FinderX86Test, Accept) {
@@ -247,13 +300,14 @@ TEST(Rel32FinderX86Test, Accept) {
   ConstBufferView image =
       ConstBufferView::FromRange(std::begin(data), std::end(data));
 
-  auto next_location = [&](Rel32FinderX86& rel_finder) -> size_t {
-    auto result = rel_finder.GetNext();
-    EXPECT_TRUE(result.has_value());
-    return result->location - image.begin();
+  auto next_location = [](Rel32FinderX86& rel_finder) -> offset_t {
+    EXPECT_TRUE(rel_finder.FindNext());
+    auto rel32 = rel_finder.GetRel32();
+    return rel32.location;
   };
 
-  Rel32FinderX86 rel_finder;
+  AddressTranslator translator(GetTrivialTranslator(image.size()));
+  Rel32FinderX86 rel_finder(image, translator);
   rel_finder.SetRegion(image);
 
   EXPECT_EQ(0x05U, next_location(rel_finder));  // False positive.
@@ -264,57 +318,68 @@ TEST(Rel32FinderX86Test, Accept) {
   EXPECT_EQ(0x0BU, next_location(rel_finder));  // Found if 0x0A is discarded.
 }
 
-TEST(Rel32FinderX64Test, FindNext) {
-  constexpr uint8_t data[] = {
-      0x55,                                      // 00: push  ebp
-      0x8B, 0xEC,                                // 01: mov   ebp,esp
-      0xE8, 0x00, 0x00, 0x00, 0x00,              // 03: call  08
-      0xE9, 0x00, 0x00, 0x00, 0x00,              // 08: jmp   0D
-      0x0F, 0x80, 0x00, 0x00, 0x00, 0x00,        // 0D: jo    13
-      0x0F, 0x81, 0x00, 0x00, 0x00, 0x00,        // 13: jno   19
-      0x0F, 0x82, 0x00, 0x00, 0x00, 0x00,        // 19: jb    1F
-      0x0F, 0x83, 0x00, 0x00, 0x00, 0x00,        // 1F: jae   25
-      0x0F, 0x84, 0x00, 0x00, 0x00, 0x00,        // 25: je    2B
-      0x0F, 0x85, 0x00, 0x00, 0x00, 0x00,        // 2B: jne   31
-      0x0F, 0x86, 0x00, 0x00, 0x00, 0x00,        // 31: jbe   37
-      0x0F, 0x87, 0x00, 0x00, 0x00, 0x00,        // 37: ja    3D
-      0x0F, 0x88, 0x00, 0x00, 0x00, 0x00,        // 3D: js    43
-      0x0F, 0x89, 0x00, 0x00, 0x00, 0x00,        // 43: jns   49
-      0x0F, 0x8A, 0x00, 0x00, 0x00, 0x00,        // 49: jp    4F
-      0x0F, 0x8B, 0x00, 0x00, 0x00, 0x00,        // 4F: jnp   55
-      0x0F, 0x8C, 0x00, 0x00, 0x00, 0x00,        // 55: jl    5B
-      0x0F, 0x8D, 0x00, 0x00, 0x00, 0x00,        // 5B: jge   61
-      0x0F, 0x8E, 0x00, 0x00, 0x00, 0x00,        // 61: jle   67
-      0x0F, 0x8F, 0x00, 0x00, 0x00, 0x00,        // 67: jg    6F
-      0xFF, 0x15, 0x00, 0x00, 0x00, 0x00,        // 6D: call  [rip+00]
-      0xFF, 0x25, 0x00, 0x00, 0x00, 0x00,        // 73: jmp   [rip+00]
-      0x8B, 0x05, 0x00, 0x00, 0x00, 0x00,        // 79: mov   eax,[rip+00]
-      0x8B, 0x3D, 0x00, 0x00, 0x00, 0x00,        // 7F: mov   edi,[rip+00]
-      0x8D, 0x05, 0x00, 0x00, 0x00, 0x00,        // 85: lea   eax,[rip+00]
-      0x8D, 0x3D, 0x00, 0x00, 0x00, 0x00,        // 8B: lea   edi,[rip+00]
-      0x48, 0x8B, 0x05, 0x00, 0x00, 0x00, 0x00,  // 91: mov   rax,[rip+00]
-      0x48, 0x8B, 0x3D, 0x00, 0x00, 0x00, 0x00,  // 98: mov   rdi,[rip+00]
-      0x48, 0x8D, 0x05, 0x00, 0x00, 0x00, 0x00,  // 9F: lea   rax,[rip+00]
-      0x48, 0x8D, 0x3D, 0x00, 0x00, 0x00, 0x00,  // A6: lea   rdi,[rip+00]
-      0x4C, 0x8B, 0x05, 0x00, 0x00, 0x00, 0x00,  // AD: mov   r8,[rip+00]
-      0x4C, 0x8B, 0x3D, 0x00, 0x00, 0x00, 0x00,  // B4: mov   r15,[rip+00]
-      0x4C, 0x8D, 0x05, 0x00, 0x00, 0x00, 0x00,  // BB: lea   r8,[rip+00]
-      0x4C, 0x8D, 0x3D, 0x00, 0x00, 0x00, 0x00,  // C2: lea   r15,[rip+00]
-      0x66, 0x8B, 0x05, 0x00, 0x00, 0x00, 0x00,  // C9: mov   ax,[rip+00]
-      0x66, 0x8B, 0x3D, 0x00, 0x00, 0x00, 0x00,  // D0: mov   di,[rip+00]
-      0x66, 0x8D, 0x05, 0x00, 0x00, 0x00, 0x00,  // D7: lea   ax,[rip+00]
-      0x66, 0x8D, 0x3D, 0x00, 0x00, 0x00, 0x00,  // DE: lea   di,[rip+00]
-      0x5D,                                      // E5: pop   ebp
-      0xC3,                                      // E6: ret
-  };
+namespace {
 
-  ConstBufferView image =
-      ConstBufferView::FromRange(std::begin(data), std::end(data));
+// X64 test data. (x) and +x entries are covered by abs32 references, which have
+// width = 8.
+constexpr uint8_t kDataX64[] = {
+    0x55,                                      // 00: push  ebp
+    0x8B,   0xEC,                              // 01: mov   ebp,esp
+    0xE8,   0,      0,     0,   0,             // 03: call  08
+    0xE9,   0,      0,     0,   (0),           // 08: jmp   0D
+    +0x0F,  +0x80,  +0,    +0,  +0,  +0,       // 0D: jo    13
+    +0x0F,  0x81,   0,     0,   0,   0,        // 13: jno   19
+    0x0F,   0x82,   0,     0,   0,   0,        // 19: jb    1F
+    (0x0F), +0x83,  +0,    +0,  +0,  +0,       // 1F: jae   25
+    +0x0F,  +0x84,  0,     0,   0,   0,        // 25: je    2B
+    0x0F,   0x85,   0,     0,   0,   0,        // 2B: jne   31
+    0x0F,   0x86,   (0),   +0,  +0,  +0,       // 31: jbe   37
+    +0x0F,  +0x87,  +0,    +0,  (0), +0,       // 37: ja    3D
+    +0x0F,  +0x88,  +0,    +0,  +0,  +0,       // 3D: js    43
+    0x0F,   0x89,   0,     0,   0,   0,        // 43: jns   49
+    (0x0F), +0x8A,  +0,    +0,  +0,  +0,       // 49: jp    4F
+    +0x0F,  +0x8B,  0,     0,   0,   0,        // 4F: jnp   55
+    0x0F,   0x8C,   0,     0,   0,   0,        // 55: jl    5B
+    0x0F,   0x8D,   0,     0,   0,   0,        // 5B: jge   61
+    0x0F,   0x8E,   0,     0,   0,   0,        // 61: jle   67
+    0x0F,   0x8F,   0,     (0), +0,  +0,       // 67: jg    6F
+    +0xFF,  +0x15,  +0,    +0,  +0,  0,        // 6D: call  [rip+00]      # 73
+    0xFF,   0x25,   0,     0,   0,   0,        // 73: jmp   [rip+00]      # 79
+    0x8B,   0x05,   0,     0,   0,   0,        // 79: mov   eax,[rip+00]  # 7F
+    0x8B,   0x3D,   0,     0,   0,   0,        // 7F: mov   edi,[rip+00]  # 85
+    0x8D,   0x05,   0,     0,   0,   0,        // 85: lea   eax,[rip+00]  # 8B
+    0x8D,   0x3D,   0,     0,   0,   0,        // 8B: lea   edi,[rip+00]  # 91
+    0x48,   0x8B,   0x05,  0,   0,   0,  0,    // 91: mov   rax,[rip+00]  # 98
+    0x48,   (0x8B), +0x3D, +0,  +0,  +0, +0,   // 98: mov   rdi,[rip+00]  # 9F
+    +0x48,  +0x8D,  0x05,  0,   0,   0,  0,    // 9F: lea   rax,[rip+00]  # A6
+    0x48,   0x8D,   0x3D,  0,   0,   0,  0,    // A6: lea   rdi,[rip+00]  # AD
+    0x4C,   0x8B,   0x05,  0,   0,   0,  (0),  // AD: mov   r8,[rip+00]   # B4
+    +0x4C,  +0x8B,  +0x3D, +0,  +0,  +0, +0,   // B4: mov   r15,[rip+00]  # BB
+    0x4C,   0x8D,   0x05,  0,   0,   0,  0,    // BB: lea   r8,[rip+00]   # C2
+    0x4C,   0x8D,   0x3D,  0,   0,   0,  0,    // C2: lea   r15,[rip+00]  # C9
+    0x66,   0x8B,   0x05,  (0), +0,  +0, +0,   // C9: mov   ax,[rip+00]   # D0
+    +0x66,  +0x8B,  +0x3D, +0,  0,   0,  0,    // D0: mov   di,[rip+00]   # D7
+    0x66,   0x8D,   0x05,  0,   0,   0,  0,    // D7: lea   ax,[rip+00]   # DE
+    0x66,   0x8D,   0x3D,  0,   0,   0,  0,    // DE: lea   di,[rip+00]   # E5
+    0x5D,                                      // E5: pop   ebp
+    0xC3,                                      // E6: ret
+};
+
+// Abs32 locations corresponding to |kDataX64|, with width = 8.
+constexpr uint8_t kAbs32X64[] = {0x0C, 0x1F, 0x33, 0x3B, 0x49,
+                                 0x6A, 0x99, 0xB3, 0xCC};
 
-  Rel32FinderX64 rel_finder;
+}  // namespace
+
+TEST(Rel32FinderX64Test, FindNext) {
+  ConstBufferView image =
+      ConstBufferView::FromRange(std::begin(kDataX64), std::end(kDataX64));
+  AddressTranslator translator(GetTrivialTranslator(image.size()));
+  Rel32FinderX64 rel_finder(image, translator);
   rel_finder.SetRegion(image);
 
-  // Lists of expected locations as pairs of {cursor offset, rel32 offset}.
+  // Lists of expected locations as pairs of {cursor offset, rel32 offset},
+  // ignoring |kAbs32X64|.
   std::vector<std::pair<size_t, size_t>> expected_locations = {
       {0x04, 0x04}, {0x09, 0x09}, {0x0E, 0x0F}, {0x14, 0x15}, {0x1A, 0x1B},
       {0x20, 0x21}, {0x26, 0x27}, {0x2C, 0x2D}, {0x32, 0x33}, {0x38, 0x39},
@@ -329,29 +394,76 @@ TEST(Rel32FinderX64Test, FindNext) {
   };
   // Jump instructions, which cannot point outside section.
   for (auto location : expected_locations) {
-    auto result = rel_finder.GetNext();
-    EXPECT_TRUE(result.has_value());
+    EXPECT_TRUE(rel_finder.FindNext());
+    auto rel32 = rel_finder.GetRel32();
     EXPECT_EQ(location.first,
               size_t(rel_finder.region().begin() - image.begin()));
-    EXPECT_EQ(location.second, size_t(result->location - image.begin()));
-    EXPECT_EQ(result->location + 4, rel_finder.accept_it());
-    EXPECT_FALSE(result->can_point_outside_section);
+    EXPECT_EQ(location.second, rel32.location);
+    EXPECT_EQ(image.begin() + (rel32.location + 4), rel_finder.accept_it());
+    EXPECT_FALSE(rel32.can_point_outside_section);
     rel_finder.Accept();
   }
   // PC-relative data access instructions, which can point outside section.
   for (auto location : expected_locations_rip) {
-    auto result = rel_finder.GetNext();
-    EXPECT_TRUE(result.has_value());
+    EXPECT_TRUE(rel_finder.FindNext());
+    auto rel32 = rel_finder.GetRel32();
     EXPECT_EQ(location.first,
               size_t(rel_finder.region().begin() - image.begin()));
-    EXPECT_EQ(location.second, size_t(result->location - image.begin()));
-    EXPECT_EQ(result->location + 4, rel_finder.accept_it());
-    EXPECT_TRUE(result->can_point_outside_section);
+    EXPECT_EQ(location.second, rel32.location);
+    EXPECT_EQ(image.begin() + (rel32.location + 4), rel_finder.accept_it());
+    EXPECT_TRUE(rel32.can_point_outside_section);  // Different from before.
     rel_finder.Accept();
   }
-  EXPECT_EQ(absl::nullopt, rel_finder.GetNext());
+  EXPECT_FALSE(rel_finder.FindNext());
 }
 
-// TODO(huangs): Test that integrates Abs32GapFinder and Rel32Finder.
+TEST(Rel32FinderX64Test, Integrated) {
+  // Truncated form of Rel32FinderIntel::Result.
+  typedef std::pair<offset_t, rva_t> TruncatedResults;
+
+  ConstBufferView image =
+      ConstBufferView::FromRange(std::begin(kDataX64), std::end(kDataX64));
+  std::vector<offset_t> abs32_locations(std::begin(kAbs32X64),
+                                        std::end(kAbs32X64));
+  std::vector<TruncatedResults> rel32_results;
+
+  Abs32GapFinder gap_finder(image, image, abs32_locations, 8U);
+  AddressTranslator translator(GetTrivialTranslator(image.size()));
+  Rel32FinderX64 rel_finder(image, translator);
+  while (gap_finder.FindNext()) {
+    auto gap = gap_finder.GetGap();
+    rel_finder.SetRegion(gap_finder.GetGap());
+    while (rel_finder.FindNext()) {
+      auto rel32 = rel_finder.GetRel32();
+      rel32_results.emplace_back(
+          TruncatedResults{rel32.location, rel32.target_rva});
+    }
+  }
+
+  std::vector<TruncatedResults> expected_rel32_results = {
+      {0x04, 0x08},
+      /* {0x09, 0x0D}, */
+      /*{0x0F, 0x13}, */ /* {0x15, 0x19}, */ {0x1B, 0x1F},
+      /* {0x21, 0x25}, */ /* {0x27, 0x2B}, */ {0x2D, 0x31},
+      /* {0x33, 0x37}, */ /* {0x39, 0x3D}, */
+      /* {0x3F, 0x43}, */ {0x45, 0x49},
+      /* {0x4B, 0x4F}, */ /* {0x51, 0x55}, */
+      {0x57, 0x5B},
+      {0x5D, 0x61},
+      {0x63, 0x67}, /* {0x69, 0x6F}, */
+      /* {0x6F, 0x73}, */ {0x75, 0x79},
+      {0x7B, 0x7F},
+      {0x81, 0x85},
+      {0x87, 0x8B},
+      {0x8D, 0x91},
+      {0x94, 0x98},
+      /* {0x9B, 0x9F}, */ /* {0xA2, 0xA6}, */ {0xA9, 0xAD},
+      /* {0xB0, 0xB4}, */ /* {0xB7, 0xBB}, */ {0xBE, 0xC2},
+      {0xC5, 0xC9},
+      /* {0xCC, 0xD0}, */ /* {0xD3, 0xD7}, */ {0xDA, 0xDE},
+      {0xE1, 0xE5},
+  };
+  EXPECT_EQ(expected_rel32_results, rel32_results);
+}
 
 }  // namespace zucchini