[llvm-objcopy] Refactor llvm-objcopy to use reader and writer objects

[jakehehrlich]

Summary

While writing code for input and output formats in llvm-objcopy it became apparent that there was a code health problem. This change attempts to solve that problem by refactoring the code to use Reader and Writer objects that can read in different objects in different formats, convert them to a single shared internal representation, and then write them to any other representation.

New classes:
Reader: the base class used to construct instances of the internal representation
Writer: the base class used to write out instances of the internal representation
ELFBuilder: a helper class for ELFWriter that takes an ELFFile and converts it to a Object
SectionVisitor: it became necessary to remove writeSection from SectionBase because, under the new Reader/Writer scheme, it's possible to convert between ELF Types such as ELF32LE and ELF32BE. This isn't possible with writeSection because it (dynamically) depends on the underlying section type *and* (statically) depends on the ELF type. Bad things would happen if the underlying sections for ELF32LE were used for writing to ELF64BE. To avoid this code smell (which would have compiled, run, and output some nonsesnse) I decoupled writing of sections from a class.
SectionWriter: This is just the ELFT templated implementation of SectionVisitor. Many classes now have this class as a friend so that the writing methods in this class can write out private data.
ELFWriter: This is the Writer that outputs to ELF
BinaryWriter: This is the Writer that outputs to Binary
ElfType: Because the ELF Type is not a part of the Object anymore we need a way to construct the correct default Writer based on properties of the Reader. This enum just keeps track of the ELF type of the input so it can be used as the default output type as well.

Object has correspondingly undergone some serious changes as well. It now has more generic methods for building and manipulating ELF binaries. This interface makes ELFBuilder easy enough to use and will make the BinaryReader/Builder easy to create as well. Most changes in this diff are cosmetic and deal with the fact that a method has been moved from one class to another or a change from a pointer to a reference. Almost no changes should result in a functional difference (this is after all a refactor). One minor functional change was made and the result can be seen in remove-shstrtab-error.test. The fact that it fails hasn't changed but the error message has changed because that failure is detected at a later point in the code now (because WriteSectionHeaders is a property of the ElfWriter *not* a property of the Object). I'd say roughly 80-90% of this code is cosmetically different, 10-19% is different but functionally the same, and 1-5% is functionally different despite not causing a change in tests.

Known Code Smells:

I'm actully aware of a number of issues I'd personally like to fix before this is submitted. This code is still rough and in need of some polish. For instance there's a bit of code duplication that I haven't removed. There are several "if(auto *o = dyn_cast<ELFObjectFile<...>(...))" blocks around that I'd like to remove. Also the handling of BufPtr and SectionWriter in BianryWriter and ElfWriter is duplicated. I'd like to remove that (I think that one can be accomplished by just folding it into the Writer base class). Also it might be possible to split this up into separate diffs. Any recommendations on how to split it up would be welcome. I figured now that it a) compiles and b) appears to pass all tests that I could start getting some feedback.

[jhenderson]

Overall, the approach seems sound. I particularly like how it removes the templates from many of our classes, and removes the unfortunate SymbolTableSectionImpl class. Have you tried extending this with new input types, to make sure this design works?

I agree with your points about the code smells, and think they should be fixed too. As to how to split it up, could the SectionWriter/SectionVisitor related changes be done first? After that a logical split would be to do the Reader-related classes, then the Writer classes, if it's possible without having to make too many intermediate changes that are just going to be thrown away.

tools/llvm-objcopy/Object.cpp

77 ↗

(On Diff #130334)

This should go above the previous function.

667 ↗

(On Diff #130334)

I'm not sure you want this to take an r-value reference? Shouldn't it be a const reference, since you don't need to change the range?

966 ↗

(On Diff #130334)

Braces here aren't needed.

1060 ↗

(On Diff #130334)

This line looks a little dodgy. Doesn't that mean we'll write ELF64LE sections to binary output, no matter what the input was?

tools/llvm-objcopy/Object.h

76–82 ↗

(On Diff #130334)

It would probably make more sense for this to come after SectionVisitor.

499 ↗

(On Diff #130334)

What items are in this namespace that means we need this using in a header?

503–505 ↗

(On Diff #130334)

This will require us to always have the ELFReader allocated until we write, which seems a little less than ideal?

518 ↗

(On Diff #130334)

Do you plan on having any child classes? Otherwise, why protected?

tools/llvm-objcopy/llvm-objcopy.cpp

175–182 ↗

(On Diff #130334)

Any particular reason this is no longer in its own function?

317 ↗

(On Diff #130334)

Unused return value and unnecessary braces.

329 ↗

(On Diff #130334)

What's complicated? :)

[jakehehrlich]

Have you tried extending this with new input types, to make sure this design works?

No but I was thinking about the case while designing this approach. I should probably actually do that just to make sure. In principal it should just be as simple as making a new Reader class and constructs all the needed parts.

tools/llvm-objcopy/Object.cpp

667 ↗

(On Diff #130334)

It's not an rvalue reference, it's a "universal reference". It's whatever the user passes it.

1060 ↗

(On Diff #130334)

Ah yes, I remember having this dilemma before pushing it to the side in favor of progress. I was thinking about splitting up SectionWriter into a non-ELFT-templated base class and a separate ELFT-template base class and then *also* a binary section writer (that is non-ELFT-templated) that throws errors for the methods that need ELFT to output correctly. How does that sound?

tools/llvm-objcopy/Object.h

499 ↗

(On Diff #130334)

ELFFile, ELFObjectFile, and maybe something else I'm not quite remembering. I should probably just explicitly use "using" on those things I need rather than massively pollute the whole namespace, huh?

503–505 ↗

(On Diff #130334)

I want to keep it so that the data between the Object and the Binary is not duplicated. I do agree that it doesn't seem like the Reader shouldn't *have* to stay open the whole time but I'd like the Binary to stay open the whole time (until all output files have been written at least). I think this can be remedied in two opposite ways. One way is for the Reader to maintain ownership of the Objects so that the Binary and the Objects that reference it's data have a shared lifetime. The other idea is that this could perhaps be decoupled and the Reader could store a reference to the Binary. It then becomes the user's job to make sure that the Bianry stays open as long as any object created by the Reader. Right now it's (oddly) the user's job to make sure that the Reader stays open as like as any object created by it. I agree that's wrong.

What are your thoughts?

518 ↗

(On Diff #130334)

Good, point. I don't think I'll ever have any child classes. It was mostly holdover from the previous Object type.

tools/llvm-objcopy/llvm-objcopy.cpp

175–182 ↗

(On Diff #130334)

Not that I remember, I'll put it back.

329 ↗

(On Diff #130334)

Haha, that was a comment to my self that I left in, whoops. I belive that was one of the "update to use reader and writer" todos that I wasn't 100% clear on how to acomplish.

[jakehehrlich]

As to how to split it up, could the SectionWriter/SectionVisitor related changes be done first? After that a logical split would be to do the Reader-related classes, then the Writer classes, if it's possible without having to make too many intermediate changes that are just going to be thrown away.

I'll look into that. I think the SectionWriter/SectionVisitor split is very doable and should be done without question. The Reader and Writer split was the split I was thinking of initially (I was grouping the SectionVisitor split as being a part of the Writer split). I think I'm just going to have to try that one and see what happens as I don't have a good intuition for how hard that split would be.

[jakehehrlich]

1. Fixed BufPtr duplication in Writers
2. Moved Writer class to below SectionWriter
3. Removed using namespace object in favor of using specific types
4. Removed protected members (made them private)
5. Move SectionVisitor destructor to above the first SectionWriter call
6. Removed curly braces on if-stmt for WhiteSectionHeaders
7. Put code for SplitDWOToFile back in a function
8. Removed unused variable for section return
9. Removed TODO, whoops

[jhenderson]

tools/llvm-objcopy/Object.cpp

667 ↗

(On Diff #130334)

Huh, I can't say I was aware of the distinction - I had to go and look it up!

1060 ↗

(On Diff #130334)

I think that would make things a little clearer, yes. I'd like to see which methods you're referring to that should throw errors, and why we don't need them for a binary section writer though.

tools/llvm-objcopy/Object.h

499 ↗

(On Diff #130334)

Yes, I think that's probably right in a header file (no problem in cpp files, mind you).

503–505 ↗

(On Diff #130334)

I'm actually wondering whether we could transfer ownership of either the Reader, or just the Binary associated with it to the Object. That way the original Reader can be disposed of at any point after creating the Object. Unfortunately, this doesn't play particularly well with multiple input types. Of the two ideas you suggested I'd prefer that Readers have ownership of their Objects.

[jakehehrlich]

tools/llvm-objcopy/Object.cpp

1060 ↗

(On Diff #130334)

Non-allocated relocations, SHT_SYMTAB, and GnuDebugLinkSections (which are non-allocated as well) are the only ones that need to be written as such. There are many section types that should never make into the binary output that these methods would not throw errors on (string tables for instance) but there are no cases in which the above 3 sections should be output for -O binary. That way if this ever comes up and we get an error it either means a) the code is messed up or b) someone marked .symtab or .gnu-debuglink as allocated.

tools/llvm-objcopy/Object.h

503–505 ↗

(On Diff #130334)

I wonder if there's a way to use reference counting here? I can't find a way to make a binary shared_ptr but that would be the time to use it if there ever was one. I played around with Readers owning Objects though and it can be enforced really well using references. Basically objects are stored in a vector and references to them are returned on creation (I haven't considered how collection of these objects should occur). If the ELFBuilder (which is owned by the ELFReader) owns the binary then we don't even need to use unique pointers to allocate objects (they can just be allocated in the vector).

[jakehehrlich]

Added BinarySectionWriter to avoid always using ELFWriter<ELF64LE> for writing binaries. Errors are now thrown for certain input we don't know how to handle (like allocated SHT_SYMTAB, allocated relcoations, and allocated .gnudebuglink)

[jakehehrlich]

Updated code to handle data ownership in a better way. Objects now own all their data using a shared_ptr. This eliminates the potential for dangling references if the Reader is not left open.

[jhenderson]

Okay, I like the changes now. Perhaps worth adding a test with a crazy input being converted to Binary, resulting in errors on those section types? Alternatively, instead of emitting an error, maybe it makes sense to simply write the table as some arbitrary binary blob and warn? I doubt there's a use case for this though, so I think emitting an error for now is fine, and change if the need arises.

tools/llvm-objcopy/Object.cpp

73 ↗

(On Diff #130955)

bianry -> binary (also other two cases below)

I think that it would be better if these error messages had the section name included.

tools/llvm-objcopy/Object.h

94–102 ↗

(On Diff #130955)

To keep a consistent order of classes, this should go below ELFSectionWriter, or ELFWriter should go below BinaryWriter.

[jakehehrlich]

1. Moved BinarySectionWriter to after ELFWriter
2. Fixed bianry/binary typos
3. Added section names to error messages

[jakehehrlich]

Added test to check one of the error messages that BianrySectionWriter can generate. I only test the symtab case. The other two cases are hard to test without a way to edit the sections after they've been read in (with like --set-section-flags or something). Relocations are read in as DynamicRelocationSections when allocated so this whole issue is bypassed. Since GnuDebuglinkSection is never read in, only added, there isn't a way to make a binary that will trigger that code path. Once --set-section-flags exists it will be able to dynamically change these values and trigger these cases.

Oh and alloc-symtab.o is just a small relocatable ELF file where I manually flipped the flag bit to mark it allocated. I literally had to use xxd to binary patch the file. GNU objcopy won't respect --set-section-flags on symbol tables and no tool I'm aware of will produce an allocated SHT_SYMTAB section.

[jhenderson]

One small inline comment. Otherwise, I'm happy with this as-is. Are you still planning on splitting it up? It'll make it somewhat easier, I hope to spot any problems, but if it's a lot of extra work, may no longer be worth it.

test/tools/llvm-objcopy/binary-out-error.test

1 ↗

(On Diff #131149)

What's going on with the "/dev/null" here?

[jakehehrlich]

In D42222#986138, @jhenderson wrote:

One small inline comment. Otherwise, I'm happy with this as-is. Are you still planning on splitting it up? It'll make it somewhat easier, I hope to spot any problems, but if it's a lot of extra work, may no longer be worth it.

It's a lot of work to split it up. I'd rather not.

test/tools/llvm-objcopy/binary-out-error.test

1 ↗

(On Diff #131149)

We need to throw an error but that error is written to stderr not stdout but the pipe only pipes stdout to FileCheck. So we redirect stderr (2) to where stdout is going (&1) and then we redirect stdout to /dev/null so nothing comes from stdout (because as is both stderr and stdout would be pipped to FileCheck).

In this case it doesn't matter but I copy-pasted this from other code that checks errors and that come along with it. It's part of the standard way of checking error messages in lit tests. Run "git grep /dev/null | wc -l" in the test directory. I get 1113 uses but not all of those are to suppress stdout, some suppress stderr.

[jhenderson]

Okay, LGTM.

llvm/trunk/test/tools/llvm-objcopy/binary-out-error.test

+# RUN: not llvm-objcopy -O binary %p/Inputs/alloc-symtab.o %t2 2>&1 >/dev/null | FileCheck %s --check-prefix=SYMTAB
+# SYMTAB: Cannot write symbol table '.symtab' out to binary

llvm/trunk/test/tools/llvm-objcopy/remove-shstrtab-error.test

 # RUN: yaml2obj %s > %t
 # RUN: not llvm-objcopy -R .shstrtab %t %t2 2>&1 >/dev/null | FileCheck %s
 
 !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_REL
   Machine:         EM_X86_64
 
-# CHECK: Cannot remove .shstrtab because it is the section header string table.
+# CHECK: Cannot write section header table because section header string table was removed.

llvm/trunk/tools/llvm-objcopy/Object.h

 #define LLVM_TOOLS_OBJCOPY_OBJECT_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/MC/StringTableBuilder.h"
 #include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/JamCRC.h"
 #include <cstddef>
 #include <cstdint>
 #include <functional>
 #include <memory>
 #include <set>
 #include <vector>
 
 namespace llvm {
 
-class FileOutputBuffer;
 class SectionBase;
+class Section;
+class OwnedDataSection;
+class StringTableSection;
+class SymbolTableSection;
+class RelocationSection;
+class DynamicRelocationSection;
+class GnuDebugLinkSection;
 class Segment;
+class Object;
 
 class SectionTableRef {
 private:
-  ArrayRef<std::unique_ptr<SectionBase>> Sections;
+  MutableArrayRef<std::unique_ptr<SectionBase>> Sections;
 
 public:
-  SectionTableRef(ArrayRef<std::unique_ptr<SectionBase>> Secs)
+  using iterator = pointee_iterator<std::unique_ptr<SectionBase> *>;
+
+  SectionTableRef(MutableArrayRef<std::unique_ptr<SectionBase>> Secs)
       : Sections(Secs) {}
   SectionTableRef(const SectionTableRef &) = default;
 
+  iterator begin() { return iterator(Sections.data()); }
+  iterator end() { return iterator(Sections.data() + Sections.size()); }
+
   SectionBase *getSection(uint16_t Index, Twine ErrMsg);
 
   template <class T>
   T *getSectionOfType(uint16_t Index, Twine IndexErrMsg, Twine TypeErrMsg);
 };
 
+enum ElfType { ELFT_ELF32LE, ELFT_ELF64LE, ELFT_ELF32BE, ELFT_ELF64BE };
+
+class SectionVisitor {
+public:
+  virtual ~SectionVisitor();
+
+  virtual void visit(const Section &Sec) = 0;
+  virtual void visit(const OwnedDataSection &Sec) = 0;
+  virtual void visit(const StringTableSection &Sec) = 0;
+  virtual void visit(const SymbolTableSection &Sec) = 0;
+  virtual void visit(const RelocationSection &Sec) = 0;
+  virtual void visit(const DynamicRelocationSection &Sec) = 0;
+  virtual void visit(const GnuDebugLinkSection &Sec) = 0;
+};
+
+class SectionWriter : public SectionVisitor {
+protected:
+  FileOutputBuffer &Out;
+
+public:
+  virtual ~SectionWriter(){};
+
+  void visit(const Section &Sec) override;
+  void visit(const OwnedDataSection &Sec) override;
+  void visit(const StringTableSection &Sec) override;
+  void visit(const DynamicRelocationSection &Sec) override;
+  virtual void visit(const SymbolTableSection &Sec) override = 0;
+  virtual void visit(const RelocationSection &Sec) override = 0;
+  virtual void visit(const GnuDebugLinkSection &Sec) override = 0;
+
+  SectionWriter(FileOutputBuffer &Buf) : Out(Buf) {}
+};
+
+template <class ELFT> class ELFSectionWriter : public SectionWriter {
+private:
+  using Elf_Word = typename ELFT::Word;
+  using Elf_Rel = typename ELFT::Rel;
+  using Elf_Rela = typename ELFT::Rela;
+
+public:
+  virtual ~ELFSectionWriter() {}
+  void visit(const SymbolTableSection &Sec) override;
+  void visit(const RelocationSection &Sec) override;
+  void visit(const GnuDebugLinkSection &Sec) override;
+
+  ELFSectionWriter(FileOutputBuffer &Buf) : SectionWriter(Buf) {}
+};
+
+#define MAKE_SEC_WRITER_FRIEND                                                 \
+  friend class SectionWriter;                                                  \
+  template <class ELFT> friend class ELFSectionWriter;
+
+class BinarySectionWriter : public SectionWriter {
+public:
+  virtual ~BinarySectionWriter() {}
+
+  void visit(const SymbolTableSection &Sec) override;
+  void visit(const RelocationSection &Sec) override;
+  void visit(const GnuDebugLinkSection &Sec) override;
+  BinarySectionWriter(FileOutputBuffer &Buf) : SectionWriter(Buf) {}
+};
+
+class Writer {
+protected:
+  StringRef File;
+  Object &Obj;
+  std::unique_ptr<FileOutputBuffer> BufPtr;
+
+  void createBuffer(uint64_t Size);
+
+public:
+  virtual ~Writer();
+
+  virtual void finalize() = 0;
+  virtual void write() = 0;
+
+  Writer(StringRef File, Object &Obj) : File(File), Obj(Obj) {}
+};
+
+template <class ELFT> class ELFWriter : public Writer {
+private:
+  using Elf_Shdr = typename ELFT::Shdr;
+  using Elf_Phdr = typename ELFT::Phdr;
+  using Elf_Ehdr = typename ELFT::Ehdr;
+
+  void writeEhdr();
+  void writePhdr(const Segment &Seg);
+  void writeShdr(const SectionBase &Sec);
+
+  void writePhdrs();
+  void writeShdrs();
+  void writeSectionData();
+
+  void assignOffsets();
+
+  std::unique_ptr<ELFSectionWriter<ELFT>> SecWriter;
+
+  size_t totalSize() const;
+
+public:
+  virtual ~ELFWriter() {}
+  bool WriteSectionHeaders = true;
+
+  void finalize() override;
+  void write() override;
+  ELFWriter(StringRef File, Object &Obj, bool WSH)
+      : Writer(File, Obj), WriteSectionHeaders(WSH) {}
+};
+
+class BinaryWriter : public Writer {
+private:
+  std::unique_ptr<BinarySectionWriter> SecWriter;
+
+  uint64_t TotalSize;
+
+public:
+  ~BinaryWriter() {}
+  void finalize() override;
+  void write() override;
+  BinaryWriter(StringRef File, Object &Obj) : Writer(File, Obj) {}
+};
+
 class SectionBase {
 public:
   StringRef Name;
   Segment *ParentSegment = nullptr;
   uint64_t HeaderOffset;
   uint64_t OriginalOffset;
   uint32_t Index;
 
   uint64_t Addr = 0;
   uint64_t Align = 1;
   uint32_t EntrySize = 0;
   uint64_t Flags = 0;
   uint64_t Info = 0;
   uint64_t Link = ELF::SHN_UNDEF;
   uint64_t NameIndex = 0;
   uint64_t Offset = 0;
   uint64_t Size = 0;
   uint64_t Type = ELF::SHT_NULL;
 
   virtual ~SectionBase() = default;
 
   virtual void initialize(SectionTableRef SecTable);
   virtual void finalize();
   virtual void removeSectionReferences(const SectionBase *Sec);
-  template <class ELFT> void writeHeader(FileOutputBuffer &Out) const;
-  virtual void writeSection(FileOutputBuffer &Out) const = 0;
+  virtual void accept(SectionVisitor &Visitor) const = 0;
 };
 
 class Segment {
 private:
   struct SectionCompare {
     bool operator()(const SectionBase *Lhs, const SectionBase *Rhs) const {
       // Some sections might have the same address if one of them is empty. To
       // fix this we can use the lexicographic ordering on ->Addr and the
public:
   const SectionBase *firstSection() const {
     if (!Sections.empty())
       return *Sections.begin();
     return nullptr;
   }
 
   void removeSection(const SectionBase *Sec) { Sections.erase(Sec); }
   void addSection(const SectionBase *Sec) { Sections.insert(Sec); }
-  template <class ELFT> void writeHeader(FileOutputBuffer &Out) const;
-  void writeSegment(FileOutputBuffer &Out) const;
 };
 
 class Section : public SectionBase {
+  MAKE_SEC_WRITER_FRIEND
+
 private:
   ArrayRef<uint8_t> Contents;
 
 public:
   Section(ArrayRef<uint8_t> Data) : Contents(Data) {}
 
-  void writeSection(FileOutputBuffer &Out) const override;
+  void accept(SectionVisitor &Visitor) const override;
 };
 
 class OwnedDataSection : public SectionBase {
+  MAKE_SEC_WRITER_FRIEND
+
 private:
   std::vector<uint8_t> Data;
 
 public:
   OwnedDataSection(StringRef SecName, ArrayRef<uint8_t> Data)
       : Data(std::begin(Data), std::end(Data)) {
     Name = SecName;
     Type = ELF::SHT_PROGBITS;
     Size = Data.size();
+    OriginalOffset = std::numeric_limits<uint64_t>::max();
   }
-  void writeSection(FileOutputBuffer &Out) const override;
+
+  void accept(SectionVisitor &Sec) const override;
 };
 
 // There are two types of string tables that can exist, dynamic and not dynamic.
 // In the dynamic case the string table is allocated. Changing a dynamic string
 // table would mean altering virtual addresses and thus the memory image. So
 // dynamic string tables should not have an interface to modify them or
 // reconstruct them. This type lets us reconstruct a string table. To avoid
 // this class being used for dynamic string tables (which has happened) the
 // classof method checks that the particular instance is not allocated. This
 // then agrees with the makeSection method used to construct most sections.
 class StringTableSection : public SectionBase {
+  MAKE_SEC_WRITER_FRIEND
+
 private:
   StringTableBuilder StrTabBuilder;
 
 public:
   StringTableSection() : StrTabBuilder(StringTableBuilder::ELF) {
     Type = ELF::SHT_STRTAB;
   }
 
   void addString(StringRef Name);
   uint32_t findIndex(StringRef Name) const;
   void finalize() override;
-  void writeSection(FileOutputBuffer &Out) const override;
+  void accept(SectionVisitor &Visitor) const override;
 
   static bool classof(const SectionBase *S) {
     if (S->Flags & ELF::SHF_ALLOC)
       return false;
     return S->Type == ELF::SHT_STRTAB;
   }
 };
 
struct Symbol {
   uint8_t Type;
   uint64_t Value;
   uint8_t Visibility;
 
   uint16_t getShndx() const;
 };
 
 class SymbolTableSection : public SectionBase {
+  MAKE_SEC_WRITER_FRIEND
+
 protected:
   std::vector<std::unique_ptr<Symbol>> Symbols;
   StringTableSection *SymbolNames = nullptr;
 
   using SymPtr = std::unique_ptr<Symbol>;
 
 public:
   void setStrTab(StringTableSection *StrTab) { SymbolNames = StrTab; }
   void addSymbol(StringRef Name, uint8_t Bind, uint8_t Type,
                  SectionBase *DefinedIn, uint64_t Value, uint8_t Visibility,
                  uint16_t Shndx, uint64_t Sz);
   void addSymbolNames();
   const SectionBase *getStrTab() const { return SymbolNames; }
   const Symbol *getSymbolByIndex(uint32_t Index) const;
   void removeSectionReferences(const SectionBase *Sec) override;
   void localize(std::function<bool(const Symbol &)> ToLocalize);
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
+  void accept(SectionVisitor &Visitor) const override;
 
   static bool classof(const SectionBase *S) {
     return S->Type == ELF::SHT_SYMTAB;
   }
 };
 
-// Only writeSection depends on the ELF type so we implement it in a subclass.
-template <class ELFT> class SymbolTableSectionImpl : public SymbolTableSection {
-  void writeSection(FileOutputBuffer &Out) const override;
-};
-
 struct Relocation {
   const Symbol *RelocSymbol = nullptr;
   uint64_t Offset;
   uint64_t Addend;
   uint32_t Type;
 };
 
 // All relocation sections denote relocations to apply to another section.

 
 public:
   void setSymTab(SymTabType *StrTab) { Symbols = StrTab; }
   void removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
 };
 
-template <class ELFT>
 class RelocationSection
     : public RelocSectionWithSymtabBase<SymbolTableSection> {
-private:
-  using Elf_Rel = typename ELFT::Rel;
-  using Elf_Rela = typename ELFT::Rela;
+  MAKE_SEC_WRITER_FRIEND
 
+private:
   std::vector<Relocation> Relocations;
 
-  template <class T> void writeRel(T *Buf) const;
-
 public:
   void addRelocation(Relocation Rel) { Relocations.push_back(Rel); }
-  void writeSection(FileOutputBuffer &Out) const override;
+  void accept(SectionVisitor &Visitor) const override;
 
   static bool classof(const SectionBase *S) {
     if (S->Flags & ELF::SHF_ALLOC)
       return false;
     return S->Type == ELF::SHT_REL || S->Type == ELF::SHT_RELA;
   }
 };
 
public:
 
   static bool classof(const SectionBase *S) {
     return S->Type == ELF::SHT_DYNAMIC;
   }
 };
 
 class DynamicRelocationSection
     : public RelocSectionWithSymtabBase<DynamicSymbolTableSection> {
+  MAKE_SEC_WRITER_FRIEND
+
 private:
   ArrayRef<uint8_t> Contents;
 
 public:
   DynamicRelocationSection(ArrayRef<uint8_t> Data) : Contents(Data) {}
 
-  void writeSection(FileOutputBuffer &Out) const override;
+  void accept(SectionVisitor &) const override;
 
   static bool classof(const SectionBase *S) {
     if (!(S->Flags & ELF::SHF_ALLOC))
       return false;
     return S->Type == ELF::SHT_REL || S->Type == ELF::SHT_RELA;
   }
 };
 
-template <class ELFT> class GnuDebugLinkSection : public SectionBase {
+class GnuDebugLinkSection : public SectionBase {
+  MAKE_SEC_WRITER_FRIEND
+
 private:
-  // Elf_Word is 4-bytes on every format but has the same endianess as the elf
-  // type ELFT. We'll need to write the CRC32 out in the proper endianess so
-  // we'll make sure to use this type.
-  using Elf_Word = typename ELFT::Word;
 
   StringRef FileName;
   uint32_t CRC32;
 
   void init(StringRef File, StringRef Data);
 
 public:
   // If we add this section from an external source we can use this ctor.
   GnuDebugLinkSection(StringRef File);
-  void writeSection(FileOutputBuffer &Out) const override;
+  void accept(SectionVisitor &Visitor) const override;
 };
 
-template <class ELFT> class Object {
-private:
-  using SecPtr = std::unique_ptr<SectionBase>;
-  using SegPtr = std::unique_ptr<Segment>;
+class Reader {
+public:
+  virtual ~Reader();
+  virtual std::unique_ptr<Object> create() const = 0;
+};
+
+using object::OwningBinary;
+using object::Binary;
+using object::ELFFile;
+using object::ELFObjectFile;
 
+template <class ELFT> class ELFBuilder {
+private:
   using Elf_Shdr = typename ELFT::Shdr;
-  using Elf_Ehdr = typename ELFT::Ehdr;
-  using Elf_Phdr = typename ELFT::Phdr;
 
-  void initSymbolTable(const object::ELFFile<ELFT> &ElfFile,
-                       SymbolTableSection *SymTab, SectionTableRef SecTable);
-  SecPtr makeSection(const object::ELFFile<ELFT> &ElfFile,
-                     const Elf_Shdr &Shdr);
-  void readProgramHeaders(const object::ELFFile<ELFT> &ElfFile);
-  SectionTableRef readSectionHeaders(const object::ELFFile<ELFT> &ElfFile);
+  const ELFFile<ELFT> &ElfFile;
+  Object &Obj;
 
-protected:
-  StringTableSection *SectionNames = nullptr;
-  SymbolTableSection *SymbolTable = nullptr;
-  std::vector<SecPtr> Sections;
-  std::vector<SegPtr> Segments;
-
-  void writeHeader(FileOutputBuffer &Out) const;
-  void writeProgramHeaders(FileOutputBuffer &Out) const;
-  void writeSectionData(FileOutputBuffer &Out) const;
-  void writeSectionHeaders(FileOutputBuffer &Out) const;
+  void readProgramHeaders();
+  void initSymbolTable(SymbolTableSection *SymTab);
+  void readSectionHeaders();
+  SectionBase &makeSection(const Elf_Shdr &Shdr);
 
 public:
-  uint8_t Ident[16];
-  uint64_t Entry;
-  uint64_t SHOffset;
-  uint32_t Type;
-  uint32_t Machine;
-  uint32_t Version;
-  uint32_t Flags;
-  bool WriteSectionHeaders = true;
-
-  Object(const object::ELFObjectFile<ELFT> &Obj);
-  virtual ~Object() = default;
+  ELFBuilder(const ELFObjectFile<ELFT> &ElfObj, Object &Obj)
+      : ElfFile(*ElfObj.getELFFile()), Obj(Obj) {}
 
-  SymbolTableSection *getSymTab() const { return SymbolTable; }
-  const SectionBase *getSectionHeaderStrTab() const { return SectionNames; }
-  void removeSections(std::function<bool(const SectionBase &)> ToRemove);
-  void addSection(StringRef SecName, ArrayRef<uint8_t> Data);
-  void addGnuDebugLink(StringRef File);
-  virtual size_t totalSize() const = 0;
-  virtual void finalize() = 0;
-  virtual void write(FileOutputBuffer &Out) const = 0;
+  void build();
 };
 
-template <class ELFT> class ELFObject : public Object<ELFT> {
+class ELFReader : public Reader {
 private:
-  using SecPtr = std::unique_ptr<SectionBase>;
-  using SegPtr = std::unique_ptr<Segment>;
-
-  using Elf_Shdr = typename ELFT::Shdr;
-  using Elf_Ehdr = typename ELFT::Ehdr;
-  using Elf_Phdr = typename ELFT::Phdr;
-
-  void sortSections();
-  void assignOffsets();
+  std::unique_ptr<Binary> Binary;
+  std::shared_ptr<MemoryBuffer> Data;
 
 public:
-  ELFObject(const object::ELFObjectFile<ELFT> &Obj) : Object<ELFT>(Obj) {}
-
-  void finalize() override;
-  size_t totalSize() const override;
-  void write(FileOutputBuffer &Out) const override;
+  ElfType getElfType() const;
+  std::unique_ptr<Object> create() const override;
+  ELFReader(StringRef File);
 };
 
-template <class ELFT> class BinaryObject : public Object<ELFT> {
+class Object {
 private:
   using SecPtr = std::unique_ptr<SectionBase>;
   using SegPtr = std::unique_ptr<Segment>;
 
-  uint64_t TotalSize;
+  std::shared_ptr<MemoryBuffer> OwnedData;
+  std::vector<SecPtr> Sections;
+  std::vector<SegPtr> Segments;
 
 public:
-  BinaryObject(const object::ELFObjectFile<ELFT> &Obj) : Object<ELFT>(Obj) {}
+  template <class T>
+  using Range = iterator_range<
+      pointee_iterator<typename std::vector<std::unique_ptr<T>>::iterator>>;
 
-  void finalize() override;
-  size_t totalSize() const override;
-  void write(FileOutputBuffer &Out) const override;
+  template <class T>
+  using ConstRange = iterator_range<pointee_iterator<
+      typename std::vector<std::unique_ptr<T>>::const_iterator>>;
+
+  uint8_t Ident[16];
+  uint64_t Entry;
+  uint64_t SHOffset;
+  uint32_t Type;
+  uint32_t Machine;
+  uint32_t Version;
+  uint32_t Flags;
+
+  StringTableSection *SectionNames = nullptr;
+  SymbolTableSection *SymbolTable = nullptr;
+
+  Object(std::shared_ptr<MemoryBuffer> Data) : OwnedData(Data) {}
+  virtual ~Object() = default;
+
+  void sortSections();
+  SectionTableRef sections() { return SectionTableRef(Sections); }
+  ConstRange<SectionBase> sections() const {
+    return make_pointee_range(Sections);
+  }
+  Range<Segment> segments() { return make_pointee_range(Segments); }
+  ConstRange<Segment> segments() const { return make_pointee_range(Segments); }
+
+  void removeSections(std::function<bool(const SectionBase &)> ToRemove);
+  template <class T, class... Ts> T &addSection(Ts &&... Args) {
+    auto Sec = llvm::make_unique<T>(std::forward<Ts>(Args)...);
+    auto Ptr = Sec.get();
+    Sections.emplace_back(std::move(Sec));
+    return *Ptr;
+  }
+  Segment &addSegment(ArrayRef<uint8_t> Data) {
+    Segments.emplace_back(llvm::make_unique<Segment>(Data));
+    return *Segments.back();
+  }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_OBJCOPY_OBJECT_H

llvm/trunk/tools/llvm-objcopy/Object.cpp

 #include <iterator>
 #include <utility>
 #include <vector>
 
 using namespace llvm;
 using namespace object;
 using namespace ELF;
 
-template <class ELFT> void Segment::writeHeader(FileOutputBuffer &Out) const {
+template <class ELFT> void ELFWriter<ELFT>::writePhdr(const Segment &Seg) {
   using Elf_Ehdr = typename ELFT::Ehdr;
   using Elf_Phdr = typename ELFT::Phdr;
 
-  uint8_t *Buf = Out.getBufferStart();
-  Buf += sizeof(Elf_Ehdr) + Index * sizeof(Elf_Phdr);
+  uint8_t *Buf = BufPtr->getBufferStart();
+  Buf += sizeof(Elf_Ehdr) + Seg.Index * sizeof(Elf_Phdr);
   Elf_Phdr &Phdr = *reinterpret_cast<Elf_Phdr *>(Buf);
-  Phdr.p_type = Type;
-  Phdr.p_flags = Flags;
-  Phdr.p_offset = Offset;
-  Phdr.p_vaddr = VAddr;
-  Phdr.p_paddr = PAddr;
-  Phdr.p_filesz = FileSize;
-  Phdr.p_memsz = MemSize;
-  Phdr.p_align = Align;
-}
-
-void Segment::writeSegment(FileOutputBuffer &Out) const {
-  uint8_t *Buf = Out.getBufferStart() + Offset;
-  // We want to maintain segments' interstitial data and contents exactly.
-  // This lets us just copy segments directly.
-  std::copy(std::begin(Contents), std::end(Contents), Buf);
+  Phdr.p_type = Seg.Type;
+  Phdr.p_flags = Seg.Flags;
+  Phdr.p_offset = Seg.Offset;
+  Phdr.p_vaddr = Seg.VAddr;
+  Phdr.p_paddr = Seg.PAddr;
+  Phdr.p_filesz = Seg.FileSize;
+  Phdr.p_memsz = Seg.MemSize;
+  Phdr.p_align = Seg.Align;
 }
 
 void SectionBase::removeSectionReferences(const SectionBase *Sec) {}
 void SectionBase::initialize(SectionTableRef SecTable) {}
 void SectionBase::finalize() {}
 
-template <class ELFT>
-void SectionBase::writeHeader(FileOutputBuffer &Out) const {
-  uint8_t *Buf = Out.getBufferStart();
-  Buf += HeaderOffset;
+template <class ELFT> void ELFWriter<ELFT>::writeShdr(const SectionBase &Sec) {
+  uint8_t *Buf = BufPtr->getBufferStart();
+  Buf += Sec.HeaderOffset;
   typename ELFT::Shdr &Shdr = *reinterpret_cast<typename ELFT::Shdr *>(Buf);
-  Shdr.sh_name = NameIndex;
-  Shdr.sh_type = Type;
-  Shdr.sh_flags = Flags;
-  Shdr.sh_addr = Addr;
-  Shdr.sh_offset = Offset;
-  Shdr.sh_size = Size;
-  Shdr.sh_link = Link;
-  Shdr.sh_info = Info;
-  Shdr.sh_addralign = Align;
-  Shdr.sh_entsize = EntrySize;
+  Shdr.sh_name = Sec.NameIndex;
+  Shdr.sh_type = Sec.Type;
+  Shdr.sh_flags = Sec.Flags;
+  Shdr.sh_addr = Sec.Addr;
+  Shdr.sh_offset = Sec.Offset;
+  Shdr.sh_size = Sec.Size;
+  Shdr.sh_link = Sec.Link;
+  Shdr.sh_info = Sec.Info;
+  Shdr.sh_addralign = Sec.Align;
+  Shdr.sh_entsize = Sec.EntrySize;
+}
+
+SectionVisitor::~SectionVisitor() {}
+
+void BinarySectionWriter::visit(const SymbolTableSection &Sec) {
+  error("Cannot write symbol table '" + Sec.Name + "' out to binary");
 }
 
-void Section::writeSection(FileOutputBuffer &Out) const {
-  if (Type == SHT_NOBITS)
+void BinarySectionWriter::visit(const RelocationSection &Sec) {
+  error("Cannot write relocation section '" + Sec.Name + "' out to binary");
+}
+
+void BinarySectionWriter::visit(const GnuDebugLinkSection &Sec) {
+  error("Cannot write '.gnu_debuglink' out to binary");
+}
+
+void SectionWriter::visit(const Section &Sec) {
+  if (Sec.Type == SHT_NOBITS)
     return;
-  uint8_t *Buf = Out.getBufferStart() + Offset;
-  std::copy(std::begin(Contents), std::end(Contents), Buf);
+  uint8_t *Buf = Out.getBufferStart() + Sec.Offset;
+  std::copy(std::begin(Sec.Contents), std::end(Sec.Contents), Buf);
 }
 
-void OwnedDataSection::writeSection(FileOutputBuffer &Out) const {
-  uint8_t *Buf = Out.getBufferStart() + Offset;
-  std::copy(std::begin(Data), std::end(Data), Buf);
+void Section::accept(SectionVisitor &Visitor) const { Visitor.visit(*this); }
+
+void SectionWriter::visit(const OwnedDataSection &Sec) {
+  uint8_t *Buf = Out.getBufferStart() + Sec.Offset;
+  std::copy(std::begin(Sec.Data), std::end(Sec.Data), Buf);
+}
+
+void OwnedDataSection::accept(SectionVisitor &Visitor) const {
+  Visitor.visit(*this);
 }
 
 void StringTableSection::addString(StringRef Name) {
   StrTabBuilder.add(Name);
   Size = StrTabBuilder.getSize();
 }
 
 uint32_t StringTableSection::findIndex(StringRef Name) const {
   return StrTabBuilder.getOffset(Name);
 }
 
 void StringTableSection::finalize() { StrTabBuilder.finalize(); }
 
-void StringTableSection::writeSection(FileOutputBuffer &Out) const {
-  StrTabBuilder.write(Out.getBufferStart() + Offset);
+void SectionWriter::visit(const StringTableSection &Sec) {
+  Sec.StrTabBuilder.write(Out.getBufferStart() + Sec.Offset);
+}
+
+void StringTableSection::accept(SectionVisitor &Visitor) const {
+  Visitor.visit(*this);
 }
 
 static bool isValidReservedSectionIndex(uint16_t Index, uint16_t Machine) {
   switch (Index) {
   case SHN_ABS:
   case SHN_COMMON:
     return true;
   }

 
 const Symbol *SymbolTableSection::getSymbolByIndex(uint32_t Index) const {
   if (Symbols.size() <= Index)
     error("Invalid symbol index: " + Twine(Index));
   return Symbols[Index].get();
 }
 
 template <class ELFT>
-void SymbolTableSectionImpl<ELFT>::writeSection(FileOutputBuffer &Out) const {
+void ELFSectionWriter<ELFT>::visit(const SymbolTableSection &Sec) {
   uint8_t *Buf = Out.getBufferStart();
-  Buf += Offset;
+  Buf += Sec.Offset;
   typename ELFT::Sym *Sym = reinterpret_cast<typename ELFT::Sym *>(Buf);
   // Loop though symbols setting each entry of the symbol table.
-  for (auto &Symbol : Symbols) {
+  for (auto &Symbol : Sec.Symbols) {
     Sym->st_name = Symbol->NameIndex;
     Sym->st_value = Symbol->Value;
     Sym->st_size = Symbol->Size;
     Sym->st_other = Symbol->Visibility;
     Sym->setBinding(Symbol->Binding);
     Sym->setType(Symbol->Type);
     Sym->st_shndx = Symbol->getShndx();
     ++Sym;
   }
 }
 
+void SymbolTableSection::accept(SectionVisitor &Visitor) const {
+  Visitor.visit(*this);
+}
+
 template <class SymTabType>
 void RelocSectionWithSymtabBase<SymTabType>::removeSectionReferences(
     const SectionBase *Sec) {
   if (Symbols == Sec) {
     error("Symbol table " + Symbols->Name + " cannot be removed because it is "
                                             "referenced by the relocation "
                                             "section " +
           this->Name);

 template <class ELFT>
 void setAddend(Elf_Rel_Impl<ELFT, false> &Rel, uint64_t Addend) {}
 
 template <class ELFT>
 void setAddend(Elf_Rel_Impl<ELFT, true> &Rela, uint64_t Addend) {
   Rela.r_addend = Addend;
 }
 
-template <class ELFT>
-template <class T>
-void RelocationSection<ELFT>::writeRel(T *Buf) const {
+template <class RelRange, class T>
+void writeRel(const RelRange &Relocations, T *Buf) {
   for (const auto &Reloc : Relocations) {
     Buf->r_offset = Reloc.Offset;
     setAddend(*Buf, Reloc.Addend);
     Buf->setSymbolAndType(Reloc.RelocSymbol->Index, Reloc.Type, false);
     ++Buf;
   }
 }
 
 template <class ELFT>
-void RelocationSection<ELFT>::writeSection(FileOutputBuffer &Out) const {
-  uint8_t *Buf = Out.getBufferStart() + Offset;
-  if (Type == SHT_REL)
-    writeRel(reinterpret_cast<Elf_Rel *>(Buf));
+void ELFSectionWriter<ELFT>::visit(const RelocationSection &Sec) {
+  uint8_t *Buf = Out.getBufferStart() + Sec.Offset;
+  if (Sec.Type == SHT_REL)
+    writeRel(Sec.Relocations, reinterpret_cast<Elf_Rel *>(Buf));
   else
-    writeRel(reinterpret_cast<Elf_Rela *>(Buf));
+    writeRel(Sec.Relocations, reinterpret_cast<Elf_Rela *>(Buf));
+}
+
+void RelocationSection::accept(SectionVisitor &Visitor) const {
+  Visitor.visit(*this);
 }
 
-void DynamicRelocationSection::writeSection(FileOutputBuffer &Out) const {
-  std::copy(std::begin(Contents), std::end(Contents),
-            Out.getBufferStart() + Offset);
+void SectionWriter::visit(const DynamicRelocationSection &Sec) {
+  std::copy(std::begin(Sec.Contents), std::end(Sec.Contents),
+            Out.getBufferStart() + Sec.Offset);
+}
+
+void DynamicRelocationSection::accept(SectionVisitor &Visitor) const {
+  Visitor.visit(*this);
 }
 
 void SectionWithStrTab::removeSectionReferences(const SectionBase *Sec) {
   if (StrTab == Sec) {
     error("String table " + StrTab->Name + " cannot be removed because it is "
                                            "referenced by the section " +
           this->Name);
   }
if (StrTab->Type != SHT_STRTAB) {
     error("Link field value " + Twine(Link) + " in section " + Name +
           " is not a string table");
   }
   setStrTab(StrTab);
 }
 
 void SectionWithStrTab::finalize() { this->Link = StrTab->Index; }
 
-template <class ELFT>
-void GnuDebugLinkSection<ELFT>::init(StringRef File, StringRef Data) {
+void GnuDebugLinkSection::init(StringRef File, StringRef Data) {
   FileName = sys::path::stem(File);
   // The format for the .gnu_debuglink starts with the stemmed file name and is
   // followed by a null terminator and then the CRC32 of the file. The CRC32
   // should be 4 byte aligned. So we add the FileName size, a 1 for the null
   // byte, and then finally push the size to alignment and add 4.
   Size = alignTo(FileName.size() + 1, 4) + 4;
   // The CRC32 will only be aligned if we align the whole section.
   Align = 4;
   Type = ELF::SHT_PROGBITS;
   Name = ".gnu_debuglink";
   // For sections not found in segments, OriginalOffset is only used to
   // establish the order that sections should go in. By using the maximum
   // possible offset we cause this section to wind up at the end.
   OriginalOffset = std::numeric_limits<uint64_t>::max();
   JamCRC crc;
   crc.update(ArrayRef<char>(Data.data(), Data.size()));
   // The CRC32 value needs to be complemented because the JamCRC dosn't
   // finalize the CRC32 value. It also dosn't negate the initial CRC32 value
   // but it starts by default at 0xFFFFFFFF which is the complement of zero.
   CRC32 = ~crc.getCRC();
 }
 
-template <class ELFT>
-GnuDebugLinkSection<ELFT>::GnuDebugLinkSection(StringRef File)
-    : FileName(File) {
+GnuDebugLinkSection::GnuDebugLinkSection(StringRef File) : FileName(File) {
   // Read in the file to compute the CRC of it.
   auto DebugOrErr = MemoryBuffer::getFile(File);
   if (!DebugOrErr)
     error("'" + File + "': " + DebugOrErr.getError().message());
   auto Debug = std::move(*DebugOrErr);
   init(File, Debug->getBuffer());
 }
 
 template <class ELFT>
-void GnuDebugLinkSection<ELFT>::writeSection(FileOutputBuffer &Out) const {
-  auto Buf = Out.getBufferStart() + Offset;
+void ELFSectionWriter<ELFT>::visit(const GnuDebugLinkSection &Sec) {
+  auto Buf = Out.getBufferStart() + Sec.Offset;
   char *File = reinterpret_cast<char *>(Buf);
-  Elf_Word *CRC = reinterpret_cast<Elf_Word *>(Buf + Size - sizeof(Elf_Word));
-  *CRC = CRC32;
-  std::copy(std::begin(FileName), std::end(FileName), File);
+  Elf_Word *CRC =
+      reinterpret_cast<Elf_Word *>(Buf + Sec.Size - sizeof(Elf_Word));
+  *CRC = Sec.CRC32;
+  std::copy(std::begin(Sec.FileName), std::end(Sec.FileName), File);
+}
+
+void GnuDebugLinkSection::accept(SectionVisitor &Visitor) const {
+  Visitor.visit(*this);
 }
 
 // Returns true IFF a section is wholly inside the range of a segment
 static bool sectionWithinSegment(const SectionBase &Section,
                                  const Segment &Segment) {
   // If a section is empty it should be treated like it has a size of 1. This is
   // to clarify the case when an empty section lies on a boundary between two
   // segments and ensures that the section "belongs" to the second segment and

 static bool compareSegmentsByPAddr(const Segment *A, const Segment *B) {
   if (A->PAddr < B->PAddr)
     return true;
   if (A->PAddr > B->PAddr)
     return false;
   return A->Index < B->Index;
 }
 
-template <class ELFT>
-void Object<ELFT>::readProgramHeaders(const ELFFile<ELFT> &ElfFile) {
+template <class ELFT> void ELFBuilder<ELFT>::readProgramHeaders() {
   uint32_t Index = 0;
   for (const auto &Phdr : unwrapOrError(ElfFile.program_headers())) {
     ArrayRef<uint8_t> Data{ElfFile.base() + Phdr.p_offset,
                            (size_t)Phdr.p_filesz};
-    Segments.emplace_back(llvm::make_unique<Segment>(Data));
-    Segment &Seg = *Segments.back();
+    Segment &Seg = Obj.addSegment(Data);
     Seg.Type = Phdr.p_type;
     Seg.Flags = Phdr.p_flags;
     Seg.OriginalOffset = Phdr.p_offset;
     Seg.Offset = Phdr.p_offset;
     Seg.VAddr = Phdr.p_vaddr;
     Seg.PAddr = Phdr.p_paddr;
     Seg.FileSize = Phdr.p_filesz;
     Seg.MemSize = Phdr.p_memsz;
     Seg.Align = Phdr.p_align;
     Seg.Index = Index++;
-    for (auto &Section : Sections) {
-      if (sectionWithinSegment(*Section, Seg)) {
-        Seg.addSection(&*Section);
-        if (!Section->ParentSegment ||
-            Section->ParentSegment->Offset > Seg.Offset) {
-          Section->ParentSegment = &Seg;
+    for (auto &Section : Obj.sections()) {
+      if (sectionWithinSegment(Section, Seg)) {
+        Seg.addSection(&Section);
+        if (!Section.ParentSegment ||
+            Section.ParentSegment->Offset > Seg.Offset) {
+          Section.ParentSegment = &Seg;
         }
       }
     }
   }
   // Now we do an O(n^2) loop through the segments in order to match up
   // segments.
-  for (auto &Child : Segments) {
-    for (auto &Parent : Segments) {
+  for (auto &Child : Obj.segments()) {
+    for (auto &Parent : Obj.segments()) {
       // Every segment will overlap with itself but we don't want a segment to
       // be it's own parent so we avoid that situation.
-      if (&Child != &Parent && segmentOverlapsSegment(*Child, *Parent)) {
+      if (&Child != &Parent && segmentOverlapsSegment(Child, Parent)) {
         // We want a canonical "most parental" segment but this requires
         // inspecting the ParentSegment.
-        if (compareSegmentsByOffset(Parent.get(), Child.get()))
-          if (Child->ParentSegment == nullptr ||
-              compareSegmentsByOffset(Parent.get(), Child->ParentSegment)) {
-            Child->ParentSegment = Parent.get();
+        if (compareSegmentsByOffset(&Parent, &Child))
+          if (Child.ParentSegment == nullptr ||
+              compareSegmentsByOffset(&Parent, Child.ParentSegment)) {
+            Child.ParentSegment = &Parent;
           }
       }
     }
   }
 }
 
 template <class ELFT>
-void Object<ELFT>::initSymbolTable(const object::ELFFile<ELFT> &ElfFile,
-                                   SymbolTableSection *SymTab,
-                                   SectionTableRef SecTable) {
+void ELFBuilder<ELFT>::initSymbolTable(SymbolTableSection *SymTab) {
   const Elf_Shdr &Shdr = *unwrapOrError(ElfFile.getSection(SymTab->Index));
   StringRef StrTabData = unwrapOrError(ElfFile.getStringTableForSymtab(Shdr));
 
   for (const auto &Sym : unwrapOrError(ElfFile.symbols(&Shdr))) {
     SectionBase *DefSection = nullptr;
     StringRef Name = unwrapOrError(Sym.getName(StrTabData));
 
     if (Sym.st_shndx >= SHN_LORESERVE) {
-      if (!isValidReservedSectionIndex(Sym.st_shndx, Machine)) {
+      if (!isValidReservedSectionIndex(Sym.st_shndx, Obj.Machine)) {
         error(
             "Symbol '" + Name +
             "' has unsupported value greater than or equal to SHN_LORESERVE: " +
             Twine(Sym.st_shndx));
       }
     } else if (Sym.st_shndx != SHN_UNDEF) {
-      DefSection = SecTable.getSection(
+      DefSection = Obj.sections().getSection(
           Sym.st_shndx,
           "Symbol '" + Name + "' is defined in invalid section with index " +
               Twine(Sym.st_shndx));
     }
 
     SymTab->addSymbol(Name, Sym.getBinding(), Sym.getType(), DefSection,
                       Sym.getValue(), Sym.st_other, Sym.st_shndx, Sym.st_size);
   }
 }
 
 template <class ELFT>
 static void getAddend(uint64_t &ToSet, const Elf_Rel_Impl<ELFT, false> &Rel) {}
 
 template <class ELFT>
 static void getAddend(uint64_t &ToSet, const Elf_Rel_Impl<ELFT, true> &Rela) {
   ToSet = Rela.r_addend;
 }
 
-template <class ELFT, class T>
-void initRelocations(RelocationSection<ELFT> *Relocs,
-                     SymbolTableSection *SymbolTable, T RelRange) {
+template <class T>
+void initRelocations(RelocationSection *Relocs, SymbolTableSection *SymbolTable,
+                     T RelRange) {
   for (const auto &Rel : RelRange) {
     Relocation ToAdd;
     ToAdd.Offset = Rel.r_offset;
     getAddend(ToAdd.Addend, Rel);
     ToAdd.Type = Rel.getType(false);
     ToAdd.RelocSymbol = SymbolTable->getSymbolByIndex(Rel.getSymbol(false));
     Relocs->addRelocation(ToAdd);
   }

 T *SectionTableRef::getSectionOfType(uint16_t Index, Twine IndexErrMsg,
                                      Twine TypeErrMsg) {
   if (T *Sec = dyn_cast<T>(getSection(Index, IndexErrMsg)))
     return Sec;
   error(TypeErrMsg);
 }
 
 template <class ELFT>
-std::unique_ptr<SectionBase>
-Object<ELFT>::makeSection(const object::ELFFile<ELFT> &ElfFile,
-                          const Elf_Shdr &Shdr) {
+SectionBase &ELFBuilder<ELFT>::makeSection(const Elf_Shdr &Shdr) {
   ArrayRef<uint8_t> Data;
   switch (Shdr.sh_type) {
   case SHT_REL:
   case SHT_RELA:
     if (Shdr.sh_flags & SHF_ALLOC) {
       Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
-      return llvm::make_unique<DynamicRelocationSection>(Data);
+      return Obj.addSection<DynamicRelocationSection>(Data);
     }
-    return llvm::make_unique<RelocationSection<ELFT>>();
+    return Obj.addSection<RelocationSection>();
   case SHT_STRTAB:
     // If a string table is allocated we don't want to mess with it. That would
     // mean altering the memory image. There are no special link types or
     // anything so we can just use a Section.
     if (Shdr.sh_flags & SHF_ALLOC) {
       Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
-      return llvm::make_unique<Section>(Data);
+      return Obj.addSection<Section>(Data);
     }
-    return llvm::make_unique<StringTableSection>();
+    return Obj.addSection<StringTableSection>();
   case SHT_HASH:
   case SHT_GNU_HASH:
     // Hash tables should refer to SHT_DYNSYM which we're not going to change.
     // Because of this we don't need to mess with the hash tables either.
     Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
-    return llvm::make_unique<Section>(Data);
+    return Obj.addSection<Section>(Data);
   case SHT_DYNSYM:
     Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
-    return llvm::make_unique<DynamicSymbolTableSection>(Data);
+    return Obj.addSection<DynamicSymbolTableSection>(Data);
   case SHT_DYNAMIC:
     Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
-    return llvm::make_unique<DynamicSection>(Data);
+    return Obj.addSection<DynamicSection>(Data);
   case SHT_SYMTAB: {
-    auto SymTab = llvm::make_unique<SymbolTableSectionImpl<ELFT>>();
-    SymbolTable = SymTab.get();
-    return std::move(SymTab);
+    auto &SymTab = Obj.addSection<SymbolTableSection>();
+    Obj.SymbolTable = &SymTab;
+    return SymTab;
   }
   case SHT_NOBITS:
-    return llvm::make_unique<Section>(Data);
+    return Obj.addSection<Section>(Data);
   default:
     Data = unwrapOrError(ElfFile.getSectionContents(&Shdr));
-    return llvm::make_unique<Section>(Data);
+    return Obj.addSection<Section>(Data);
   }
 }
 
-template <class ELFT>
-SectionTableRef Object<ELFT>::readSectionHeaders(const ELFFile<ELFT> &ElfFile) {
+template <class ELFT> void ELFBuilder<ELFT>::readSectionHeaders() {
   uint32_t Index = 0;
   for (const auto &Shdr : unwrapOrError(ElfFile.sections())) {
     if (Index == 0) {
       ++Index;
       continue;
     }
-    SecPtr Sec = makeSection(ElfFile, Shdr);
-    Sec->Name = unwrapOrError(ElfFile.getSectionName(&Shdr));
-    Sec->Type = Shdr.sh_type;
-    Sec->Flags = Shdr.sh_flags;
-    Sec->Addr = Shdr.sh_addr;
-    Sec->Offset = Shdr.sh_offset;
-    Sec->OriginalOffset = Shdr.sh_offset;
-    Sec->Size = Shdr.sh_size;
-    Sec->Link = Shdr.sh_link;
-    Sec->Info = Shdr.sh_info;
-    Sec->Align = Shdr.sh_addralign;
-    Sec->EntrySize = Shdr.sh_entsize;
-    Sec->Index = Index++;
-    Sections.push_back(std::move(Sec));
+    auto &Sec = makeSection(Shdr);
+    Sec.Name = unwrapOrError(ElfFile.getSectionName(&Shdr));
+    Sec.Type = Shdr.sh_type;
+    Sec.Flags = Shdr.sh_flags;
+    Sec.Addr = Shdr.sh_addr;
+    Sec.Offset = Shdr.sh_offset;
+    Sec.OriginalOffset = Shdr.sh_offset;
+    Sec.Size = Shdr.sh_size;
+    Sec.Link = Shdr.sh_link;
+    Sec.Info = Shdr.sh_info;
+    Sec.Align = Shdr.sh_addralign;
+    Sec.EntrySize = Shdr.sh_entsize;
+    Sec.Index = Index++;
   }
 
-  SectionTableRef SecTable(Sections);
-
   // Now that all of the sections have been added we can fill out some extra
   // details about symbol tables. We need the symbol table filled out before
   // any relocations.
-  if (SymbolTable) {
-    SymbolTable->initialize(SecTable);
-    initSymbolTable(ElfFile, SymbolTable, SecTable);
+  if (Obj.SymbolTable) {
+    Obj.SymbolTable->initialize(Obj.sections());
+    initSymbolTable(Obj.SymbolTable);
   }
 
   // Now that all sections and symbols have been added we can add
   // relocations that reference symbols and set the link and info fields for
   // relocation sections.
-  for (auto &Section : Sections) {
-    if (Section.get() == SymbolTable)
+  for (auto &Section : Obj.sections()) {
+    if (&Section == Obj.SymbolTable)
       continue;
-    Section->initialize(SecTable);
-    if (auto RelSec = dyn_cast<RelocationSection<ELFT>>(Section.get())) {
+    Section.initialize(Obj.sections());
+    if (auto RelSec = dyn_cast<RelocationSection>(&Section)) {
       auto Shdr = unwrapOrError(ElfFile.sections()).begin() + RelSec->Index;
       if (RelSec->Type == SHT_REL)
-        initRelocations(RelSec, SymbolTable, unwrapOrError(ElfFile.rels(Shdr)));
+        initRelocations(RelSec, Obj.SymbolTable,
+                        unwrapOrError(ElfFile.rels(Shdr)));
       else
-        initRelocations(RelSec, SymbolTable,
+        initRelocations(RelSec, Obj.SymbolTable,
                         unwrapOrError(ElfFile.relas(Shdr)));
     }
   }
-
-  return SecTable;
 }
 
-template <class ELFT> Object<ELFT>::Object(const ELFObjectFile<ELFT> &Obj) {
-  const auto &ElfFile = *Obj.getELFFile();
+template <class ELFT> void ELFBuilder<ELFT>::build() {
   const auto &Ehdr = *ElfFile.getHeader();
 
-  std::copy(Ehdr.e_ident, Ehdr.e_ident + 16, Ident);
-  Type = Ehdr.e_type;
-  Machine = Ehdr.e_machine;
-  Version = Ehdr.e_version;
-  Entry = Ehdr.e_entry;
-  Flags = Ehdr.e_flags;
+  std::copy(Ehdr.e_ident, Ehdr.e_ident + 16, Obj.Ident);
+  Obj.Type = Ehdr.e_type;
+  Obj.Machine = Ehdr.e_machine;
+  Obj.Version = Ehdr.e_version;
+  Obj.Entry = Ehdr.e_entry;
+  Obj.Flags = Ehdr.e_flags;
 
-  SectionTableRef SecTable = readSectionHeaders(ElfFile);
-  readProgramHeaders(ElfFile);
+  readSectionHeaders();
+  readProgramHeaders();
 
-  SectionNames = SecTable.getSectionOfType<StringTableSection>(
+  Obj.SectionNames =
+      Obj.sections().template getSectionOfType<StringTableSection>(
           Ehdr.e_shstrndx,
-      "e_shstrndx field value " + Twine(Ehdr.e_shstrndx) + " in elf header " +
-          " is invalid",
-      "e_shstrndx field value " + Twine(Ehdr.e_shstrndx) + " in elf header " +
-          " is not a string table");
+          "e_shstrndx field value " + Twine(Ehdr.e_shstrndx) +
+              " in elf header " + " is invalid",
+          "e_shstrndx field value " + Twine(Ehdr.e_shstrndx) +
+              " in elf header " + " is not a string table");
+}
+
+// A generic size function which computes sizes of any random access range.
+template <class R> size_t size(R &&Range) {
+  return static_cast<size_t>(std::end(Range) - std::begin(Range));
+}
+
+Writer::~Writer() {}
+
+Reader::~Reader() {}
+
+ELFReader::ELFReader(StringRef File) {
+  auto BinaryOrErr = createBinary(File);
+  if (!BinaryOrErr)
+    reportError(File, BinaryOrErr.takeError());
+  auto Bin = std::move(BinaryOrErr.get());
+  std::tie(Binary, Data) = Bin.takeBinary();
+}
+
+ElfType ELFReader::getElfType() const {
+  if (auto *o = dyn_cast<ELFObjectFile<ELF32LE>>(Binary.get()))
+    return ELFT_ELF32LE;
+  if (auto *o = dyn_cast<ELFObjectFile<ELF64LE>>(Binary.get()))
+    return ELFT_ELF64LE;
+  if (auto *o = dyn_cast<ELFObjectFile<ELF32BE>>(Binary.get()))
+    return ELFT_ELF32BE;
+  if (auto *o = dyn_cast<ELFObjectFile<ELF64BE>>(Binary.get()))
+    return ELFT_ELF64BE;
+  llvm_unreachable("Invalid ELFType");
+}
+
+std::unique_ptr<Object> ELFReader::create() const {
+  auto Obj = llvm::make_unique<Object>(Data);
+  if (auto *o = dyn_cast<ELFObjectFile<ELF32LE>>(Binary.get())) {
+    ELFBuilder<ELF32LE> Builder(*o, *Obj);
+    Builder.build();
+    return Obj;
+  } else if (auto *o = dyn_cast<ELFObjectFile<ELF64LE>>(Binary.get())) {
+    ELFBuilder<ELF64LE> Builder(*o, *Obj);
+    Builder.build();
+    return Obj;
+  } else if (auto *o = dyn_cast<ELFObjectFile<ELF32BE>>(Binary.get())) {
+    ELFBuilder<ELF32BE> Builder(*o, *Obj);
+    Builder.build();
+    return Obj;
+  } else if (auto *o = dyn_cast<ELFObjectFile<ELF64BE>>(Binary.get())) {
+    ELFBuilder<ELF64BE> Builder(*o, *Obj);
+    Builder.build();
+    return Obj;
+  }
+  error("Invalid file type");
 }
 
-template <class ELFT>
-void Object<ELFT>::writeHeader(FileOutputBuffer &Out) const {
-  uint8_t *Buf = Out.getBufferStart();
+template <class ELFT> void ELFWriter<ELFT>::writeEhdr() {
+  uint8_t *Buf = BufPtr->getBufferStart();
   Elf_Ehdr &Ehdr = *reinterpret_cast<Elf_Ehdr *>(Buf);
-  std::copy(Ident, Ident + 16, Ehdr.e_ident);
-  Ehdr.e_type = Type;
-  Ehdr.e_machine = Machine;
-  Ehdr.e_version = Version;
-  Ehdr.e_entry = Entry;
+  std::copy(Obj.Ident, Obj.Ident + 16, Ehdr.e_ident);
+  Ehdr.e_type = Obj.Type;
+  Ehdr.e_machine = Obj.Machine;
+  Ehdr.e_version = Obj.Version;
+  Ehdr.e_entry = Obj.Entry;
   Ehdr.e_phoff = sizeof(Elf_Ehdr);
-  Ehdr.e_flags = Flags;
+  Ehdr.e_flags = Obj.Flags;
   Ehdr.e_ehsize = sizeof(Elf_Ehdr);
   Ehdr.e_phentsize = sizeof(Elf_Phdr);
-  Ehdr.e_phnum = Segments.size();
+  Ehdr.e_phnum = size(Obj.segments());
   Ehdr.e_shentsize = sizeof(Elf_Shdr);
   if (WriteSectionHeaders) {
-    Ehdr.e_shoff = SHOffset;
-    Ehdr.e_shnum = Sections.size() + 1;
-    Ehdr.e_shstrndx = SectionNames->Index;
+    Ehdr.e_shoff = Obj.SHOffset;
+    Ehdr.e_shnum = size(Obj.sections()) + 1;
+    Ehdr.e_shstrndx = Obj.SectionNames->Index;
   } else {
     Ehdr.e_shoff = 0;
     Ehdr.e_shnum = 0;
     Ehdr.e_shstrndx = 0;
   }
 }
 
-template <class ELFT>
-void Object<ELFT>::writeProgramHeaders(FileOutputBuffer &Out) const {
-  for (auto &Phdr : Segments)
-    Phdr->template writeHeader<ELFT>(Out);
+template <class ELFT> void ELFWriter<ELFT>::writePhdrs() {
+  for (auto &Seg : Obj.segments())
+    writePhdr(Seg);
 }
 
-template <class ELFT>
-void Object<ELFT>::writeSectionHeaders(FileOutputBuffer &Out) const {
-  uint8_t *Buf = Out.getBufferStart() + SHOffset;
+template <class ELFT> void ELFWriter<ELFT>::writeShdrs() {
+  uint8_t *Buf = BufPtr->getBufferStart() + Obj.SHOffset;
   // This reference serves to write the dummy section header at the begining
   // of the file. It is not used for anything else
   Elf_Shdr &Shdr = *reinterpret_cast<Elf_Shdr *>(Buf);
   Shdr.sh_name = 0;
   Shdr.sh_type = SHT_NULL;
   Shdr.sh_flags = 0;
   Shdr.sh_addr = 0;
   Shdr.sh_offset = 0;
   Shdr.sh_size = 0;
   Shdr.sh_link = 0;
   Shdr.sh_info = 0;
   Shdr.sh_addralign = 0;
   Shdr.sh_entsize = 0;
 
-  for (auto &Section : Sections)
-    Section->template writeHeader<ELFT>(Out);
+  for (auto &Sec : Obj.sections())
+    writeShdr(Sec);
 }
 
-template <class ELFT>
-void Object<ELFT>::writeSectionData(FileOutputBuffer &Out) const {
-  for (auto &Section : Sections)
-    Section->writeSection(Out);
+template <class ELFT> void ELFWriter<ELFT>::writeSectionData() {
+  for (auto &Sec : Obj.sections())
+    Sec.accept(*SecWriter);
 }
 
-template <class ELFT>
-void Object<ELFT>::removeSections(
-    std::function<bool(const SectionBase &)> ToRemove) {
+void Object::removeSections(std::function<bool(const SectionBase &)> ToRemove) {
 
   auto Iter = std::stable_partition(
       std::begin(Sections), std::end(Sections), [=](const SecPtr &Sec) {
         if (ToRemove(*Sec))
           return false;
         if (auto RelSec = dyn_cast<RelocationSectionBase>(Sec.get())) {
           if (auto ToRelSec = RelSec->getSection())
             return !ToRemove(*ToRelSec);
         }
         return true;
       });
   if (SymbolTable != nullptr && ToRemove(*SymbolTable))
     SymbolTable = nullptr;
-  if (ToRemove(*SectionNames)) {
-    if (WriteSectionHeaders)
-      error("Cannot remove " + SectionNames->Name +
-            " because it is the section header string table.");
+  if (SectionNames != nullptr && ToRemove(*SectionNames)) {
     SectionNames = nullptr;
   }
   // Now make sure there are no remaining references to the sections that will
   // be removed. Sometimes it is impossible to remove a reference so we emit
   // an error here instead.
   for (auto &RemoveSec : make_range(Iter, std::end(Sections))) {
     for (auto &Segment : Segments)
       Segment->removeSection(RemoveSec.get());
     for (auto &KeepSec : make_range(std::begin(Sections), Iter))
       KeepSec->removeSectionReferences(RemoveSec.get());
   }
   // Now finally get rid of them all togethor.
   Sections.erase(Iter, std::end(Sections));
 }
 
-template <class ELFT>
-void Object<ELFT>::addSection(StringRef SecName, ArrayRef<uint8_t> Data) {
-  auto Sec = llvm::make_unique<OwnedDataSection>(SecName, Data);
-  Sec->OriginalOffset = ~0ULL;
-  Sections.push_back(std::move(Sec));
-}
-
-template <class ELFT> void Object<ELFT>::addGnuDebugLink(StringRef File) {
-  Sections.emplace_back(llvm::make_unique<GnuDebugLinkSection<ELFT>>(File));
-}
-
-template <class ELFT> void ELFObject<ELFT>::sortSections() {
+void Object::sortSections() {
   // Put all sections in offset order. Maintain the ordering as closely as
   // possible while meeting that demand however.
   auto CompareSections = [](const SecPtr &A, const SecPtr &B) {
     return A->OriginalOffset < B->OriginalOffset;
   };
   std::stable_sort(std::begin(this->Sections), std::end(this->Sections),
                    CompareSections);
 }

 }
 
 // This function finds a consistent layout for a list of sections. It assumes
 // that the ->ParentSegment of each section has already been laid out. The
 // supplied starting Offset is used for the starting offset of any section that
 // does not have a ParentSegment. It returns either the offset given if all
 // sections had a ParentSegment or an offset one past the last section if there
 // was a section that didn't have a ParentSegment.
-template <class SecPtr>
-static uint64_t LayoutSections(std::vector<SecPtr> &Sections, uint64_t Offset) {
+template <class Range>
+static uint64_t LayoutSections(Range Sections, uint64_t Offset) {
   // Now the offset of every segment has been set we can assign the offsets
   // of each section. For sections that are covered by a segment we should use
   // the segment's original offset and the section's original offset to compute
   // the offset from the start of the segment. Using the offset from the start
   // of the segment we can assign a new offset to the section. For sections not
   // covered by segments we can just bump Offset to the next valid location.
   uint32_t Index = 1;
   for (auto &Section : Sections) {
-    Section->Index = Index++;
-    if (Section->ParentSegment != nullptr) {
-      auto Segment = Section->ParentSegment;
-      Section->Offset =
-          Segment->Offset + (Section->OriginalOffset - Segment->OriginalOffset);
+    Section.Index = Index++;
+    if (Section.ParentSegment != nullptr) {
+      auto Segment = *Section.ParentSegment;
+      Section.Offset =
+          Segment.Offset + (Section.OriginalOffset - Segment.OriginalOffset);
     } else {
-      Offset = alignTo(Offset, Section->Align == 0 ? 1 : Section->Align);
-      Section->Offset = Offset;
-      if (Section->Type != SHT_NOBITS)
-        Offset += Section->Size;
+      Offset = alignTo(Offset, Section.Align == 0 ? 1 : Section.Align);
+      Section.Offset = Offset;
+      if (Section.Type != SHT_NOBITS)
+        Offset += Section.Size;
     }
   }
   return Offset;
 }
 
-template <class ELFT> void ELFObject<ELFT>::assignOffsets() {
+template <class ELFT> void ELFWriter<ELFT>::assignOffsets() {
   // We need a temporary list of segments that has a special order to it
   // so that we know that anytime ->ParentSegment is set that segment has
   // already had its offset properly set.
   std::vector<Segment *> OrderedSegments;
-  for (auto &Segment : this->Segments)
-    OrderedSegments.push_back(Segment.get());
+  for (auto &Segment : Obj.segments())
+    OrderedSegments.push_back(&Segment);
   OrderSegments(OrderedSegments);
   // The size of ELF + program headers will not change so it is ok to assume
   // that the first offset of the first segment is a good place to start
   // outputting sections. This covers both the standard case and the PT_PHDR
   // case.
   uint64_t Offset;
   if (!OrderedSegments.empty()) {
     Offset = OrderedSegments[0]->Offset;
   } else {
     Offset = sizeof(Elf_Ehdr);
   }
   Offset = LayoutSegments(OrderedSegments, Offset);
-  Offset = LayoutSections(this->Sections, Offset);
+  Offset = LayoutSections(Obj.sections(), Offset);
   // If we need to write the section header table out then we need to align the
   // Offset so that SHOffset is valid.
-  if (this->WriteSectionHeaders)
+  if (WriteSectionHeaders)
     Offset = alignTo(Offset, sizeof(typename ELFT::Addr));
-  this->SHOffset = Offset;
+  Obj.SHOffset = Offset;
 }
 
-template <class ELFT> size_t ELFObject<ELFT>::totalSize() const {
+template <class ELFT> size_t ELFWriter<ELFT>::totalSize() const {
   // We already have the section header offset so we can calculate the total
   // size by just adding up the size of each section header.
-  auto NullSectionSize = this->WriteSectionHeaders ? sizeof(Elf_Shdr) : 0;
-  return this->SHOffset + this->Sections.size() * sizeof(Elf_Shdr) +
+  auto NullSectionSize = WriteSectionHeaders ? sizeof(Elf_Shdr) : 0;
+  return Obj.SHOffset + size(Obj.sections()) * sizeof(Elf_Shdr) +
          NullSectionSize;
 }
 
-template <class ELFT> void ELFObject<ELFT>::write(FileOutputBuffer &Out) const {
-  this->writeHeader(Out);
-  this->writeProgramHeaders(Out);
-  this->writeSectionData(Out);
-  if (this->WriteSectionHeaders)
-    this->writeSectionHeaders(Out);
+template <class ELFT> void ELFWriter<ELFT>::write() {
+  writeEhdr();
+  writePhdrs();
+  writeSectionData();
+  if (WriteSectionHeaders)
+    writeShdrs();
+  if (auto E = BufPtr->commit())
+    reportError(File, errorToErrorCode(std::move(E)));
 }
 
-template <class ELFT> void ELFObject<ELFT>::finalize() {
+void Writer::createBuffer(uint64_t Size) {
+  auto BufferOrErr =
+      FileOutputBuffer::create(File, Size, FileOutputBuffer::F_executable);
+  handleAllErrors(BufferOrErr.takeError(), [this](const ErrorInfoBase &) {
+    error("failed to open " + File);
+  });
+  BufPtr = std::move(*BufferOrErr);
+}
+
+template <class ELFT> void ELFWriter<ELFT>::finalize() {
+  // It could happen that SectionNames has been removed and yet the user wants
+  // a section header table output. We need to throw an error if a user tries
+  // to do that.
+  if (Obj.SectionNames == nullptr && WriteSectionHeaders)
+    error("Cannot write section header table because section header string "
+          "table was removed.");
+
   // Make sure we add the names of all the sections.
-  if (this->SectionNames != nullptr)
-    for (const auto &Section : this->Sections) {
-      this->SectionNames->addString(Section->Name);
+  if (Obj.SectionNames != nullptr)
+    for (const auto &Section : Obj.sections()) {
+      Obj.SectionNames->addString(Section.Name);
     }
   // Make sure we add the names of all the symbols.
-  if (this->SymbolTable != nullptr)
-    this->SymbolTable->addSymbolNames();
+  if (Obj.SymbolTable != nullptr)
+    Obj.SymbolTable->addSymbolNames();
 
-  sortSections();
+  Obj.sortSections();
   assignOffsets();
 
   // Finalize SectionNames first so that we can assign name indexes.
-  if (this->SectionNames != nullptr)
-    this->SectionNames->finalize();
+  if (Obj.SectionNames != nullptr)
+    Obj.SectionNames->finalize();
   // Finally now that all offsets and indexes have been set we can finalize any
   // remaining issues.
-  uint64_t Offset = this->SHOffset + sizeof(Elf_Shdr);
-  for (auto &Section : this->Sections) {
-    Section->HeaderOffset = Offset;
+  uint64_t Offset = Obj.SHOffset + sizeof(Elf_Shdr);
+  for (auto &Section : Obj.sections()) {
+    Section.HeaderOffset = Offset;
     Offset += sizeof(Elf_Shdr);
-    if (this->WriteSectionHeaders)
-      Section->NameIndex = this->SectionNames->findIndex(Section->Name);
-    Section->finalize();
-  }
+    if (WriteSectionHeaders)
+      Section.NameIndex = Obj.SectionNames->findIndex(Section.Name);
+    Section.finalize();
   }
 
-template <class ELFT> size_t BinaryObject<ELFT>::totalSize() const {
-  return TotalSize;
+  createBuffer(totalSize());
+  SecWriter = llvm::make_unique<ELFSectionWriter<ELFT>>(*BufPtr);
 }
 
-template <class ELFT>
-void BinaryObject<ELFT>::write(FileOutputBuffer &Out) const {
-  for (auto &Section : this->Sections) {
-    if ((Section->Flags & SHF_ALLOC) == 0)
+void BinaryWriter::write() {
+  for (auto &Section : Obj.sections()) {
+    if ((Section.Flags & SHF_ALLOC) == 0)
       continue;
-    Section->writeSection(Out);
+    Section.accept(*SecWriter);
   }
 }
 
-template <class ELFT> void BinaryObject<ELFT>::finalize() {
+void BinaryWriter::finalize() {
   // TODO: Create a filter range to construct OrderedSegments from so that this
   // code can be deduped with assignOffsets above. This should also solve the
   // todo below for LayoutSections.
   // We need a temporary list of segments that has a special order to it
   // so that we know that anytime ->ParentSegment is set that segment has
   // already had it's offset properly set. We only want to consider the segments
   // that will affect layout of allocated sections so we only add those.
   std::vector<Segment *> OrderedSegments;
-  for (auto &Section : this->Sections) {
-    if ((Section->Flags & SHF_ALLOC) != 0 &&
-        Section->ParentSegment != nullptr) {
-      OrderedSegments.push_back(Section->ParentSegment);
+  for (auto &Section : Obj.sections()) {
+    if ((Section.Flags & SHF_ALLOC) != 0 && Section.ParentSegment != nullptr) {
+      OrderedSegments.push_back(Section.ParentSegment);
     }
   }
 
   // For binary output, we're going to use physical addresses instead of
   // virtual addresses, since a binary output is used for cases like ROM
   // loading and physical addresses are intended for ROM loading.
   // However, if no segment has a physical address, we'll fallback to using
   // virtual addresses for all.
if (!OrderedSegments.empty()) {
     }
   }
 
   // TODO: generalize LayoutSections to take a range. Pass a special range
   // constructed from an iterator that skips values for which a predicate does
   // not hold. Then pass such a range to LayoutSections instead of constructing
   // AllocatedSections here.
   std::vector<SectionBase *> AllocatedSections;
-  for (auto &Section : this->Sections) {
-    if ((Section->Flags & SHF_ALLOC) == 0)
+  for (auto &Section : Obj.sections()) {
+    if ((Section.Flags & SHF_ALLOC) == 0)
       continue;
-    AllocatedSections.push_back(Section.get());
+    AllocatedSections.push_back(&Section);
   }
-  LayoutSections(AllocatedSections, Offset);
+  LayoutSections(make_pointee_range(AllocatedSections), Offset);
 
   // Now that every section has been laid out we just need to compute the total
   // file size. This might not be the same as the offset returned by
   // LayoutSections, because we want to truncate the last segment to the end of
   // its last section, to match GNU objcopy's behaviour.
   TotalSize = 0;
   for (const auto &Section : AllocatedSections) {
     if (Section->Type != SHT_NOBITS)
       TotalSize = std::max(TotalSize, Section->Offset + Section->Size);
   }
+
+  createBuffer(TotalSize);
+  SecWriter = llvm::make_unique<BinarySectionWriter>(*BufPtr);
 }
 
 namespace llvm {
 
-template class Object<ELF64LE>;
-template class Object<ELF64BE>;
-template class Object<ELF32LE>;
-template class Object<ELF32BE>;
-
-template class ELFObject<ELF64LE>;
-template class ELFObject<ELF64BE>;
-template class ELFObject<ELF32LE>;
-template class ELFObject<ELF32BE>;
-
-template class BinaryObject<ELF64LE>;
-template class BinaryObject<ELF64BE>;
-template class BinaryObject<ELF32LE>;
-template class BinaryObject<ELF32BE>;
+template class ELFBuilder<ELF64LE>;
+template class ELFBuilder<ELF64BE>;
+template class ELFBuilder<ELF32LE>;
+template class ELFBuilder<ELF32BE>;
+
+template class ELFWriter<ELF64LE>;
+template class ELFWriter<ELF64BE>;
+template class ELFWriter<ELF32LE>;
+template class ELFWriter<ELF32BE>;
 
 } // end namespace llvm

llvm/trunk/tools/llvm-objcopy/llvm-objcopy.h

 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
 
 namespace llvm {
 
 LLVM_ATTRIBUTE_NORETURN extern void error(Twine Message);
+LLVM_ATTRIBUTE_NORETURN extern void reportError(StringRef File, Error E);
+LLVM_ATTRIBUTE_NORETURN extern void reportError(StringRef File,
+                                                std::error_code EC);
 
 // This is taken from llvm-readobj.
 // [see here](llvm/tools/llvm-readobj/llvm-readobj.h:38)
 template <class T> T unwrapOrError(Expected<T> EO) {
   if (EO)
     return *EO;
   std::string Buf;
   raw_string_ostream OS(Buf);
   logAllUnhandledErrors(EO.takeError(), OS, "");
   OS.flush();
   error(Buf);
 }
 
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_OBJCOPY_OBJCOPY_H

llvm/trunk/tools/llvm-objcopy/llvm-objcopy.cpp

static cl::opt<std::string>
     AddGnuDebugLink("add-gnu-debuglink",
                     cl::desc("adds a .gnu_debuglink for <debug-file>"),
                     cl::value_desc("debug-file"));
 
 using SectionPred = std::function<bool(const SectionBase &Sec)>;
 
 bool IsDWOSection(const SectionBase &Sec) { return Sec.Name.endswith(".dwo"); }
 
-template <class ELFT>
-bool OnlyKeepDWOPred(const Object<ELFT> &Obj, const SectionBase &Sec) {
+bool OnlyKeepDWOPred(const Object &Obj, const SectionBase &Sec) {
   // We can't remove the section header string table.
-  if (&Sec == Obj.getSectionHeaderStrTab())
+  if (&Sec == Obj.SectionNames)
     return false;
   // Short of keeping the string table we want to keep everything that is a DWO
   // section and remove everything else.
   return !IsDWOSection(Sec);
 }
 
-template <class ELFT>
-void WriteObjectFile(const Object<ELFT> &Obj, StringRef File) {
-  std::unique_ptr<FileOutputBuffer> Buffer;
-  Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
-      FileOutputBuffer::create(File, Obj.totalSize(),
-                               FileOutputBuffer::F_executable);
-  handleAllErrors(BufferOrErr.takeError(), [](const ErrorInfoBase &) {
-    error("failed to open " + OutputFilename);
-  });
-  Buffer = std::move(*BufferOrErr);
-
-  Obj.write(*Buffer);
-  if (auto E = Buffer->commit())
-    reportError(File, errorToErrorCode(std::move(E)));
-}
-
-template <class ELFT>
-void SplitDWOToFile(const ELFObjectFile<ELFT> &ObjFile, StringRef File) {
-  // Construct a second output file for the DWO sections.
-  ELFObject<ELFT> DWOFile(ObjFile);
+static ElfType OutputElfType;
 
-  DWOFile.removeSections([&](const SectionBase &Sec) {
-    return OnlyKeepDWOPred<ELFT>(DWOFile, Sec);
-  });
-  DWOFile.finalize();
-  WriteObjectFile(DWOFile, File);
+std::unique_ptr<Writer> CreateWriter(Object &Obj, StringRef File) {
+  if (OutputFormat == "binary") {
+    return llvm::make_unique<BinaryWriter>(OutputFilename, Obj);
+  }
+  // Depending on the initial ELFT and OutputFormat we need a different Writer.
+  switch (OutputElfType) {
+  case ELFT_ELF32LE:
+    return llvm::make_unique<ELFWriter<ELF32LE>>(File, Obj, !StripSections);
+  case ELFT_ELF64LE:
+    return llvm::make_unique<ELFWriter<ELF64LE>>(File, Obj, !StripSections);
+  case ELFT_ELF32BE:
+    return llvm::make_unique<ELFWriter<ELF32BE>>(File, Obj, !StripSections);
+  case ELFT_ELF64BE:
+    return llvm::make_unique<ELFWriter<ELF64BE>>(File, Obj, !StripSections);
+  }
+  llvm_unreachable("Invalid output format");
+}
+
+void SplitDWOToFile(const Reader &Reader, StringRef File) {
+  auto DWOFile = Reader.create();
+  DWOFile->removeSections(
+      [&](const SectionBase &Sec) { return OnlyKeepDWOPred(*DWOFile, Sec); });
+  auto Writer = CreateWriter(*DWOFile, File);
+  Writer->finalize();
+  Writer->write();
 }
 
 // This function handles the high level operations of GNU objcopy including
 // handling command line options. It's important to outline certain properties
 // we expect to hold of the command line operations. Any operation that "keeps"
 // should keep regardless of a remove. Additionally any removal should respect
 // any previous removals. Lastly whether or not something is removed shouldn't
 // depend a) on the order the options occur in or b) on some opaque priority
 // system. The only priority is that keeps/copies overrule removes.
-template <class ELFT> void CopyBinary(const ELFObjectFile<ELFT> &ObjFile) {
-  std::unique_ptr<Object<ELFT>> Obj;
-
-  if (!OutputFormat.empty() && OutputFormat != "binary")
-    error("invalid output format '" + OutputFormat + "'");
-  if (!OutputFormat.empty() && OutputFormat == "binary")
-    Obj = llvm::make_unique<BinaryObject<ELFT>>(ObjFile);
-  else
-    Obj = llvm::make_unique<ELFObject<ELFT>>(ObjFile);
+void HandleArgs(Object &Obj, const Reader &Reader) {
 
-  if (!SplitDWO.empty())
-    SplitDWOToFile<ELFT>(ObjFile, SplitDWO.getValue());
+  if (!SplitDWO.empty()) {
+    SplitDWOToFile(Reader, SplitDWO);
+  }
 
   // Localize:
 
   if (LocalizeHidden) {
-    Obj->getSymTab()->localize([](const Symbol &Sym) {
+    Obj.SymbolTable->localize([](const Symbol &Sym) {
       return Sym.Visibility == STV_HIDDEN || Sym.Visibility == STV_INTERNAL;
     });
   }
 
   SectionPred RemovePred = [](const SectionBase &) { return false; };
 
   // Removes:
 
   if (!ToRemove.empty()) {
     RemovePred = [&](const SectionBase &Sec) {
       return std::find(std::begin(ToRemove), std::end(ToRemove), Sec.Name) !=
              std::end(ToRemove);
     };
   }
 
   if (StripDWO || !SplitDWO.empty())
     RemovePred = [RemovePred](const SectionBase &Sec) {
       return IsDWOSection(Sec) || RemovePred(Sec);
     };
 
   if (ExtractDWO)
     RemovePred = [RemovePred, &Obj](const SectionBase &Sec) {
-      return OnlyKeepDWOPred(*Obj, Sec) || RemovePred(Sec);
+      return OnlyKeepDWOPred(Obj, Sec) || RemovePred(Sec);
     };
 
   if (StripAllGNU)
     RemovePred = [RemovePred, &Obj](const SectionBase &Sec) {
       if (RemovePred(Sec))
         return true;
       if ((Sec.Flags & SHF_ALLOC) != 0)
         return false;
-      if (&Sec == Obj->getSectionHeaderStrTab())
+      if (&Sec == Obj.SectionNames)
         return false;
       switch (Sec.Type) {
       case SHT_SYMTAB:
       case SHT_REL:
       case SHT_RELA:
       case SHT_STRTAB:
         return true;
       }
       return Sec.Name.startswith(".debug");
     };
 
   if (StripSections) {
     RemovePred = [RemovePred](const SectionBase &Sec) {
       return RemovePred(Sec) || (Sec.Flags & SHF_ALLOC) == 0;
     };
-    Obj->WriteSectionHeaders = false;
   }
 
   if (StripDebug) {
     RemovePred = [RemovePred](const SectionBase &Sec) {
       return RemovePred(Sec) || Sec.Name.startswith(".debug");
     };
   }
 
   if (StripNonAlloc)
     RemovePred = [RemovePred, &Obj](const SectionBase &Sec) {
       if (RemovePred(Sec))
         return true;
-      if (&Sec == Obj->getSectionHeaderStrTab())
+      if (&Sec == Obj.SectionNames)
         return false;
       return (Sec.Flags & SHF_ALLOC) == 0;
     };
 
   if (StripAll)
     RemovePred = [RemovePred, &Obj](const SectionBase &Sec) {
       if (RemovePred(Sec))
         return true;
-      if (&Sec == Obj->getSectionHeaderStrTab())
+      if (&Sec == Obj.SectionNames)
         return false;
       if (Sec.Name.startswith(".gnu.warning"))
         return false;
       return (Sec.Flags & SHF_ALLOC) == 0;
     };
 
   // Explicit copies:
 
   if (!OnlyKeep.empty()) {
     RemovePred = [RemovePred, &Obj](const SectionBase &Sec) {
       // Explicitly keep these sections regardless of previous removes.
       if (std::find(std::begin(OnlyKeep), std::end(OnlyKeep), Sec.Name) !=
           std::end(OnlyKeep))
         return false;
 
       // Allow all implicit removes.
-      if (RemovePred(Sec)) {
+      if (RemovePred(Sec))
         return true;
-      }
 
       // Keep special sections.
-      if (Obj->getSectionHeaderStrTab() == &Sec) {
+      if (Obj.SectionNames == &Sec)
         return false;
-      }
-      if (Obj->getSymTab() == &Sec || Obj->getSymTab()->getStrTab() == &Sec) {
+      if (Obj.SymbolTable == &Sec || Obj.SymbolTable->getStrTab() == &Sec)
         return false;
-      }
+
       // Remove everything else.
       return true;
     };
   }
 
   if (!Keep.empty()) {
     RemovePred = [RemovePred](const SectionBase &Sec) {
       // Explicitly keep these sections regardless of previous removes.
       if (std::find(std::begin(Keep), std::end(Keep), Sec.Name) !=
           std::end(Keep))
         return false;
       // Otherwise defer to RemovePred.
       return RemovePred(Sec);
     };
   }
 
-  Obj->removeSections(RemovePred);
+  Obj.removeSections(RemovePred);
 
   if (!AddSection.empty()) {
     for (const auto &Flag : AddSection) {
       auto SecPair = StringRef(Flag).split("=");
       auto SecName = SecPair.first;
       auto File = SecPair.second;
       auto BufOrErr = MemoryBuffer::getFile(File);
       if (!BufOrErr)
         reportError(File, BufOrErr.getError());
       auto Buf = std::move(*BufOrErr);
       auto BufPtr = reinterpret_cast<const uint8_t *>(Buf->getBufferStart());
       auto BufSize = Buf->getBufferSize();
-      Obj->addSection(SecName, ArrayRef<uint8_t>(BufPtr, BufSize));
+      Obj.addSection<OwnedDataSection>(SecName,
+                                       ArrayRef<uint8_t>(BufPtr, BufSize));
     }
   }
 
   if (!AddGnuDebugLink.empty()) {
-    Obj->addGnuDebugLink(AddGnuDebugLink);
-  }
+    Obj.addSection<GnuDebugLinkSection>(StringRef(AddGnuDebugLink));
+  }
 
-  Obj->finalize();
-  WriteObjectFile(*Obj, OutputFilename.getValue());
+std::unique_ptr<Reader> CreateReader() {
+  // Right now we can only read ELF files so there's only one reader;
+  auto Out = llvm::make_unique<ELFReader>(StringRef(InputFilename));
+  // We need to set the default ElfType for output.
+  OutputElfType = Out->getElfType();
+  return std::move(Out);
 }
 
 int main(int argc, char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
   llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
   cl::ParseCommandLineOptions(argc, argv, "llvm objcopy utility\n");
   ToolName = argv[0];
   if (InputFilename.empty()) {
     cl::PrintHelpMessage();
     return 2;
   }
-  Expected<OwningBinary<Binary>> BinaryOrErr = createBinary(InputFilename);
-  if (!BinaryOrErr)
-    reportError(InputFilename, BinaryOrErr.takeError());
-  Binary &Binary = *BinaryOrErr.get().getBinary();
-  if (auto *o = dyn_cast<ELFObjectFile<ELF64LE>>(&Binary)) {
-    CopyBinary(*o);
-    return 0;
-  }
-  if (auto *o = dyn_cast<ELFObjectFile<ELF32LE>>(&Binary)) {
-    CopyBinary(*o);
-    return 0;
-  }
-  if (auto *o = dyn_cast<ELFObjectFile<ELF64BE>>(&Binary)) {
-    CopyBinary(*o);
-    return 0;
-  }
-  if (auto *o = dyn_cast<ELFObjectFile<ELF32BE>>(&Binary)) {
-    CopyBinary(*o);
-    return 0;
-  }
-  reportError(InputFilename, object_error::invalid_file_type);
+
+  auto Reader = CreateReader();
+  auto Obj = Reader->create();
+  auto Writer = CreateWriter(*Obj, OutputFilename);
+  HandleArgs(*Obj, *Reader);
+  Writer->finalize();
+  Writer->write();
 }