From fb345573ac09d9e282569d01fceadda5bb570bc4 Mon Sep 17 00:00:00 2001 From: Samuel Huang Date: Thu, 8 Nov 2018 21:53:27 +0000 Subject: [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 Reviewed-by: Samuel Huang Commit-Queue: Samuel Huang Cr-Commit-Position: refs/heads/master@{#606612} NOKEYCHECK=True GitOrigin-RevId: 9076fc4939ced233b85e5f8942ba947b6143aba3 --- buffer_view.h | 6 ------ disassembler_elf.cc | 18 +++++++----------- disassembler_elf.h | 5 +---- element_detection.cc | 8 ++++---- reloc_elf.cc | 3 --- reloc_elf.h | 2 +- 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::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 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::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 DisassemblerElfIntel::MakeReferenceGroups() {ReferenceTypeTraits{Traits::kVAWidth, TypeTag(kAbs32), PoolTag(kAbs32)}, &DisassemblerElfIntel::MakeReadAbs32, &DisassemblerElfIntel::MakeWriteAbs32}, + // N.B.: Rel32 |width| is 4 bytes, even for x64. {ReferenceTypeTraits{4, TypeTag(kRel32), PoolTag(kRel32)}, &DisassemblerElfIntel::MakeReadRel32, &DisassemblerElfIntel::MakeWriteRel32}}; @@ -405,8 +403,7 @@ std::unique_ptr DisassemblerElfIntel::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 DisassemblerElfIntel::MakeReadAbs32( template std::unique_ptr DisassemblerElfIntel::MakeWriteAbs32( MutableBufferView image) { - // TODO(huangs): For AArch64, see if |Traits::kBitness| should be used here? return std::make_unique( 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 DisassemblerElf : public Disassembler { public: - using HeaderVector = std::vector; - // 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 -#include -#include #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 +#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" -- cgit v1.2.3