 Chromium Code Reviews
 Chromium Code Reviews Issue 2462993003:
  [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in 2 passes.  (Closed)
    
  
    Issue 2462993003:
  [Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in 2 passes.  (Closed) 
  | Index: courgette/disassembler_elf_32.cc | 
| diff --git a/courgette/disassembler_elf_32.cc b/courgette/disassembler_elf_32.cc | 
| index a6cb16fab8a5a6a558a47d9c663c23a4329749fe..87e2b983054b4783748dff7af85c80a8661d8662 100644 | 
| --- a/courgette/disassembler_elf_32.cc | 
| +++ b/courgette/disassembler_elf_32.cc | 
| @@ -179,8 +179,18 @@ bool DisassemblerElf32::Disassemble(AssemblyProgram* target) { | 
| PrecomputeLabels(target); | 
| RemoveUnusedRel32Locations(target); | 
| - if (!ParseFile(target)) | 
| + // Pass 1: Count the space needed to store instructions. | 
| + InstructionCountReceptor* count_receptor = nullptr; | 
| 
grt (UTC plus 2)
2016/11/04 09:38:53
how will this be used?
 
huangs
2016/11/04 18:20:40
This CL is focusing on handling the callers.  The
 | 
| + if (!target->CreateInstructionCountReceptor(&count_receptor) || | 
| + !ParseFile(target, count_receptor)) { | 
| return false; | 
| + } | 
| + // Pass 2: Emit all instructions. | 
| + InstructionStoreReceptor* store_receptor = nullptr; | 
| + if (!target->CreateInstructionStoreReceptor(&store_receptor) || | 
| + !ParseFile(target, store_receptor)) { | 
| + return false; | 
| + } | 
| // Finally sort rel32 locations. | 
| std::sort(rel32_locations_.begin(), | 
| @@ -301,7 +311,7 @@ CheckBool DisassemblerElf32::SectionName(const Elf32_Shdr& shdr, | 
| CheckBool DisassemblerElf32::RVAsToFileOffsets( | 
| const std::vector<RVA>& rvas, | 
| - std::vector<FileOffset>* file_offsets) { | 
| + std::vector<FileOffset>* file_offsets) const { | 
| file_offsets->clear(); | 
| file_offsets->reserve(rvas.size()); | 
| for (RVA rva : rvas) { | 
| @@ -314,7 +324,7 @@ CheckBool DisassemblerElf32::RVAsToFileOffsets( | 
| } | 
| CheckBool DisassemblerElf32::RVAsToFileOffsets( | 
| - std::vector<std::unique_ptr<TypedRVA>>* typed_rvas) { | 
| + std::vector<std::unique_ptr<TypedRVA>>* typed_rvas) const { | 
| for (auto& typed_rva : *typed_rvas) { | 
| FileOffset file_offset = RVAToFileOffset(typed_rva->rva()); | 
| if (file_offset == kNoFileOffset) | 
| @@ -350,7 +360,8 @@ void DisassemblerElf32::RemoveUnusedRel32Locations(AssemblyProgram* program) { | 
| rel32_locations_.resize(std::distance(rel32_locations_.begin(), tail_it)); | 
| } | 
| -CheckBool DisassemblerElf32::ParseFile(AssemblyProgram* program) { | 
| +CheckBool DisassemblerElf32::ParseFile(AssemblyProgram* program, | 
| + InstructionReceptor* receptor) const { | 
| // Walk all the bytes in the file, whether or not in a section. | 
| FileOffset file_offset = 0; | 
| @@ -384,24 +395,21 @@ CheckBool DisassemblerElf32::ParseFile(AssemblyProgram* program) { | 
| if (section_header->sh_type == SHT_NOBITS) | 
| continue; | 
| - if (!ParseSimpleRegion(file_offset, section_header->sh_offset, program)) | 
| + if (!ParseSimpleRegion(file_offset, section_header->sh_offset, receptor)) | 
| return false; | 
| file_offset = section_header->sh_offset; | 
| switch (section_header->sh_type) { | 
| case SHT_REL: | 
| - if (!ParseRelocationSection(section_header, program)) | 
| + if (!ParseRelocationSection(section_header, receptor)) | 
| return false; | 
| file_offset = section_header->sh_offset + section_header->sh_size; | 
| break; | 
| case SHT_PROGBITS: | 
| - if (!ParseProgbitsSection(section_header, | 
| - ¤t_abs_offset, | 
| - end_abs_offset, | 
| - ¤t_rel, | 
| - end_rel, | 
| - program)) { | 
| + if (!ParseProgbitsSection(section_header, ¤t_abs_offset, | 
| + end_abs_offset, ¤t_rel, end_rel, | 
| + program, receptor)) { | 
| return false; | 
| } | 
| file_offset = section_header->sh_offset + section_header->sh_size; | 
| @@ -432,7 +440,7 @@ CheckBool DisassemblerElf32::ParseFile(AssemblyProgram* program) { | 
| } | 
| // Rest of the file past the last section | 
| - if (!ParseSimpleRegion(file_offset, length(), program)) | 
| + if (!ParseSimpleRegion(file_offset, length(), receptor)) | 
| return false; | 
| // Make certain we consume all of the relocations as expected | 
| @@ -445,14 +453,15 @@ CheckBool DisassemblerElf32::ParseProgbitsSection( | 
| std::vector<FileOffset>::iterator end_abs_offset, | 
| std::vector<std::unique_ptr<TypedRVA>>::iterator* current_rel, | 
| std::vector<std::unique_ptr<TypedRVA>>::iterator end_rel, | 
| - AssemblyProgram* program) { | 
| + AssemblyProgram* program, | 
| + InstructionReceptor* receptor) const { | 
| // Walk all the bytes in the file, whether or not in a section. | 
| FileOffset file_offset = section_header->sh_offset; | 
| FileOffset section_end = section_header->sh_offset + section_header->sh_size; | 
| Elf32_Addr origin = section_header->sh_addr; | 
| FileOffset origin_offset = section_header->sh_offset; | 
| - if (!program->EmitOriginInstruction(origin)) | 
| + if (!receptor->EmitOrigin(origin)) | 
| return false; | 
| while (file_offset < section_end) { | 
| @@ -479,7 +488,7 @@ CheckBool DisassemblerElf32::ParseProgbitsSection( | 
| next_relocation = (**current_rel)->file_offset(); | 
| if (next_relocation > file_offset) { | 
| - if (!ParseSimpleRegion(file_offset, next_relocation, program)) | 
| + if (!ParseSimpleRegion(file_offset, next_relocation, receptor)) | 
| return false; | 
| file_offset = next_relocation; | 
| @@ -493,7 +502,7 @@ CheckBool DisassemblerElf32::ParseProgbitsSection( | 
| Label* label = program->FindAbs32Label(target_rva); | 
| CHECK(label); | 
| - if (!program->EmitAbs32(label)) | 
| + if (!receptor->EmitAbs32(label)) | 
| return false; | 
| file_offset += sizeof(RVA); | 
| ++(*current_abs_offset); | 
| @@ -513,7 +522,7 @@ CheckBool DisassemblerElf32::ParseProgbitsSection( | 
| Label* label = program->FindRel32Label(target_rva); | 
| CHECK(label); | 
| - if (!(**current_rel)->EmitInstruction(program, label)) | 
| + if (!(**current_rel)->EmitInstruction(label, receptor)) | 
| return false; | 
| file_offset += (**current_rel)->op_size(); | 
| ++(*current_rel); | 
| @@ -522,20 +531,21 @@ CheckBool DisassemblerElf32::ParseProgbitsSection( | 
| } | 
| // Rest of the section (if any) | 
| - return ParseSimpleRegion(file_offset, section_end, program); | 
| + return ParseSimpleRegion(file_offset, section_end, receptor); | 
| } | 
| -CheckBool DisassemblerElf32::ParseSimpleRegion(FileOffset start_file_offset, | 
| - FileOffset end_file_offset, | 
| - AssemblyProgram* program) { | 
| +CheckBool DisassemblerElf32::ParseSimpleRegion( | 
| + FileOffset start_file_offset, | 
| + FileOffset end_file_offset, | 
| + InstructionReceptor* receptor) const { | 
| // Callers don't guarantee start < end | 
| if (start_file_offset >= end_file_offset) | 
| return true; | 
| const size_t len = end_file_offset - start_file_offset; | 
| - if (!program->EmitBytesInstruction(FileOffsetToPointer(start_file_offset), | 
| - len)) { | 
| + if (!receptor->EmitMultipleBytes(FileOffsetToPointer(start_file_offset), | 
| + len)) { | 
| return false; | 
| } |