[Zucchini] Cleanup before adding ARM support.

Update includes and comments, and remove some unused code.

In particular, remove TODO comments for figuring out whether ARM
abs32 references can be 4 bytes long: Turns out ARM absolute
references are 8 bytes long. It's rel32 refereneces that can be
4 bytes long.

Change-Id: I02dc905885f6cb5ff929efe0fb1f9a6593ee05a8
Reviewed-on: https://chromium-review.googlesource.com/c/1327559
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606612}
NOKEYCHECK=True
GitOrigin-RevId: 9076fc4939ced233b85e5f8942ba947b6143aba3

6 files changed, 13 insertions, 29 deletions

diff --git a/buffer_view.h b/buffer_view.h
index 727ebd1..bb86a9b 100644
--- a/buffer_view.h
+++ b/buffer_view.h
@@ -33,12 +33,6 @@ struct BufferRegion {
   size_t InclusiveClamp(size_t v) const {
     return zucchini::InclusiveClamp(v, lo(), hi());
   }
-  friend bool operator==(const BufferRegion& a, const BufferRegion& b) {
-    return a.offset == b.offset && a.size == b.size;
-  }
-  friend bool operator!=(const BufferRegion& a, const BufferRegion& b) {
-    return !(a == b);
-  }
 
   // Region data use size_t to match BufferViewBase::size_type, to make it
   // convenient to index into buffer view.
diff --git a/disassembler_elf.cc b/disassembler_elf.cc
index be01c4c..ef050a4 100644
--- a/disassembler_elf.cc
+++ b/disassembler_elf.cc
@@ -43,13 +43,13 @@ bool IsExecSection(const typename Traits::Elf_Shdr& section) {
 
 }  // namespace
 
-/******** ELF32Traits ********/
+/******** Elf32Traits ********/
 
 // static
 constexpr Bitness Elf32Traits::kBitness;
 constexpr elf::FileClass Elf32Traits::kIdentificationClass;
 
-/******** ELF32IntelTraits ********/
+/******** Elf32IntelTraits ********/
 
 // static
 constexpr ExecutableType Elf32IntelTraits::kExeType;
@@ -57,13 +57,13 @@ const char Elf32IntelTraits::kExeTypeString[] = "ELF x86";
 constexpr elf::MachineArchitecture Elf32IntelTraits::kMachineValue;
 constexpr uint32_t Elf32IntelTraits::kRelType;
 
-/******** ELF64Traits ********/
+/******** Elf64Traits ********/
 
 // static
 constexpr Bitness Elf64Traits::kBitness;
 constexpr elf::FileClass Elf64Traits::kIdentificationClass;
 
-/******** ELF64IntelTraits ********/
+/******** Elf64IntelTraits ********/
 
 // static
 constexpr ExecutableType Elf64IntelTraits::kExeType;
@@ -291,7 +291,7 @@ void DisassemblerElf<Traits>::GetAbs32FromRelocSections() {
   constexpr int kAbs32Width = Traits::kVAWidth;
   DCHECK(abs32_locations_.empty());
 
-  // Read reloc targets as preliminary abs32 locations.
+  // Read reloc targets to get preliminary abs32 locations.
   std::unique_ptr<ReferenceReader> relocs = MakeReadRelocs(0, offset_t(size()));
   for (auto ref = relocs->GetNext(); ref.has_value(); ref = relocs->GetNext())
     abs32_locations_.push_back(ref->target);
@@ -300,9 +300,6 @@ void DisassemblerElf<Traits>::GetAbs32FromRelocSections() {
 
   // Abs32 references must have targets translatable to offsets. Remove those
   // that are unable to do so.
-  // TODO(huangs): Investigate whether passing |Traits::kBitness| is correct:
-  // Some architectures using ELF might have 4-byte long abs32 body regardless
-  // of bitness.
   size_t num_untranslatable =
       RemoveUntranslatableAbs32(image_, {Traits::kBitness, kElfImageBase},
                                 translator_, &abs32_locations_);
@@ -352,6 +349,7 @@ std::vector<ReferenceGroup> DisassemblerElfIntel<Traits>::MakeReferenceGroups()
       {ReferenceTypeTraits{Traits::kVAWidth, TypeTag(kAbs32), PoolTag(kAbs32)},
        &DisassemblerElfIntel<Traits>::MakeReadAbs32,
        &DisassemblerElfIntel<Traits>::MakeWriteAbs32},
+      // N.B.: Rel32 |width| is 4 bytes, even for x64.
       {ReferenceTypeTraits{4, TypeTag(kRel32), PoolTag(kRel32)},
        &DisassemblerElfIntel<Traits>::MakeReadRel32,
        &DisassemblerElfIntel<Traits>::MakeWriteRel32}};
@@ -405,8 +403,7 @@ std::unique_ptr<ReferenceReader> DisassemblerElfIntel<Traits>::MakeReadAbs32(
     offset_t lo,
     offset_t hi) {
   // TODO(huangs): Don't use Abs32RvaExtractorWin32 here; use new class that
-  // caters to different ELF architectures (e.g., abs32 in AArch64 are 4 bytes
-  // long, not 8 bytes long).
+  // caters to different ELF architectures.
   Abs32RvaExtractorWin32 abs_rva_extractor(
       this->image_, AbsoluteAddress(Traits::kBitness, kElfImageBase),
       this->abs32_locations_, lo, hi);
@@ -417,7 +414,6 @@ std::unique_ptr<ReferenceReader> DisassemblerElfIntel<Traits>::MakeReadAbs32(
 template <class Traits>
 std::unique_ptr<ReferenceWriter> DisassemblerElfIntel<Traits>::MakeWriteAbs32(
     MutableBufferView image) {
-  // TODO(huangs): For AArch64, see if |Traits::kBitness| should be used here?
   return std::make_unique<Abs32WriterWin32>(
       image, AbsoluteAddress(Traits::kBitness, kElfImageBase),
       this->translator_);
diff --git a/disassembler_elf.h b/disassembler_elf.h
index d8012db..866c3b6 100644
--- a/disassembler_elf.h
+++ b/disassembler_elf.h
@@ -67,8 +67,6 @@ struct Elf64IntelTraits : public Elf64Traits {
 template <class Traits>
 class DisassemblerElf : public Disassembler {
  public:
-  using HeaderVector = std::vector<const typename Traits::Elf_Shdr*>;
-
   // Applies quick checks to determine whether |image| *may* point to the start
   // of an executable. Returns true iff the check passes.
   static bool QuickDetect(ConstBufferView image);
@@ -110,9 +108,8 @@ class DisassemblerElf : public Disassembler {
   // Processes rel32 data after they are extracted from executable sections.
   virtual void PostProcessRel32() = 0;
 
-  // The parsing routines below return true on success, and false on failure.
-
   // Parses ELF header and section headers, and performs basic validation.
+  // Returns whether parsing was successful.
   bool ParseHeader();
 
   // Extracts and stores section headers that we need.
diff --git a/element_detection.cc b/element_detection.cc
index fa5adf4..d4347c5 100644
--- a/element_detection.cc
+++ b/element_detection.cc
@@ -15,14 +15,14 @@
 #include "components/zucchini/disassembler_dex.h"
 #endif  // BUILDFLAG(ENABLE_DEX)
 
-#if BUILDFLAG(ENABLE_WIN)
-#include "components/zucchini/disassembler_win32.h"
-#endif  // BUILDFLAG(ENABLE_WIN)
-
 #if BUILDFLAG(ENABLE_ELF)
 #include "components/zucchini/disassembler_elf.h"
 #endif  // BUILDFLAG(ENABLE_ELF)
 
+#if BUILDFLAG(ENABLE_WIN)
+#include "components/zucchini/disassembler_win32.h"
+#endif  // BUILDFLAG(ENABLE_WIN)
+
 #if BUILDFLAG(ENABLE_ZTF)
 #include "components/zucchini/disassembler_ztf.h"
 #endif  // BUILDFLAG(ENABLE_ZTF)
diff --git a/reloc_elf.cc b/reloc_elf.cc
index 6a32184..eef1c96 100644
--- a/reloc_elf.cc
+++ b/reloc_elf.cc
@@ -5,11 +5,8 @@
 #include "components/zucchini/reloc_elf.h"
 
 #include <algorithm>
-#include <tuple>
-#include <utility>
 
 #include "base/logging.h"
-#include "base/numerics/safe_conversions.h"
 #include "components/zucchini/algorithm.h"
 
 namespace zucchini {
diff --git a/reloc_elf.h b/reloc_elf.h
index 1b5ba11..7fcdbd3 100644
--- a/reloc_elf.h
+++ b/reloc_elf.h
@@ -10,9 +10,9 @@
 
 #include <vector>
 
+#include "base/numerics/safe_conversions.h"
 #include "base/optional.h"
 #include "components/zucchini/address_translator.h"
-#include "components/zucchini/buffer_source.h"
 #include "components/zucchini/buffer_view.h"
 #include "components/zucchini/image_utils.h"
 #include "components/zucchini/type_elf.h"