[Courgette] Refactor: Add and use Instruction*Receptor classes; call ParseFile() in 2 passes.

courgette/assembly_program.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "courgette/assembly_program.h"
 
 #include <memory.h>
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 89 matching lines @@
   uint16_t op_size() const { return op_size_; }
 
  private:
   uint16_t compressed_op_;
   const uint8_t* arm_op_;
   uint16_t op_size_;
 };
 
 }  // namespace
 
+/******** InstructionReceptor ********/
+
+InstructionReceptor::InstructionReceptor() {}
+
+InstructionReceptor::~InstructionReceptor() {}
+
+/******** InstructionCountReceptor ********/
+
+InstructionCountReceptor::InstructionCountReceptor() : size_(0) {}
+
+InstructionCountReceptor::~InstructionCountReceptor() {}
+
+// TODO(huangs): Populate these with size_ += ...

[grt (UTC plus 2), 2016/11/04 09:38:53]

TODO when?

[huangs, 2016/11/04 18:20:39]

The follow-up CL, which focuses on AssemblyProgram.  Added 2016/11.

+CheckBool InstructionCountReceptor::EmitPeRelocs() {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitElfRelocation() {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitElfARMRelocation() {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitOrigin(RVA rva) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitSingleByte(uint8_t byte) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitMultipleBytes(const uint8_t* value,
+                                                      size_t len) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitRel32(Label* label) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitRel32ARM(uint16_t op,
+                                                 Label* label,
+                                                 const uint8_t* arm_op,
+                                                 uint16_t op_size) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitAbs32(Label* label) {
+  return true;
+}
+
+CheckBool InstructionCountReceptor::EmitAbs64(Label* label) {
+  return true;
+}
+
+/******** InstructionStoreReceptor ********/
+
+// TODO(huangs): Populate these.

[grt (UTC plus 2), 2016/11/04 09:38:53]

when?

[huangs, 2016/11/04 18:20:39]

Same.

+InstructionStoreReceptor::InstructionStoreReceptor(size_t init_size,
+                                                   AssemblyProgram* program)
+    : program_(program) {
+  CHECK(program_ != nullptr);

[grt (UTC plus 2), 2016/11/04 09:38:53]

CHECK_NE(program_, nullptr);

[huangs, 2016/11/04 18:20:39]

Should be consistent with later uses (DCHECK). Comparing choices:

CHECK(pointer);  // Must not be null.
CHECK(!pointer);  // Must be null.

vs.

CHECK_NE(pointer, nullptr);  // Must not be null.
CHECK_EQ(pointer, nullptr);  // Must be null.

Changing to the first one for now because I think it's easier to read. But let
me know if you prefer the other and I'll change throughout.

[grt (UTC plus 2), 2016/11/07 09:36:17]

I like the brief approach as well.

[huangs, 2016/11/07 19:46:52]

Acknowledged.

+}
+
+InstructionStoreReceptor::~InstructionStoreReceptor() {}
+
+CheckBool InstructionStoreReceptor::EmitPeRelocs() {
+  return program_->EmitPeRelocs();
+}
+
+CheckBool InstructionStoreReceptor::EmitElfRelocation() {
+  return program_->EmitElfRelocation();
+}
+
+CheckBool InstructionStoreReceptor::EmitElfARMRelocation() {
+  return program_->EmitElfARMRelocation();
+}
+
+CheckBool InstructionStoreReceptor::EmitOrigin(RVA rva) {
+  return program_->EmitOrigin(rva);
+}
+
+CheckBool InstructionStoreReceptor::EmitSingleByte(uint8_t byte) {
+  return program_->EmitSingleByte(byte);
+}
+
+CheckBool InstructionStoreReceptor::EmitMultipleBytes(const uint8_t* value,
+                                                      size_t len) {
+  return program_->EmitMultipleBytes(value, len);
+}
+
+CheckBool InstructionStoreReceptor::EmitRel32(Label* label) {
+  return program_->EmitRel32(label);
+}
+
+CheckBool InstructionStoreReceptor::EmitRel32ARM(uint16_t op,
+                                                 Label* label,
+                                                 const uint8_t* arm_op,
+                                                 uint16_t op_size) {
+  return program_->EmitRel32ARM(op, label, arm_op, op_size);
+}
+
+CheckBool InstructionStoreReceptor::EmitAbs32(Label* label) {
+  return program_->EmitAbs32(label);
+}
+
+CheckBool InstructionStoreReceptor::EmitAbs64(Label* label) {
+  return program_->EmitAbs64(label);
+}
+
+/******** AssemblyProgram ********/
+
 AssemblyProgram::AssemblyProgram(ExecutableType kind)
   : kind_(kind), image_base_(0) {
 }
 
 AssemblyProgram::~AssemblyProgram() {
   for (size_t i = 0;  i < instructions_.size();  ++i) {
     Instruction* instruction = instructions_[i];
     if (instruction->op() != DEFBYTE)  // Owned by byte_instruction_cache_.
       UncheckedDelete(instruction);
   }
   if (byte_instruction_cache_.get()) {
     for (size_t i = 0;  i < 256;  ++i)
       UncheckedDelete(byte_instruction_cache_[i]);
   }
 }
 
-CheckBool AssemblyProgram::EmitPeRelocsInstruction() {
+CheckBool AssemblyProgram::EmitPeRelocs() {
   return Emit(ScopedInstruction(UncheckedNew<PeRelocsInstruction>()));
 }
 
-CheckBool AssemblyProgram::EmitElfRelocationInstruction() {
+CheckBool AssemblyProgram::EmitElfRelocation() {
   return Emit(ScopedInstruction(UncheckedNew<ElfRelocsInstruction>()));
 }
 
-CheckBool AssemblyProgram::EmitElfARMRelocationInstruction() {
+CheckBool AssemblyProgram::EmitElfARMRelocation() {
   return Emit(ScopedInstruction(UncheckedNew<ElfARMRelocsInstruction>()));
 }
 
-CheckBool AssemblyProgram::EmitOriginInstruction(RVA rva) {
+CheckBool AssemblyProgram::EmitOrigin(RVA rva) {
   return Emit(ScopedInstruction(UncheckedNew<OriginInstruction>(rva)));
 }
 
-CheckBool AssemblyProgram::EmitByteInstruction(uint8_t byte) {
+CheckBool AssemblyProgram::EmitSingleByte(uint8_t byte) {
   return EmitShared(GetByteInstruction(byte));
 }
 
-CheckBool AssemblyProgram::EmitBytesInstruction(const uint8_t* values,
-                                                size_t len) {
+CheckBool AssemblyProgram::EmitMultipleBytes(const uint8_t* values,
+                                             size_t len) {
   return Emit(ScopedInstruction(UncheckedNew<BytesInstruction>(values, len)));
 }
 
 CheckBool AssemblyProgram::EmitRel32(Label* label) {
   return Emit(
       ScopedInstruction(UncheckedNew<InstructionWithLabel>(REL32, label)));
 }
 
 CheckBool AssemblyProgram::EmitRel32ARM(uint16_t op,
                                         Label* label,
@@ skipping 63 matching lines @@
     const AssemblyProgram::LabelHandlerMap& handler_map) const {
   for (const Instruction* instruction : instructions_) {
     LabelHandlerMap::const_iterator it = handler_map.find(instruction->op());
     if (it != handler_map.end()) {
       it->second.Run(
           static_cast<const InstructionWithLabel*>(instruction)->label());
     }
   }
 }
 
+CheckBool AssemblyProgram::CreateInstructionCountReceptor(

[grt (UTC plus 2), 2016/11/04 09:38:53]

why CheckBool rather than returning a pointer since this can never fail?

[huangs, 2016/11/04 18:20:39]

For symmetry with CreateInstructionStoreReceptor(), which may fail.

[grt (UTC plus 2), 2016/11/07 09:36:17]

Ah. May fail in the future in a different CL. I'm not sure that splitting this
into two CLs makes sense since this CL raises questions that are answered in the
other. I'll try to review them independently anyway.

[huangs, 2016/11/07 19:46:52]

Okay, thanks!

+    InstructionCountReceptor** ret) {
+  DCHECK(ret);
+  DCHECK(!count_receptor_);
+  count_receptor_.reset(new InstructionCountReceptor);
+  *ret = count_receptor_.get();
+  return true;
+}
+
+CheckBool AssemblyProgram::CreateInstructionStoreReceptor(
+    InstructionStoreReceptor** ret) {
+  DCHECK(ret);
+  DCHECK(count_receptor_);
+  DCHECK(!store_receptor_);
+  store_receptor_.reset(
+      new InstructionStoreReceptor(count_receptor_->size(), this));
+  *ret = store_receptor_.get();
+  return true;
+}
+
 CheckBool AssemblyProgram::Emit(ScopedInstruction instruction) {
   if (!instruction || !instructions_.push_back(instruction.get()))
     return false;
   // Ownership successfully passed to instructions_.
   ignore_result(instruction.release());
   return true;
 }
 
 CheckBool AssemblyProgram::EmitShared(Instruction* instruction) {
   DCHECK(!instruction || instruction->op() == DEFBYTE);
@@ skipping 226 matching lines @@
 Status Encode(const AssemblyProgram& program,
               std::unique_ptr<EncodedProgram>* output) {
   // Explicitly release any memory associated with the output before encoding.
   output->reset();
 
   *output = program.Encode();
   return (*output) ? C_OK : C_GENERAL_ERROR;
 }
 
 }  // namespace courgette