[Courgette] Using LabelManager to reduce Courgette-apply peak RAM by 25%.
AssemblyProgram previously allocates new Label instances as it parses
an executable and emits instructions. This CL replaces the flow by using
LabelManager to precompute Labels in one array. This allows us to reduce
Courgette-apply peak RAM by 25%, measured by "choke RAM until failure"
method. Details:
- We precompute Labels in AssemblyProgram::PrecomputeLabels(), which
relies on RvaVisitor inherited classes for architecture-specific
extraction of abs32 and rel32 targets.
- TrimLabel()'s complex post-processing flow is simplified using
PrecomputeLabels(), which runs before main file parse.
- This requires RemoveUnusedRel32Locations() to update rel32.
- Deprecating C_TRIM_FAILED error message.
- Moving more common functionality to Disassembler, but duplicating
some code for win32-x86 and win32-x64 to follow existing pattern.
BUG=613216
Review-Url: https://codereview.chromium.org/1935203002
Cr-Commit-Position: refs/heads/master@{#394815}
diff --git a/courgette/adjustment_method_unittest.cc b/courgette/adjustment_method_unittest.cc
index 3200f10c..d8b987c 100644
--- a/courgette/adjustment_method_unittest.cc
+++ b/courgette/adjustment_method_unittest.cc
@@ -5,11 +5,13 @@
#include <memory>
#include <string>
#include <utility>
+#include <vector>
#include "base/strings/string_util.h"
#include "courgette/assembly_program.h"
#include "courgette/courgette.h"
#include "courgette/encoded_program.h"
+#include "courgette/image_utils.h"
#include "courgette/streams.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -32,8 +34,20 @@
new courgette::AssemblyProgram(courgette::EXE_WIN_32_X86));
prog->set_image_base(0x00400000);
- courgette::Label* labelA = prog->FindOrMakeAbs32Label(0x00410000);
- courgette::Label* labelB = prog->FindOrMakeAbs32Label(0x00410004);
+ courgette::RVA kRvaA = 0x00410000;
+ courgette::RVA kRvaB = 0x00410004;
+
+ std::vector<courgette::RVA> abs32_rvas;
+ abs32_rvas.push_back(kRvaA);
+ abs32_rvas.push_back(kRvaB);
+ std::vector<courgette::RVA> rel32_rvas; // Stub.
+
+ courgette::TrivialRvaVisitor abs32_visitor(abs32_rvas);
+ courgette::TrivialRvaVisitor rel32_visitor(rel32_rvas);
+ prog->PrecomputeLabels(&abs32_visitor, &rel32_visitor);
+
+ courgette::Label* labelA = prog->FindAbs32Label(kRvaA);
+ courgette::Label* labelB = prog->FindAbs32Label(kRvaB);
EXPECT_TRUE(prog->EmitAbs32(labelA));
EXPECT_TRUE(prog->EmitAbs32(labelA));
@@ -88,7 +102,6 @@
}
};
-
void AdjustmentMethodTest::Test1() const {
std::unique_ptr<courgette::AssemblyProgram> prog1 = MakeProgramA();
std::unique_ptr<courgette::AssemblyProgram> prog2 = MakeProgramB();
@@ -109,7 +122,6 @@
EXPECT_TRUE(s5 == s6); // Adjustment did change B into A
}
-
TEST_F(AdjustmentMethodTest, All) {
Test1();
}
diff --git a/courgette/assembly_program.cc b/courgette/assembly_program.cc
index 5344cff9..f20ed67e 100644
--- a/courgette/assembly_program.cc
+++ b/courgette/assembly_program.cc
@@ -111,11 +111,6 @@
: kind_(kind), image_base_(0) {
}
-static void DeleteContainedLabels(const RVAToLabel& labels) {
- for (RVAToLabel::const_iterator p = labels.begin(); p != labels.end(); ++p)
- UncheckedDelete(p->second);
-}
-
AssemblyProgram::~AssemblyProgram() {
for (size_t i = 0; i < instructions_.size(); ++i) {
Instruction* instruction = instructions_[i];
@@ -126,8 +121,6 @@
for (size_t i = 0; i < 256; ++i)
UncheckedDelete(byte_instruction_cache_[i]);
}
- DeleteContainedLabels(rel32_labels_);
- DeleteContainedLabels(abs32_labels_);
}
CheckBool AssemblyProgram::EmitPeRelocsInstruction() {
@@ -178,27 +171,50 @@
ScopedInstruction(UncheckedNew<InstructionWithLabel>(ABS64, label)));
}
-Label* AssemblyProgram::FindOrMakeAbs32Label(RVA rva) {
- return FindLabel(rva, &abs32_labels_);
+void AssemblyProgram::PrecomputeLabels(RvaVisitor* abs32_visitor,
+ RvaVisitor* rel32_visitor) {
+ abs32_label_manager_.Read(abs32_visitor);
+ rel32_label_manager_.Read(rel32_visitor);
+ TrimLabels();
}
-Label* AssemblyProgram::FindOrMakeRel32Label(RVA rva) {
- return FindLabel(rva, &rel32_labels_);
-}
+// Chosen empirically to give the best reduction in payload size for
+// an update from daisy_3701.98.0 to daisy_4206.0.0.
+const int AssemblyProgram::kLabelLowerLimit = 5;
-void AssemblyProgram::DefaultAssignIndexes() {
- DefaultAssignIndexes(&abs32_labels_);
- DefaultAssignIndexes(&rel32_labels_);
+void AssemblyProgram::TrimLabels() {
+ // For now only trim for ARM binaries.
+ if (kind() != EXE_ELF_32_ARM)
+ return;
+
+ int lower_limit = kLabelLowerLimit;
+
+ VLOG(1) << "TrimLabels: threshold " << lower_limit;
+
+ rel32_label_manager_.RemoveUnderusedLabels(lower_limit);
}
void AssemblyProgram::UnassignIndexes() {
- UnassignIndexes(&abs32_labels_);
- UnassignIndexes(&rel32_labels_);
+ abs32_label_manager_.UnassignIndexes();
+ rel32_label_manager_.UnassignIndexes();
+}
+
+void AssemblyProgram::DefaultAssignIndexes() {
+ abs32_label_manager_.DefaultAssignIndexes();
+ rel32_label_manager_.DefaultAssignIndexes();
}
void AssemblyProgram::AssignRemainingIndexes() {
- AssignRemainingIndexes(&abs32_labels_);
- AssignRemainingIndexes(&rel32_labels_);
+ abs32_label_manager_.AssignRemainingIndexes();
+ rel32_label_manager_.AssignRemainingIndexes();
+}
+
+Label* AssemblyProgram::FindAbs32Label(RVA rva) {
+ return abs32_label_manager_.Find(rva);
+}
+
+Label* AssemblyProgram::FindRel32Label(RVA rva) {
+ return rel32_label_manager_.Find(rva);
}
Label* AssemblyProgram::InstructionAbs32Label(
@@ -238,17 +254,6 @@
return instruction && instructions_.push_back(instruction);
}
-Label* AssemblyProgram::FindLabel(RVA rva, RVAToLabel* labels) {
- Label*& slot = (*labels)[rva];
- if (slot == NULL) {
- slot = UncheckedNew<Label>(rva);
- if (slot == NULL)
- return NULL;
- }
- slot->count_++;
- return slot;
-}
-
void AssemblyProgram::UnassignIndexes(RVAToLabel* labels) {
for (RVAToLabel::iterator p = labels->begin(); p != labels->end(); ++p) {
Label* current = p->second;
@@ -367,7 +372,7 @@
encoded->set_image_base(image_base_);
- if (!encoded->DefineLabels(abs32_labels_, rel32_labels_))
+ if (!encoded->ImportLabels(abs32_label_manager_, rel32_label_manager_))
return nullptr;
for (size_t i = 0; i < instructions_.size(); ++i) {
@@ -470,61 +475,6 @@
return byte_instruction_cache_[byte];
}
-// Chosen empirically to give the best reduction in payload size for
-// an update from daisy_3701.98.0 to daisy_4206.0.0.
-const int AssemblyProgram::kLabelLowerLimit = 5;
-
-CheckBool AssemblyProgram::TrimLabels() {
- // For now only trim for ARM binaries.
- if (kind() != EXE_ELF_32_ARM)
- return true;
-
- int lower_limit = kLabelLowerLimit;
-
- VLOG(1) << "TrimLabels: threshold " << lower_limit;
-
- // Walk through the list of instructions, replacing trimmed labels
- // with the original machine instruction.
- for (size_t i = 0; i < instructions_.size(); ++i) {
- Instruction* instruction = instructions_[i];
- switch (instruction->op()) {
- case REL32ARM: {
- Label* label =
- static_cast<InstructionWithLabelARM*>(instruction)->label();
- if (label->count_ <= lower_limit) {
- const uint8_t* arm_op =
- static_cast<InstructionWithLabelARM*>(instruction)->arm_op();
- uint16_t op_size =
- static_cast<InstructionWithLabelARM*>(instruction)->op_size();
-
- if (op_size < 1)
- return false;
- UncheckedDelete(instruction);
- instructions_[i] = UncheckedNew<BytesInstruction>(arm_op, op_size);
- if (!instructions_[i])
- return false;
- }
- break;
- }
- default:
- break;
- }
- }
-
- // Remove and deallocate underused Labels.
- RVAToLabel::iterator it = rel32_labels_.begin();
- while (it != rel32_labels_.end()) {
- if (it->second->count_ <= lower_limit) {
- UncheckedDelete(it->second);
- rel32_labels_.erase(it++);
- } else {
- ++it;
- }
- }
-
- return true;
-}
-
////////////////////////////////////////////////////////////////////////////////
Status Encode(const AssemblyProgram& program,
diff --git a/courgette/assembly_program.h b/courgette/assembly_program.h
index 2d157dd..96d6daf 100644
--- a/courgette/assembly_program.h
+++ b/courgette/assembly_program.h
@@ -123,16 +123,23 @@
// Generates 8-byte absolute reference to address of 'label'.
CheckBool EmitAbs64(Label* label) WARN_UNUSED_RESULT;
- // Looks up a label or creates a new one. Might return NULL.
- Label* FindOrMakeAbs32Label(RVA rva);
+ // Traverses RVAs in |abs32_visitor| and |rel32_visitor| to precompute Labels.
+ void PrecomputeLabels(RvaVisitor* abs32_visitor, RvaVisitor* rel32_visitor);
- // Looks up a label or creates a new one. Might return NULL.
- Label* FindOrMakeRel32Label(RVA rva);
+ // Removes underused Labels. Thresholds used (0 = no trimming) is
+ // architecture-dependent.
+ void TrimLabels();
- void DefaultAssignIndexes();
void UnassignIndexes();
+ void DefaultAssignIndexes();
void AssignRemainingIndexes();
+ // Looks up abs32 label. Returns null if none found.
+ Label* FindAbs32Label(RVA rva);
+
+ // Looks up rel32 label. Returns null if none found.
+ Label* FindRel32Label(RVA rva);
+
std::unique_ptr<EncodedProgram> Encode() const;
// Accessor for instruction list.
@@ -152,10 +159,6 @@
// otherwise returns NULL.
Label* InstructionRel32Label(const Instruction* instruction) const;
- // Removes underused Labels. Thresholds used (may be 0, i.e., no trimming) is
- // dependent on architecture. Returns true on success, and false otherwise.
- CheckBool TrimLabels();
-
private:
using ScopedInstruction =
std::unique_ptr<Instruction, UncheckedDeleter<Instruction>>;
@@ -183,11 +186,10 @@
InstructionVector instructions_; // All the instructions in program.
- // These are lookup maps to find the label associated with a given address.
- // We have separate label spaces for addresses referenced by rel32 labels and
- // abs32 labels. This is somewhat arbitrary.
- RVAToLabel rel32_labels_;
- RVAToLabel abs32_labels_;
+ // Storage and lookup of Labels associated with target addresses. We use
+ // separate abs32 and rel32 labels.
+ LabelManager abs32_label_manager_;
+ LabelManager rel32_label_manager_;
DISALLOW_COPY_AND_ASSIGN(AssemblyProgram);
};
diff --git a/courgette/courgette.h b/courgette/courgette.h
index 84ab7cd..5f3dbb8 100644
--- a/courgette/courgette.h
+++ b/courgette/courgette.h
@@ -48,7 +48,6 @@
C_DISASSEMBLY_FAILED = 25, //
C_ASSEMBLY_FAILED = 26, //
C_ADJUSTMENT_FAILED = 27, //
- C_TRIM_FAILED = 28, // TrimLabels failed
};
// What type of executable is something
diff --git a/courgette/disassembler.cc b/courgette/disassembler.cc
index 9b58ba0..baa3b1f 100644
--- a/courgette/disassembler.cc
+++ b/courgette/disassembler.cc
@@ -4,10 +4,35 @@
#include "courgette/disassembler.h"
+#include <memory>
+
#include "base/logging.h"
+#include "courgette/assembly_program.h"
namespace courgette {
+Disassembler::RvaVisitor_Abs32::RvaVisitor_Abs32(
+ const std::vector<RVA>& rva_locations,
+ const AddressTranslator& translator)
+ : VectorRvaVisitor<RVA>(rva_locations), translator_(translator) {
+}
+
+RVA Disassembler::RvaVisitor_Abs32::Get() const {
+ // For Abs32 targets, get target RVA from architecture-dependent functions.
+ return translator_.PointerToTargetRVA(translator_.RVAToPointer(*it_));
+}
+
+Disassembler::RvaVisitor_Rel32::RvaVisitor_Rel32(
+ const std::vector<RVA>& rva_locations,
+ const AddressTranslator& translator)
+ : VectorRvaVisitor<RVA>(rva_locations), translator_(translator) {
+}
+
+RVA Disassembler::RvaVisitor_Rel32::Get() const {
+ // For Rel32 targets, only handle 32-bit offsets.
+ return *it_ + 4 + Read32LittleEndian(translator_.RVAToPointer(*it_));
+}
+
Disassembler::Disassembler(const void* start, size_t length)
: failure_reason_("uninitialized") {
start_ = reinterpret_cast<const uint8_t*>(start);
@@ -40,6 +65,12 @@
return false;
}
+void Disassembler::PrecomputeLabels(AssemblyProgram* program) {
+ std::unique_ptr<RvaVisitor> abs32_visitor(CreateAbs32TargetRvaVisitor());
+ std::unique_ptr<RvaVisitor> rel32_visitor(CreateRel32TargetRvaVisitor());
+ program->PrecomputeLabels(abs32_visitor.get(), rel32_visitor.get());
+}
+
void Disassembler::ReduceLength(size_t reduced_length) {
CHECK_LE(reduced_length, length_);
length_ = reduced_length;
diff --git a/courgette/disassembler.h b/courgette/disassembler.h
index 7c57099..d84b114 100644
--- a/courgette/disassembler.h
+++ b/courgette/disassembler.h
@@ -8,6 +8,8 @@
#include <stddef.h>
#include <stdint.h>
+#include <vector>
+
#include "base/macros.h"
#include "courgette/courgette.h"
#include "courgette/image_utils.h"
@@ -18,6 +20,38 @@
class Disassembler : public AddressTranslator {
public:
+ // Visitor/adaptor to translate RVA to target RVA for abs32.
+ class RvaVisitor_Abs32 : public VectorRvaVisitor<RVA> {
+ public:
+ RvaVisitor_Abs32(const std::vector<RVA>& rva_locations,
+ const AddressTranslator& translator);
+ ~RvaVisitor_Abs32() override { }
+
+ // VectorRvaVisitor<RVA> interfaces.
+ RVA Get() const override;
+
+ private:
+ const AddressTranslator& translator_;
+
+ DISALLOW_COPY_AND_ASSIGN(RvaVisitor_Abs32);
+ };
+
+ // Visitor/adaptor to translate RVA to target RVA for rel32.
+ class RvaVisitor_Rel32 : public VectorRvaVisitor<RVA> {
+ public:
+ RvaVisitor_Rel32(const std::vector<RVA>& rva_locations,
+ const AddressTranslator& translator);
+ ~RvaVisitor_Rel32() override { }
+
+ // VectorRvaVisitor<RVA> interfaces.
+ RVA Get() const override;
+
+ private:
+ const AddressTranslator& translator_;
+
+ DISALLOW_COPY_AND_ASSIGN(RvaVisitor_Rel32);
+ };
+
virtual ~Disassembler();
// AddressTranslator interfaces.
@@ -29,6 +63,17 @@
virtual ExecutableType kind() const = 0;
+ // Returns a caller-owned new RvaVisitor to iterate through abs32 target RVAs.
+ virtual RvaVisitor* CreateAbs32TargetRvaVisitor() = 0;
+
+ // Returns a caller-owned new RvaVisitor to iterate through rel32 target RVAs.
+ virtual RvaVisitor* CreateRel32TargetRvaVisitor() = 0;
+
+ // Removes unused rel32 locations (architecture-specific). This is needed
+ // because we may remove rel32 Labels along the way. As a result the matching
+ // matching rel32 addresses become unused. Removing them saves space.
+ virtual void RemoveUnusedRel32Locations(AssemblyProgram* program) = 0;
+
// Returns true if the buffer appears to be a valid executable of the expected
// type, and false otherwise. This needs not be called before Disassemble().
virtual bool ParseHeader() = 0;
@@ -57,6 +102,9 @@
return offset <= length() && elements <= (length() - offset) / element_size;
}
+ // Computes and stores all Labels before scanning program bytes.
+ void PrecomputeLabels(AssemblyProgram* program);
+
// Reduce the length of the image in memory. Does not actually free
// (or realloc) any memory. Usually only called via ParseHeader().
void ReduceLength(size_t reduced_length);
diff --git a/courgette/disassembler_elf_32.cc b/courgette/disassembler_elf_32.cc
index d9d5043..febcdea 100644
--- a/courgette/disassembler_elf_32.cc
+++ b/courgette/disassembler_elf_32.cc
@@ -5,6 +5,7 @@
#include "courgette/disassembler_elf_32.h"
#include <algorithm>
+#include <iterator>
#include <utility>
#include "base/logging.h"
@@ -33,6 +34,15 @@
} // namespace
+DisassemblerElf32::Elf32RvaVisitor_Rel32::Elf32RvaVisitor_Rel32(
+ const std::vector<std::unique_ptr<TypedRVA>>& rva_locations)
+ : VectorRvaVisitor<std::unique_ptr<TypedRVA>>(rva_locations) {
+}
+
+RVA DisassemblerElf32::Elf32RvaVisitor_Rel32::Get() const {
+ return (*it_)->rva() + (*it_)->relative_target();
+}
+
DisassemblerElf32::DisassemblerElf32(const void* start, size_t length)
: Disassembler(start, length),
header_(nullptr),
@@ -164,12 +174,16 @@
if (!ParseAbs32Relocs())
return false;
- if (!ParseRel32RelocsFromSections())
+ if (!ParseRel32RelocsFromSections()) // Does not sort rel32 locations.
return false;
+ PrecomputeLabels(target);
+ RemoveUnusedRel32Locations(target);
+
if (!ParseFile(target))
return false;
+ // Finally sort rel32 locations.
std::sort(rel32_locations_.begin(),
rel32_locations_.end(),
TypedRVA::IsLessThanByRVA);
@@ -180,6 +194,28 @@
return true;
}
+CheckBool DisassemblerElf32::IsValidTargetRVA(RVA rva) const {
+ if (rva == kUnassignedRVA)
+ return false;
+
+ // |rva| is valid if it's contained in any program segment.
+ for (Elf32_Half segment_id = 0; segment_id < ProgramSegmentHeaderCount();
+ ++segment_id) {
+ const Elf32_Phdr* segment_header = ProgramSegmentHeader(segment_id);
+
+ if (segment_header->p_type != PT_LOAD)
+ continue;
+
+ Elf32_Addr begin = segment_header->p_vaddr;
+ Elf32_Addr end = segment_header->p_vaddr + segment_header->p_memsz;
+
+ if (rva >= begin && rva < end)
+ return true;
+ }
+
+ return false;
+}
+
bool DisassemblerElf32::UpdateLength() {
Elf32_Off result = 0;
@@ -238,32 +274,11 @@
return true;
}
-CheckBool DisassemblerElf32::IsValidTargetRVA(RVA rva) const {
- if (rva == kUnassignedRVA)
- return false;
-
- // It's valid if it's contained in any program segment
- for (Elf32_Half segment_id = 0; segment_id < ProgramSegmentHeaderCount();
- ++segment_id) {
- const Elf32_Phdr* segment_header = ProgramSegmentHeader(segment_id);
-
- if (segment_header->p_type != PT_LOAD)
- continue;
-
- Elf32_Addr begin = segment_header->p_vaddr;
- Elf32_Addr end = segment_header->p_vaddr + segment_header->p_memsz;
-
- if (rva >= begin && rva < end)
- return true;
- }
-
- return false;
-}
-
CheckBool DisassemblerElf32::RVAsToFileOffsets(
const std::vector<RVA>& rvas,
std::vector<FileOffset>* file_offsets) {
file_offsets->clear();
+ file_offsets->reserve(rvas.size());
for (RVA rva : rvas) {
FileOffset file_offset = RVAToFileOffset(rva);
if (file_offset == kNoFileOffset)
@@ -284,6 +299,32 @@
return true;
}
+RvaVisitor* DisassemblerElf32::CreateAbs32TargetRvaVisitor() {
+ return new RvaVisitor_Abs32(abs32_locations_, *this);
+}
+
+RvaVisitor* DisassemblerElf32::CreateRel32TargetRvaVisitor() {
+ return new Elf32RvaVisitor_Rel32(rel32_locations_);
+}
+
+void DisassemblerElf32::RemoveUnusedRel32Locations(AssemblyProgram* program) {
+ auto tail_it = rel32_locations_.begin();
+ for (auto head_it = rel32_locations_.begin();
+ head_it != rel32_locations_.end(); ++head_it) {
+ RVA target_rva = (*head_it)->rva() + (*head_it)->relative_target();
+ if (program->FindRel32Label(target_rva) == nullptr) {
+ // If address does not match a Label (because it was removed), deallocate.
+ (*head_it).reset(nullptr);
+ } else {
+ // Else squeeze nullptr to end to compactify.
+ if (tail_it != head_it)
+ (*tail_it).swap(*head_it);
+ ++tail_it;
+ }
+ }
+ rel32_locations_.resize(std::distance(rel32_locations_.begin(), tail_it));
+}
+
CheckBool DisassemblerElf32::ParseFile(AssemblyProgram* program) {
// Walk all the bytes in the file, whether or not in a section.
FileOffset file_offset = 0;
@@ -425,7 +466,9 @@
RVA target_rva = PointerToTargetRVA(FileOffsetToPointer(file_offset));
DCHECK_NE(kNoRVA, target_rva);
- if (!program->EmitAbs32(program->FindOrMakeAbs32Label(target_rva)))
+ Label* label = program->FindAbs32Label(target_rva);
+ CHECK(label);
+ if (!program->EmitAbs32(label))
return false;
file_offset += sizeof(RVA);
++(*current_abs_offset);
@@ -435,12 +478,17 @@
if (*current_rel != end_rel &&
file_offset == (**current_rel)->file_offset()) {
uint32_t relative_target = (**current_rel)->relative_target();
+ CHECK_EQ(RVA(origin + (file_offset - origin_offset)),
+ (**current_rel)->rva());
// This cast is for 64 bit systems, and is only safe because we
// are working on 32 bit executables.
RVA target_rva = (RVA)(origin + (file_offset - origin_offset) +
relative_target);
- if (!(**current_rel)->EmitInstruction(program, target_rva))
+ Label* label = program->FindRel32Label(target_rva);
+ CHECK(label);
+
+ if (!(**current_rel)->EmitInstruction(program, label))
return false;
file_offset += (**current_rel)->op_size();
++(*current_rel);
diff --git a/courgette/disassembler_elf_32.h b/courgette/disassembler_elf_32.h
index cf3e900..6bcf1b8 100644
--- a/courgette/disassembler_elf_32.h
+++ b/courgette/disassembler_elf_32.h
@@ -9,6 +9,7 @@
#include <stdint.h>
#include <memory>
+#include <string>
#include <vector>
#include "base/macros.h"
@@ -51,9 +52,9 @@
// Computes the relative jump's offset from the op in p.
virtual CheckBool ComputeRelativeTarget(const uint8_t* op_pointer) = 0;
- // Emits the courgette instruction corresponding to the RVA type.
+ // Emits the assembly instruction corresponding to |label|.
virtual CheckBool EmitInstruction(AssemblyProgram* program,
- RVA target_rva) = 0;
+ Label* label) = 0;
// Returns the size of the instruction containing the RVA.
virtual uint16_t op_size() const = 0;
@@ -76,6 +77,22 @@
FileOffset file_offset_ = kNoFileOffset;
};
+ // Visitor/adaptor to translate RVA to target RVA. This is the ELF
+ // counterpart to RvaVisitor_Rel32 that uses TypedRVA.
+ class Elf32RvaVisitor_Rel32 :
+ public VectorRvaVisitor<std::unique_ptr<TypedRVA>> {
+ public:
+ Elf32RvaVisitor_Rel32(
+ const std::vector<std::unique_ptr<TypedRVA>>& rva_locations);
+ ~Elf32RvaVisitor_Rel32() override { }
+
+ // VectorRvaVisitor<TypedRVA*> interfaces.
+ RVA Get() const override;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(Elf32RvaVisitor_Rel32);
+ };
+
public:
DisassemblerElf32(const void* start, size_t length);
@@ -91,6 +108,12 @@
virtual e_machine_values ElfEM() const = 0;
+ CheckBool IsValidTargetRVA(RVA rva) const WARN_UNUSED_RESULT;
+
+ // Converts an ELF relocation instruction into an RVA.
+ virtual CheckBool RelToRVA(Elf32_Rel rel, RVA* result)
+ const WARN_UNUSED_RESULT = 0;
+
// Public for unittests only
std::vector<RVA>& Abs32Locations() { return abs32_locations_; }
std::vector<std::unique_ptr<TypedRVA>>& Rel32Locations() {
@@ -132,12 +155,6 @@
// Misc address space helpers
- CheckBool IsValidTargetRVA(RVA rva) const WARN_UNUSED_RESULT;
-
- // Converts an ELF relocation instruction into an RVA.
- virtual CheckBool RelToRVA(Elf32_Rel rel, RVA* result)
- const WARN_UNUSED_RESULT = 0;
-
CheckBool RVAsToFileOffsets(const std::vector<RVA>& rvas,
std::vector<FileOffset>* file_offsets);
@@ -153,6 +170,11 @@
virtual CheckBool ParseRel32RelocsFromSection(const Elf32_Shdr* section)
WARN_UNUSED_RESULT = 0;
+ // Disassembler interfaces.
+ RvaVisitor* CreateAbs32TargetRvaVisitor() override;
+ RvaVisitor* CreateRel32TargetRvaVisitor() override;
+ void RemoveUnusedRel32Locations(AssemblyProgram* program) override;
+
CheckBool ParseFile(AssemblyProgram* target) WARN_UNUSED_RESULT;
CheckBool ParseProgbitsSection(
diff --git a/courgette/disassembler_elf_32_arm.cc b/courgette/disassembler_elf_32_arm.cc
index f1fbc7a..3ea1008c 100644
--- a/courgette/disassembler_elf_32_arm.cc
+++ b/courgette/disassembler_elf_32_arm.cc
@@ -300,11 +300,8 @@
CheckBool DisassemblerElf32ARM::TypedRVAARM::EmitInstruction(
AssemblyProgram* program,
- RVA target_rva) {
- return program->EmitRel32ARM(c_op(),
- program->FindOrMakeRel32Label(target_rva),
- arm_op_,
- op_size());
+ Label* label) {
+ return program->EmitRel32ARM(c_op(), label, arm_op_, op_size());
}
DisassemblerElf32ARM::DisassemblerElf32ARM(const void* start, size_t length)
diff --git a/courgette/disassembler_elf_32_arm.h b/courgette/disassembler_elf_32_arm.h
index 5dc6897..83f5dc6 100644
--- a/courgette/disassembler_elf_32_arm.h
+++ b/courgette/disassembler_elf_32_arm.h
@@ -36,7 +36,7 @@
// TypedRVA interfaces.
CheckBool ComputeRelativeTarget(const uint8_t* op_pointer) override;
CheckBool EmitInstruction(AssemblyProgram* program,
- RVA target_rva) override;
+ Label* label) override;
uint16_t op_size() const override;
uint16_t c_op() const { return c_op_; }
diff --git a/courgette/disassembler_elf_32_x86.cc b/courgette/disassembler_elf_32_x86.cc
index 67aa5c3..ee6e732 100644
--- a/courgette/disassembler_elf_32_x86.cc
+++ b/courgette/disassembler_elf_32_x86.cc
@@ -22,8 +22,8 @@
CheckBool DisassemblerElf32X86::TypedRVAX86::EmitInstruction(
AssemblyProgram* program,
- RVA target_rva) {
- return program->EmitRel32(program->FindOrMakeRel32Label(target_rva));
+ Label* label) {
+ return program->EmitRel32(label);
}
uint16_t DisassemblerElf32X86::TypedRVAX86::op_size() const {
diff --git a/courgette/disassembler_elf_32_x86.h b/courgette/disassembler_elf_32_x86.h
index 63be755..706e6fe 100644
--- a/courgette/disassembler_elf_32_x86.h
+++ b/courgette/disassembler_elf_32_x86.h
@@ -28,7 +28,7 @@
// TypedRVA interfaces.
CheckBool ComputeRelativeTarget(const uint8_t* op_pointer) override;
CheckBool EmitInstruction(AssemblyProgram* program,
- RVA target_rva) override;
+ Label* label) override;
uint16_t op_size() const override;
};
diff --git a/courgette/disassembler_win32_x64.cc b/courgette/disassembler_win32_x64.cc
index ffa6c36..2db35a3 100644
--- a/courgette/disassembler_win32_x64.cc
+++ b/courgette/disassembler_win32_x64.cc
@@ -252,6 +252,9 @@
ParseRel32RelocsFromSections();
+ PrecomputeLabels(target);
+ RemoveUnusedRel32Locations(target);
+
if (!ParseFile(target))
return false;
@@ -357,6 +360,26 @@
return name;
}
+RvaVisitor* DisassemblerWin32X64::CreateAbs32TargetRvaVisitor() {
+ return new RvaVisitor_Abs32(abs32_locations_, *this);
+}
+
+RvaVisitor* DisassemblerWin32X64::CreateRel32TargetRvaVisitor() {
+ return new RvaVisitor_Rel32(rel32_locations_, *this);
+}
+
+void DisassemblerWin32X64::RemoveUnusedRel32Locations(
+ AssemblyProgram* program) {
+ auto cond = [this, program](RVA rva) -> bool {
+ // + 4 since offset is relative to start of next instruction.
+ RVA target_rva = rva + 4 + Read32LittleEndian(RVAToPointer(rva));
+ return program->FindRel32Label(target_rva) == nullptr;
+ };
+ rel32_locations_.erase(
+ std::remove_if(rel32_locations_.begin(), rel32_locations_.end(), cond),
+ rel32_locations_.end());
+}
+
CheckBool DisassemblerWin32X64::ParseFile(AssemblyProgram* program) {
// Walk all the bytes in the file, whether or not in a section.
FileOffset file_offset = 0;
@@ -525,12 +548,13 @@
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.
+ // 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.
@@ -614,7 +638,9 @@
DCHECK_NE(kNoRVA, target_rva);
// TODO(sra): target could be Label+offset. It is not clear how to guess
// which it might be. We assume offset==0.
- if (!program->EmitAbs64(program->FindOrMakeAbs32Label(target_rva)))
+ Label* label = program->FindAbs32Label(target_rva);
+ DCHECK(label);
+ if (!program->EmitAbs64(label))
return false;
p += 8;
continue;
@@ -624,8 +650,11 @@
++rel32_pos;
if (rel32_pos != rel32_locations_.end() && *rel32_pos == current_rva) {
+ // + 4 since offset is relative to start of next instruction.
RVA target_rva = current_rva + 4 + Read32LittleEndian(p);
- if (!program->EmitRel32(program->FindOrMakeRel32Label(target_rva)))
+ Label* label = program->FindRel32Label(target_rva);
+ DCHECK(label);
+ if (!program->EmitRel32(label))
return false;
p += 4;
continue;
diff --git a/courgette/disassembler_win32_x64.h b/courgette/disassembler_win32_x64.h
index 60a2259f..49ca452 100644
--- a/courgette/disassembler_win32_x64.h
+++ b/courgette/disassembler_win32_x64.h
@@ -54,6 +54,11 @@
static std::string SectionName(const Section* section);
protected:
+ // Disassembler interfaces.
+ RvaVisitor* CreateAbs32TargetRvaVisitor() override;
+ RvaVisitor* CreateRel32TargetRvaVisitor() override;
+ void RemoveUnusedRel32Locations(AssemblyProgram* program) override;
+
CheckBool ParseFile(AssemblyProgram* target) WARN_UNUSED_RESULT;
bool ParseAbs32Relocs();
void ParseRel32RelocsFromSections();
diff --git a/courgette/disassembler_win32_x86.cc b/courgette/disassembler_win32_x86.cc
index 974e864..fc3f921 100644
--- a/courgette/disassembler_win32_x86.cc
+++ b/courgette/disassembler_win32_x86.cc
@@ -252,6 +252,9 @@
ParseRel32RelocsFromSections();
+ PrecomputeLabels(target);
+ RemoveUnusedRel32Locations(target);
+
if (!ParseFile(target))
return false;
@@ -362,6 +365,26 @@
return name;
}
+RvaVisitor* DisassemblerWin32X86::CreateAbs32TargetRvaVisitor() {
+ return new RvaVisitor_Abs32(abs32_locations_, *this);
+}
+
+RvaVisitor* DisassemblerWin32X86::CreateRel32TargetRvaVisitor() {
+ return new RvaVisitor_Rel32(rel32_locations_, *this);
+}
+
+void DisassemblerWin32X86::RemoveUnusedRel32Locations(
+ AssemblyProgram* program) {
+ auto cond = [this, program](RVA rva) -> bool {
+ // + 4 since offset is relative to start of next instruction.
+ RVA target_rva = rva + 4 + Read32LittleEndian(RVAToPointer(rva));
+ return program->FindRel32Label(target_rva) == nullptr;
+ };
+ rel32_locations_.erase(
+ std::remove_if(rel32_locations_.begin(), rel32_locations_.end(), cond),
+ rel32_locations_.end());
+}
+
CheckBool DisassemblerWin32X86::ParseFile(AssemblyProgram* program) {
// Walk all the bytes in the file, whether or not in a section.
FileOffset file_offset = 0;
@@ -543,7 +566,9 @@
DCHECK_NE(kNoRVA, target_rva);
// TODO(sra): target could be Label+offset. It is not clear how to guess
// which it might be. We assume offset==0.
- if (!program->EmitAbs32(program->FindOrMakeAbs32Label(target_rva)))
+ Label* label = program->FindAbs32Label(target_rva);
+ DCHECK(label);
+ if (!program->EmitAbs32(label))
return false;
p += 4;
continue;
@@ -553,8 +578,11 @@
++rel32_pos;
if (rel32_pos != rel32_locations_.end() && *rel32_pos == current_rva) {
+ // + 4 since offset is relative to start of next instruction.
RVA target_rva = current_rva + 4 + Read32LittleEndian(p);
- if (!program->EmitRel32(program->FindOrMakeRel32Label(target_rva)))
+ Label* label = program->FindRel32Label(target_rva);
+ DCHECK(label);
+ if (!program->EmitRel32(label))
return false;
p += 4;
continue;
diff --git a/courgette/disassembler_win32_x86.h b/courgette/disassembler_win32_x86.h
index 597f841ff..a01f040 100644
--- a/courgette/disassembler_win32_x86.h
+++ b/courgette/disassembler_win32_x86.h
@@ -54,6 +54,11 @@
static std::string SectionName(const Section* section);
protected:
+ // Disassembler interfaces.
+ RvaVisitor* CreateAbs32TargetRvaVisitor() override;
+ RvaVisitor* CreateRel32TargetRvaVisitor() override;
+ void RemoveUnusedRel32Locations(AssemblyProgram* program) override;
+
CheckBool ParseFile(AssemblyProgram* target) WARN_UNUSED_RESULT;
bool ParseAbs32Relocs();
void ParseRel32RelocsFromSections();
diff --git a/courgette/encoded_program.cc b/courgette/encoded_program.cc
index e14b37e..129728a 100644
--- a/courgette/encoded_program.cc
+++ b/courgette/encoded_program.cc
@@ -137,38 +137,15 @@
EncodedProgram::EncodedProgram() {}
EncodedProgram::~EncodedProgram() {}
-CheckBool EncodedProgram::DefineLabels(const RVAToLabel& abs32_labels,
- const RVAToLabel& rel32_labels) {
- // which == 0 => abs32; which == 1 => rel32.
- for (int which = 0; which < 2; ++which) {
- const RVAToLabel& labels = which == 0 ? abs32_labels : rel32_labels;
- RvaVector& rvas = which == 0 ? abs32_rva_ : rel32_rva_;
-
- if (!rvas.resize(LabelManager::GetIndexBound(labels), kUnassignedRVA))
- return false;
-
- // For each Label, write its RVA to assigned index.
- for (const auto& rva_and_label : labels) {
- const Label& label = *rva_and_label.second;
- DCHECK_EQ(rva_and_label.first, label.rva_);
- DCHECK_NE(label.index_, Label::kNoIndex);
- DCHECK_EQ(rvas[label.index_], kUnassignedRVA)
- << "DefineLabels() double assigned " << label.index_;
- rvas[label.index_] = label.rva_;
- }
-
- // Replace all unassigned slots with the value at the previous index so they
- // delta-encode to zero. (There might be better values than zero. The way to
- // get that is have the higher level assembly program assign the unassigned
- // slots.)
- RVA previous = 0;
- for (RVA& rva : rvas) {
- if (rva == kUnassignedRVA)
- rva = previous;
- else
- previous = rva;
- }
+CheckBool EncodedProgram::ImportLabels(
+ const LabelManager& abs32_label_manager,
+ const LabelManager& rel32_label_manager) {
+ if (!WriteRvasToList(abs32_label_manager, &abs32_rva_) ||
+ !WriteRvasToList(rel32_label_manager, &rel32_rva_)) {
+ return false;
}
+ FillUnassignedRvaSlots(&abs32_rva_);
+ FillUnassignedRvaSlots(&rel32_rva_);
return true;
}
@@ -733,6 +710,43 @@
RelocBlockPOD pod;
};
+// static
+// Updates |rvas| so |rvas[label.index_] == label.rva_| for each |label| in
+// |label_manager|, assuming |label.index_| is properly assigned. Takes care of
+// |rvas| resizing. Unused slots in |rvas| are assigned |kUnassignedRVA|.
+// Returns true on success, and false otherwise.
+CheckBool EncodedProgram::WriteRvasToList(const LabelManager& label_manager,
+ RvaVector* rvas) {
+ rvas->clear();
+ int index_bound = LabelManager::GetLabelIndexBound(label_manager.Labels());
+ if (!rvas->resize(index_bound, kUnassignedRVA))
+ return false;
+
+ // For each Label, write its RVA to assigned index.
+ for (const Label& label : label_manager.Labels()) {
+ DCHECK_NE(label.index_, Label::kNoIndex);
+ DCHECK_EQ((*rvas)[label.index_], kUnassignedRVA)
+ << "ExportToList() double assigned " << label.index_;
+ (*rvas)[label.index_] = label.rva_;
+ }
+ return true;
+}
+
+// static
+// Replaces all unassigned slots in |rvas| with the value at the previous index
+// so they delta-encode to zero. (There might be better values than zero. The
+// way to get that is have the higher level assembly program assign the
+// unassigned slots.)
+void EncodedProgram::FillUnassignedRvaSlots(RvaVector* rvas) {
+ RVA previous = 0;
+ for (RVA& rva : *rvas) {
+ if (rva == kUnassignedRVA)
+ rva = previous;
+ else
+ previous = rva;
+ }
+}
+
CheckBool EncodedProgram::GeneratePeRelocations(SinkStream* buffer,
uint8_t type) {
std::sort(abs32_relocs_.begin(), abs32_relocs_.end());
diff --git a/courgette/encoded_program.h b/courgette/encoded_program.h
index d0ce945f..814a45c 100644
--- a/courgette/encoded_program.h
+++ b/courgette/encoded_program.h
@@ -50,10 +50,11 @@
// (1) The image base can be specified at any time.
void set_image_base(uint64_t base) { image_base_ = base; }
- // (2) Address tables and indexes defined first.
- CheckBool DefineLabels(const RVAToLabel& abs32_labels,
- const RVAToLabel& rel32_labels) WARN_UNUSED_RESULT;
+ // (2) Address tables and indexes imported first.
+ CheckBool ImportLabels(const LabelManager& abs32_label_manager,
+ const LabelManager& rel32_label_manager)
+ WARN_UNUSED_RESULT;
// (3) Add instructions in the order needed to generate bytes of file.
// NOTE: If any of these methods ever fail, the EncodedProgram instance
@@ -114,6 +115,14 @@
typedef NoThrowBuffer<OP> OPVector;
void DebuggingSummary();
+
+ // Helper for ImportLabels().
+ static CheckBool WriteRvasToList(const LabelManager& label_manager,
+ RvaVector* rvas);
+
+ // Helper for ImportLabels().
+ static void FillUnassignedRvaSlots(RvaVector* rvas);
+
CheckBool GeneratePeRelocations(SinkStream* buffer,
uint8_t type) WARN_UNUSED_RESULT;
CheckBool GenerateElfRelocations(Elf32_Word pending_elf_relocation_table,
diff --git a/courgette/encoded_program_unittest.cc b/courgette/encoded_program_unittest.cc
index 52f8d88..e8e4614 100644
--- a/courgette/encoded_program_unittest.cc
+++ b/courgette/encoded_program_unittest.cc
@@ -21,55 +21,32 @@
namespace {
// Helper class to instantiate RVAToLabel while managing allocation.
-class RVAToLabelMaker {
+class TestLabelManager : public LabelManager {
public:
- RVAToLabelMaker() {}
- ~RVAToLabelMaker() {}
-
- // Adds a Label for storage. Must be called before Make(), since |labels_map_|
- // has pointers into |labels_|, and |labels_.push_back()| can invalidate them.
- void Add(int index, RVA rva) {
- DCHECK(label_map_.empty());
+ void RawAddLabel(int index, RVA rva) {
labels_.push_back(Label(rva, index)); // Don't care about |count_|.
}
-
- // Initializes |label_map_| and returns a pointer to it.
- RVAToLabel* Make() {
- for (size_t i = 0; i < labels_.size(); ++i) {
- DCHECK_EQ(0U, label_map_.count(labels_[i].rva_));
- label_map_[labels_[i].rva_] = &labels_[i];
- }
- return &label_map_;
- }
-
- const std::vector<Label>& GetLabels() const { return labels_; }
-
- private:
- std::vector<Label> labels_; // Stores all Labels.
- RVAToLabel label_map_; // Has pointers to |labels_| elements.
};
// Creates a simple new program with given addresses. The orders of elements
// in |abs32_specs| and |rel32_specs| are important.
std::unique_ptr<EncodedProgram> CreateTestProgram(
- RVAToLabelMaker* abs32_maker,
- RVAToLabelMaker* rel32_maker) {
- RVAToLabel* abs32_labels = abs32_maker->Make();
- RVAToLabel* rel32_labels = rel32_maker->Make();
-
+ const TestLabelManager& abs32_label_manager,
+ const TestLabelManager& rel32_label_manager) {
std::unique_ptr<EncodedProgram> program(new EncodedProgram());
uint32_t base = 0x00900000;
program->set_image_base(base);
- EXPECT_TRUE(program->DefineLabels(*abs32_labels, *rel32_labels));
+ EXPECT_TRUE(program->ImportLabels(abs32_label_manager, rel32_label_manager));
EXPECT_TRUE(program->AddOrigin(0)); // Start at base.
- // Arbitrary: Add instructions in the order they're defined in |*_maker|.
- for (const Label& label : abs32_maker->GetLabels())
+ // Add instructions. Since we're using TestLabelManager, Labels are sorted in
+ // the order they're added via Add().
+ for (const Label& label : abs32_label_manager.Labels())
EXPECT_TRUE(program->AddAbs32(label.index_));
- for (const Label& label : rel32_maker->GetLabels())
+ for (const Label& label : rel32_label_manager.Labels())
EXPECT_TRUE(program->AddRel32(label.index_));
return program;
@@ -91,14 +68,14 @@
// check that the bits produced are as expected.
TEST(EncodedProgramTest, Test) {
// ABS32 index 7 <-- base + 4.
- RVAToLabelMaker abs32_label_maker;
- abs32_label_maker.Add(7, 4);
+ TestLabelManager abs32_label_manager;
+ abs32_label_manager.RawAddLabel(7, 4);
// REL32 index 5 <-- base + 0.
- RVAToLabelMaker rel32_label_maker;
- rel32_label_maker.Add(5, 0);
+ TestLabelManager rel32_label_manager;
+ rel32_label_manager.RawAddLabel(5, 0);
std::unique_ptr<EncodedProgram> program(
- CreateTestProgram(&abs32_label_maker, &rel32_label_maker));
+ CreateTestProgram(abs32_label_manager, rel32_label_manager));
// Serialize and deserialize.
SinkStreamSet sinks;
@@ -139,18 +116,18 @@
// contents of the address streams.
TEST(EncodedProgramTest, TestWriteAddress) {
// Absolute addresses by index: [_, _, _, 2, _, 23, _, 11].
- RVAToLabelMaker abs32_label_maker;
- abs32_label_maker.Add(7, 11);
- abs32_label_maker.Add(3, 2);
- abs32_label_maker.Add(5, 23);
+ TestLabelManager abs32_label_manager;
+ abs32_label_manager.RawAddLabel(7, 11);
+ abs32_label_manager.RawAddLabel(3, 2);
+ abs32_label_manager.RawAddLabel(5, 23);
// Relative addresses by index: [16, 7, _, 32].
- RVAToLabelMaker rel32_label_maker;
- rel32_label_maker.Add(0, 16);
- rel32_label_maker.Add(3, 32);
- rel32_label_maker.Add(1, 7);
+ TestLabelManager rel32_label_manager;
+ rel32_label_manager.RawAddLabel(0, 16);
+ rel32_label_manager.RawAddLabel(3, 32);
+ rel32_label_manager.RawAddLabel(1, 7);
std::unique_ptr<EncodedProgram> program(
- CreateTestProgram(&abs32_label_maker, &rel32_label_maker));
+ CreateTestProgram(abs32_label_manager, rel32_label_manager));
SinkStreamSet sinks;
EXPECT_TRUE(program->WriteTo(&sinks));
diff --git a/courgette/image_utils.h b/courgette/image_utils.h
index 643b9fb2..19a04d5 100644
--- a/courgette/image_utils.h
+++ b/courgette/image_utils.h
@@ -101,6 +101,8 @@
// to the matching RVA target on demand without extra storage.
class RvaVisitor {
public:
+ virtual ~RvaVisitor() { }
+
// Returns the number of remaining RVAs to visit.
virtual size_t Remaining() const = 0;
@@ -118,7 +120,8 @@
public:
// Assumes |v| does not change for the lifetime of this instance.
explicit VectorRvaVisitor(const std::vector<T>& v)
- : it_(v.begin()), end_(v.end()) {}
+ : it_(v.begin()), end_(v.end()) { }
+ ~VectorRvaVisitor() override { }
// RvaVisitor interfaces.
size_t Remaining() const override { return std::distance(it_, end_); }
@@ -134,7 +137,8 @@
class TrivialRvaVisitor : public VectorRvaVisitor<RVA> {
public:
explicit TrivialRvaVisitor(const std::vector<RVA>& rvas)
- : VectorRvaVisitor<RVA>(rvas) {}
+ : VectorRvaVisitor<RVA>(rvas) { }
+ ~TrivialRvaVisitor() override { }
// VectorRvaVisitor<RVA> interfaces.
RVA Get() const override { return *it_; }
diff --git a/courgette/label_manager.cc b/courgette/label_manager.cc
index a109d8b..51e0d5de 100644
--- a/courgette/label_manager.cc
+++ b/courgette/label_manager.cc
@@ -96,17 +96,6 @@
LabelManager::~LabelManager() {}
// static
-int LabelManager::GetIndexBound(const RVAToLabel& labels_map) {
- int max_index = -1;
- for (const auto& rva_and_label : labels_map) {
- const Label& label = *rva_and_label.second;
- if (label.index_ != Label::kNoIndex)
- max_index = std::max(max_index, label.index_);
- }
- return max_index + 1;
-}
-
-// static
int LabelManager::GetLabelIndexBound(const LabelVector& labels) {
int max_index = -1;
for (const Label& label : labels) {
diff --git a/courgette/label_manager.h b/courgette/label_manager.h
index d60a274..4b252405 100644
--- a/courgette/label_manager.h
+++ b/courgette/label_manager.h
@@ -80,10 +80,6 @@
LabelManager();
~LabelManager();
- // Returns an exclusive upper bound for all existing indexes in |labels_map|.
- // TODO(huangs): Remove once all callers are gone.
- static int GetIndexBound(const RVAToLabel& labels_map);
-
// Returns an exclusive upper bound for all assigned indexes in |labels|.
static int GetLabelIndexBound(const LabelVector& labels);
diff --git a/courgette/program_detector.cc b/courgette/program_detector.cc
index 80dbd2d..60b295f 100644
--- a/courgette/program_detector.cc
+++ b/courgette/program_detector.cc
@@ -78,9 +78,6 @@
if (!disassembler->Disassemble(program.get()))
return C_DISASSEMBLY_FAILED;
- if (!program->TrimLabels())
- return C_TRIM_FAILED;
-
*output = std::move(program);
return C_OK;
}
diff --git a/courgette/rel32_finder_win32_x86.cc b/courgette/rel32_finder_win32_x86.cc
index 0ed492f..61ff9696 100644
--- a/courgette/rel32_finder_win32_x86.cc
+++ b/courgette/rel32_finder_win32_x86.cc
@@ -87,12 +87,13 @@
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.
+ // 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);
// Valid, rel32 target must be within image, and within this section.
// Subsumes |target_rva| != |kUnassignedRVA|.