[Zucchini] Disassemblers: Fix abs32 width for ELF; cleanup Traits template param.
Previously DisassemblerElfIntel<TRAITS>::ParseExecSection() passes a
hard-coded 4 to Abs32GapFinder's |abs32_width| CTOR param. This is
wrong for X64, which has abs32 pointer width of 8 bytes. This can lead
to lower quality rel32 extraction.
This CL fixes the above by replacing 4 with Traits::kVAWidth, and also
cleans up TRAITS / Traits template parameter for Disassembler:
* For template param, "template <class TRAITS>" is used throughout.
* This means function params needs to use TRAITS.
* For usage, each Disassembler class with TRAITS declares
using Traits = TRAITS;
(and variant) and uses Traits in the body of all functions. Reason:
Specialized derive classes won't have TRAITS available , so:
* Function params can use DisassemblerBase::Traits.
* Function bodies can use Traits.
* For consistency, even if TRAITS is available, still use Traits.
Bug: 1233831
Change-Id: Ie796c867fb238eca462b2fb6b4e68a965996c25a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3063919
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#908261}
NOKEYCHECK=True
GitOrigin-RevId: 294860c47cd3678c46422ce57da366724e1dc629
diff --git a/disassembler_elf.cc b/disassembler_elf.cc
index 07726a8..9c9ebb8 100644
--- a/disassembler_elf.cc
+++ b/disassembler_elf.cc
@@ -43,8 +43,8 @@
// Decides how a section affects ELF parsing, and returns a bit field composed
// from SectionJudgement values.
-template <class Traits>
-int JudgeSection(size_t image_size, const typename Traits::Elf_Shdr* section) {
+template <class TRAITS>
+int JudgeSection(size_t image_size, const typename TRAITS::Elf_Shdr* section) {
// BufferRegion uses |size_t| this can be 32-bit in some cases. For Elf64
// |sh_addr|, |sh_offset| and |sh_size| are 64-bit this can result in
// overflows in the subsequent validation steps.
@@ -96,21 +96,21 @@
}
// Determines whether |section| is a reloc section.
-template <class Traits>
-bool IsRelocSection(const typename Traits::Elf_Shdr& section) {
+template <class TRAITS>
+bool IsRelocSection(const typename TRAITS::Elf_Shdr& section) {
DCHECK_GT(section.sh_size, 0U);
if (section.sh_type == elf::SHT_REL) {
// Also validate |section.sh_entsize|, which gets used later.
- return section.sh_entsize == sizeof(typename Traits::Elf_Rel);
+ return section.sh_entsize == sizeof(typename TRAITS::Elf_Rel);
}
if (section.sh_type == elf::SHT_RELA)
- return section.sh_entsize == sizeof(typename Traits::Elf_Rela);
+ return section.sh_entsize == sizeof(typename TRAITS::Elf_Rela);
return false;
}
// Determines whether |section| is a section with executable code.
-template <class Traits>
-bool IsExecSection(const typename Traits::Elf_Shdr& section) {
+template <class TRAITS>
+bool IsExecSection(const typename TRAITS::Elf_Shdr& section) {
DCHECK_GT(section.sh_size, 0U);
return section.sh_type == elf::SHT_PROGBITS &&
(section.sh_flags & elf::SHF_EXECINSTR) != 0;
@@ -149,8 +149,8 @@
/******** DisassemblerElf ********/
// static.
-template <class Traits>
-bool DisassemblerElf<Traits>::QuickDetect(ConstBufferView image) {
+template <class TRAITS>
+bool DisassemblerElf<TRAITS>::QuickDetect(ConstBufferView image) {
BufferSource source(image);
// Do not consume the bytes for the magic value, as they are part of the
@@ -183,25 +183,25 @@
return true;
}
-template <class Traits>
-DisassemblerElf<Traits>::~DisassemblerElf() = default;
+template <class TRAITS>
+DisassemblerElf<TRAITS>::~DisassemblerElf() = default;
-template <class Traits>
-ExecutableType DisassemblerElf<Traits>::GetExeType() const {
+template <class TRAITS>
+ExecutableType DisassemblerElf<TRAITS>::GetExeType() const {
return Traits::kExeType;
}
-template <class Traits>
-std::string DisassemblerElf<Traits>::GetExeTypeString() const {
+template <class TRAITS>
+std::string DisassemblerElf<TRAITS>::GetExeTypeString() const {
return Traits::kExeTypeString;
}
// |num_equivalence_iterations_| = 2 for reloc -> abs32.
-template <class Traits>
-DisassemblerElf<Traits>::DisassemblerElf() : Disassembler(2) {}
+template <class TRAITS>
+DisassemblerElf<TRAITS>::DisassemblerElf() : Disassembler(2) {}
-template <class Traits>
-bool DisassemblerElf<Traits>::Parse(ConstBufferView image) {
+template <class TRAITS>
+bool DisassemblerElf<TRAITS>::Parse(ConstBufferView image) {
image_ = image;
if (!ParseHeader())
return false;
@@ -209,8 +209,8 @@
return true;
}
-template <class Traits>
-std::unique_ptr<ReferenceReader> DisassemblerElf<Traits>::MakeReadRelocs(
+template <class TRAITS>
+std::unique_ptr<ReferenceReader> DisassemblerElf<TRAITS>::MakeReadRelocs(
offset_t lo,
offset_t hi) {
DCHECK_LE(lo, hi);
@@ -224,14 +224,14 @@
supported_relocation_type(), lo, hi, translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceWriter> DisassemblerElf<Traits>::MakeWriteRelocs(
+template <class TRAITS>
+std::unique_ptr<ReferenceWriter> DisassemblerElf<TRAITS>::MakeWriteRelocs(
MutableBufferView image) {
return std::make_unique<RelocWriterElf>(image, Traits::kBitness, translator_);
}
-template <class Traits>
-bool DisassemblerElf<Traits>::ParseHeader() {
+template <class TRAITS>
+bool DisassemblerElf<TRAITS>::ParseHeader() {
BufferSource source(image_);
// Ensure any offsets will fit within the |image_|'s bounds.
if (!base::IsValueInRangeForNumericType<offset_t>(image_.size()))
@@ -329,8 +329,8 @@
return true;
}
-template <class Traits>
-void DisassemblerElf<Traits>::ExtractInterestingSectionHeaders() {
+template <class TRAITS>
+void DisassemblerElf<TRAITS>::ExtractInterestingSectionHeaders() {
DCHECK(reloc_section_dims_.empty());
DCHECK(exec_headers_.empty());
for (elf::Elf32_Half i = 0; i < sections_count_; ++i) {
@@ -350,8 +350,8 @@
std::sort(exec_headers_.begin(), exec_headers_.end(), comp);
}
-template <class Traits>
-void DisassemblerElf<Traits>::GetAbs32FromRelocSections() {
+template <class TRAITS>
+void DisassemblerElf<TRAITS>::GetAbs32FromRelocSections() {
constexpr int kAbs32Width = Traits::kVAWidth;
DCHECK(abs32_locations_.empty());
@@ -380,15 +380,15 @@
abs32_locations_.shrink_to_fit();
}
-template <class Traits>
-void DisassemblerElf<Traits>::GetRel32FromCodeSections() {
+template <class TRAITS>
+void DisassemblerElf<TRAITS>::GetRel32FromCodeSections() {
for (const typename Traits::Elf_Shdr* section : exec_headers_)
ParseExecSection(*section);
PostProcessRel32();
}
-template <class Traits>
-void DisassemblerElf<Traits>::ParseSections() {
+template <class TRAITS>
+void DisassemblerElf<TRAITS>::ParseSections() {
ExtractInterestingSectionHeaders();
GetAbs32FromRelocSections();
GetRel32FromCodeSections();
@@ -396,32 +396,34 @@
/******** DisassemblerElfIntel ********/
-template <class Traits>
-DisassemblerElfIntel<Traits>::DisassemblerElfIntel() = default;
+template <class TRAITS>
+DisassemblerElfIntel<TRAITS>::DisassemblerElfIntel() = default;
-template <class Traits>
-DisassemblerElfIntel<Traits>::~DisassemblerElfIntel() = default;
+template <class TRAITS>
+DisassemblerElfIntel<TRAITS>::~DisassemblerElfIntel() = default;
-template <class Traits>
-std::vector<ReferenceGroup> DisassemblerElfIntel<Traits>::MakeReferenceGroups()
+template <class TRAITS>
+std::vector<ReferenceGroup> DisassemblerElfIntel<TRAITS>::MakeReferenceGroups()
const {
return {
- {ReferenceTypeTraits{sizeof(Traits::Elf_Rel::r_offset), TypeTag(kReloc),
+ {ReferenceTypeTraits{sizeof(TRAITS::Elf_Rel::r_offset), TypeTag(kReloc),
PoolTag(kReloc)},
- &DisassemblerElfIntel<Traits>::MakeReadRelocs,
- &DisassemblerElfIntel<Traits>::MakeWriteRelocs},
+ &DisassemblerElfIntel<TRAITS>::MakeReadRelocs,
+ &DisassemblerElfIntel<TRAITS>::MakeWriteRelocs},
{ReferenceTypeTraits{Traits::kVAWidth, TypeTag(kAbs32), PoolTag(kAbs32)},
- &DisassemblerElfIntel<Traits>::MakeReadAbs32,
- &DisassemblerElfIntel<Traits>::MakeWriteAbs32},
+ &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}};
+ &DisassemblerElfIntel<TRAITS>::MakeReadRel32,
+ &DisassemblerElfIntel<TRAITS>::MakeWriteRel32}};
}
-template <class Traits>
-void DisassemblerElfIntel<Traits>::ParseExecSection(
- const typename Traits::Elf_Shdr& section) {
+template <class TRAITS>
+void DisassemblerElfIntel<TRAITS>::ParseExecSection(
+ const typename TRAITS::Elf_Shdr& section) {
+ constexpr int kAbs32Width = Traits::kVAWidth;
+
// |this->| is needed to access protected members of templated base class. To
// reduce noise, use local references for these.
ConstBufferView& image_ = this->image_;
@@ -435,8 +437,8 @@
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 rel_finder(image_, translator_);
+ Abs32GapFinder gap_finder(image_, region, abs32_locations_, kAbs32Width);
+ 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());
@@ -452,43 +454,43 @@
}
}
-template <class Traits>
-void DisassemblerElfIntel<Traits>::PostProcessRel32() {
+template <class TRAITS>
+void DisassemblerElfIntel<TRAITS>::PostProcessRel32() {
rel32_locations_.shrink_to_fit();
std::sort(rel32_locations_.begin(), rel32_locations_.end());
}
-template <class Traits>
-std::unique_ptr<ReferenceReader> DisassemblerElfIntel<Traits>::MakeReadAbs32(
+template <class TRAITS>
+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.
Abs32RvaExtractorWin32 abs_rva_extractor(
- this->image_, AbsoluteAddress(Traits::kBitness, kElfImageBase),
+ this->image_, AbsoluteAddress(TRAITS::kBitness, kElfImageBase),
this->abs32_locations_, lo, hi);
return std::make_unique<Abs32ReaderWin32>(std::move(abs_rva_extractor),
this->translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceWriter> DisassemblerElfIntel<Traits>::MakeWriteAbs32(
+template <class TRAITS>
+std::unique_ptr<ReferenceWriter> DisassemblerElfIntel<TRAITS>::MakeWriteAbs32(
MutableBufferView image) {
return std::make_unique<Abs32WriterWin32>(
- image, AbsoluteAddress(Traits::kBitness, kElfImageBase),
+ image, AbsoluteAddress(TRAITS::kBitness, kElfImageBase),
this->translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceReader> DisassemblerElfIntel<Traits>::MakeReadRel32(
+template <class TRAITS>
+std::unique_ptr<ReferenceReader> DisassemblerElfIntel<TRAITS>::MakeReadRel32(
offset_t lo,
offset_t hi) {
return std::make_unique<Rel32ReaderX86>(this->image_, lo, hi,
&rel32_locations_, this->translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceWriter> DisassemblerElfIntel<Traits>::MakeWriteRel32(
+template <class TRAITS>
+std::unique_ptr<ReferenceWriter> DisassemblerElfIntel<TRAITS>::MakeWriteRel32(
MutableBufferView image) {
return std::make_unique<Rel32WriterX86>(image, this->translator_);
}
diff --git a/disassembler_elf.h b/disassembler_elf.h
index 17e7523..ffd1690 100644
--- a/disassembler_elf.h
+++ b/disassembler_elf.h
@@ -65,9 +65,10 @@
};
// Disassembler for ELF.
-template <class Traits>
+template <class TRAITS>
class DisassemblerElf : public Disassembler {
public:
+ using Traits = TRAITS;
// 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);
@@ -155,9 +156,10 @@
};
// Disassembler for ELF with Intel architectures.
-template <class Traits>
-class DisassemblerElfIntel : public DisassemblerElf<Traits> {
+template <class TRAITS>
+class DisassemblerElfIntel : public DisassemblerElf<TRAITS> {
public:
+ using Traits = TRAITS;
enum ReferenceType : uint8_t { kReloc, kAbs32, kRel32, kTypeCount };
DisassemblerElfIntel();
diff --git a/disassembler_win32.cc b/disassembler_win32.cc
index a23201b..37e43e5 100644
--- a/disassembler_win32.cc
+++ b/disassembler_win32.cc
@@ -24,7 +24,6 @@
// Decides whether |image| points to a Win32 PE file. If this is a possibility,
// assigns |source| to enable further parsing, and returns true. Otherwise
// leaves |source| at an undefined state and returns false.
-template <class Traits>
bool ReadWin32Header(ConstBufferView image, BufferSource* source) {
*source = BufferSource(image);
@@ -47,9 +46,9 @@
return true;
}
-template <class Traits>
+template <class TRAITS>
const pe::ImageDataDirectory* ReadDataDirectory(
- const typename Traits::ImageOptionalHeader* optional_header,
+ const typename TRAITS::ImageOptionalHeader* optional_header,
size_t index) {
if (index >= optional_header->number_of_rva_and_sizes)
return nullptr;
@@ -57,7 +56,7 @@
}
// Decides whether |section| (assumed value) is a section that contains code.
-template <class Traits>
+template <class TRAITS>
bool IsWin32CodeSection(const pe::ImageSectionHeader& section) {
return (section.characteristics & kCodeCharacteristics) ==
kCodeCharacteristics;
@@ -82,31 +81,31 @@
/******** DisassemblerWin32 ********/
// static.
-template <class Traits>
-bool DisassemblerWin32<Traits>::QuickDetect(ConstBufferView image) {
+template <class TRAITS>
+bool DisassemblerWin32<TRAITS>::QuickDetect(ConstBufferView image) {
BufferSource source;
- return ReadWin32Header<Traits>(image, &source);
+ return ReadWin32Header(image, &source);
}
// |num_equivalence_iterations_| = 2 for reloc -> abs32.
-template <class Traits>
-DisassemblerWin32<Traits>::DisassemblerWin32() : Disassembler(2) {}
+template <class TRAITS>
+DisassemblerWin32<TRAITS>::DisassemblerWin32() : Disassembler(2) {}
-template <class Traits>
-DisassemblerWin32<Traits>::~DisassemblerWin32() = default;
+template <class TRAITS>
+DisassemblerWin32<TRAITS>::~DisassemblerWin32() = default;
-template <class Traits>
-ExecutableType DisassemblerWin32<Traits>::GetExeType() const {
+template <class TRAITS>
+ExecutableType DisassemblerWin32<TRAITS>::GetExeType() const {
return Traits::kExeType;
}
-template <class Traits>
-std::string DisassemblerWin32<Traits>::GetExeTypeString() const {
+template <class TRAITS>
+std::string DisassemblerWin32<TRAITS>::GetExeTypeString() const {
return Traits::kExeTypeString;
}
-template <class Traits>
-std::vector<ReferenceGroup> DisassemblerWin32<Traits>::MakeReferenceGroups()
+template <class TRAITS>
+std::vector<ReferenceGroup> DisassemblerWin32<TRAITS>::MakeReferenceGroups()
const {
return {
{ReferenceTypeTraits{2, TypeTag(kReloc), PoolTag(kReloc)},
@@ -118,8 +117,8 @@
};
}
-template <class Traits>
-std::unique_ptr<ReferenceReader> DisassemblerWin32<Traits>::MakeReadRelocs(
+template <class TRAITS>
+std::unique_ptr<ReferenceReader> DisassemblerWin32<TRAITS>::MakeReadRelocs(
offset_t lo,
offset_t hi) {
if (!ParseAndStoreRelocBlocks())
@@ -135,8 +134,8 @@
translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceReader> DisassemblerWin32<Traits>::MakeReadAbs32(
+template <class TRAITS>
+std::unique_ptr<ReferenceReader> DisassemblerWin32<TRAITS>::MakeReadAbs32(
offset_t lo,
offset_t hi) {
ParseAndStoreAbs32();
@@ -146,8 +145,8 @@
translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceReader> DisassemblerWin32<Traits>::MakeReadRel32(
+template <class TRAITS>
+std::unique_ptr<ReferenceReader> DisassemblerWin32<TRAITS>::MakeReadRel32(
offset_t lo,
offset_t hi) {
ParseAndStoreRel32();
@@ -155,8 +154,8 @@
translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceWriter> DisassemblerWin32<Traits>::MakeWriteRelocs(
+template <class TRAITS>
+std::unique_ptr<ReferenceWriter> DisassemblerWin32<TRAITS>::MakeWriteRelocs(
MutableBufferView image) {
if (!ParseAndStoreRelocBlocks())
return std::make_unique<EmptyReferenceWriter>();
@@ -166,30 +165,30 @@
translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceWriter> DisassemblerWin32<Traits>::MakeWriteAbs32(
+template <class TRAITS>
+std::unique_ptr<ReferenceWriter> DisassemblerWin32<TRAITS>::MakeWriteAbs32(
MutableBufferView image) {
return std::make_unique<Abs32WriterWin32>(
image, AbsoluteAddress(Traits::kBitness, image_base_), translator_);
}
-template <class Traits>
-std::unique_ptr<ReferenceWriter> DisassemblerWin32<Traits>::MakeWriteRel32(
+template <class TRAITS>
+std::unique_ptr<ReferenceWriter> DisassemblerWin32<TRAITS>::MakeWriteRel32(
MutableBufferView image) {
return std::make_unique<Rel32WriterX86>(image, translator_);
}
-template <class Traits>
-bool DisassemblerWin32<Traits>::Parse(ConstBufferView image) {
+template <class TRAITS>
+bool DisassemblerWin32<TRAITS>::Parse(ConstBufferView image) {
image_ = image;
return ParseHeader();
}
-template <class Traits>
-bool DisassemblerWin32<Traits>::ParseHeader() {
+template <class TRAITS>
+bool DisassemblerWin32<TRAITS>::ParseHeader() {
BufferSource source;
- if (!ReadWin32Header<Traits>(image_, &source))
+ if (!ReadWin32Header(image_, &source))
return false;
constexpr size_t kDataDirBase =
@@ -297,8 +296,8 @@
return true;
}
-template <class Traits>
-bool DisassemblerWin32<Traits>::ParseAndStoreRelocBlocks() {
+template <class TRAITS>
+bool DisassemblerWin32<TRAITS>::ParseAndStoreRelocBlocks() {
if (has_parsed_relocs_)
return reloc_region_.lo() != kInvalidOffset;
@@ -324,8 +323,8 @@
return true;
}
-template <class Traits>
-bool DisassemblerWin32<Traits>::ParseAndStoreAbs32() {
+template <class TRAITS>
+bool DisassemblerWin32<TRAITS>::ParseAndStoreAbs32() {
if (has_parsed_abs32_)
return true;
has_parsed_abs32_ = true;
@@ -355,8 +354,8 @@
return true;
}
-template <class Traits>
-bool DisassemblerWin32<Traits>::ParseAndStoreRel32() {
+template <class TRAITS>
+bool DisassemblerWin32<TRAITS>::ParseAndStoreRel32() {
if (has_parsed_rel32_)
return true;
has_parsed_rel32_ = true;
diff --git a/disassembler_win32.h b/disassembler_win32.h
index 8f9fd58..d952014 100644
--- a/disassembler_win32.h
+++ b/disassembler_win32.h
@@ -52,9 +52,10 @@
using Address = uint64_t;
};
-template <class Traits>
+template <class TRAITS>
class DisassemblerWin32 : public Disassembler {
public:
+ using Traits = TRAITS;
enum ReferenceType : uint8_t { kReloc, kAbs32, kRel32, kTypeCount };
// Applies quick checks to determine whether |image| *may* point to the start
diff --git a/reloc_elf_unittest.cc b/reloc_elf_unittest.cc
index b03f403..8a1b932 100644
--- a/reloc_elf_unittest.cc
+++ b/reloc_elf_unittest.cc
@@ -48,9 +48,10 @@
}
// Helper to manipulate an image with one or more relocation tables.
-template <class ElfIntelTraits>
+template <class ELF_INTEL_TRAITS>
class FakeImageWithReloc {
public:
+ using ElfIntelTraits = ELF_INTEL_TRAITS;
struct RelocSpec {
offset_t start;
std::vector<uint8_t> data;
