[llvm-objcopy][ELF] Implement a mutable section visitor that updates size-related fields (Size, EntrySize, Align) before layout.

[rupprecht]

Summary

Fix EntrySize, Size, and Align before doing layout calculation.

As a side cleanup, this removes a dependence on sizeof(Elf_Sym) within BinaryReader, so we can untemplatize that.

This unblocks a cleaner implementation of handling the -O<format> flag. See D53667 for a previous attempt. Actual implementation of the -O<format> flag will come in an upcoming commit, this is largely a NFC (although not _totally_ one, because alignment on binary input was actually wrong before).

[jakehehrlich]

This is a preety nice solution. I'd been working on removing a lot of the fields from SectionBase, and making an object that had all the sizes of various things listed in it, and constructing that from the ELFT and moving all the calculation of such things into the Writer. That changes kept getting larger and more hairy. This is still quite clean and far less invasive. Also I think a mutable section visitor is a generically useful thing to have around. Nice work. I'd wait for one more LGTM. Sorry I wasn't able to get the change I promised out in time.

[jhenderson]

LGTM, with a couple of minor comments.

I am somewhat concerned that we're adding another loop over all the sections, as this could be somewhat slow for very large objects, but it's probably a silly concern. However, it might be nice to move the section sizing later, to the point where all sections have been added, into the Index setting loop. Otherwise, when we are adding any additional sections we have to ensure that their sizes are correct, whereas the section sizer could do that all for us. I don't mind really though.

tools/llvm-objcopy/ELF/Object.cpp

89

I assume here and below, sh_addralign is used as a placeholder for "whatever is the largest data field in the struct" (i.e. sh_size or whatever would be equally valid)?

tools/llvm-objcopy/ELF/Object.h

91

I'm a little troubled with how similar this is to SectionVisitor, but I don't have a better solution overall. However, I would like it for the destructors to be defined in the same manner. At the moment this one is defaulted, and the other is defined out-of-line.

156–157

I'm not sure you need either of these? The class has no members, so these should be trivially generated by the compiler with no need to be explicit. I think (I might be misremembering my C++...)

[rupprecht]

• Merge loops over sections
• Clean up constructors
• Explicitly use the largest field in the struct for addralign

[rupprecht]

tools/llvm-objcopy/ELF/Object.cpp

89

Yes, I'm pretty sure that's where this is coming from. It's been a bit since I first wrote this patch though, so I'll have to do some digging today -- this matches what GNU objcopy seems to do, but there may be a more technically correct way to do this.

[jhenderson]

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

89 ↗

(On Diff #180094)

Nit: larget -> largest

97 ↗

(On Diff #180094)

Ditto.

[rupprecht]

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

89 ↗

(On Diff #180094)

d'oh.... haven't had coffee yet :(

Fixed in r350337

test/tools/llvm-objcopy/ELF/binary-input.test

 # CHECK-NEXT:     Type: SHT_SYMTAB (0x2)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Address: 0x0
 # CHECK-NEXT:     Offset:
 # CHECK-NEXT:     Size:
 # CHECK-NEXT:     Link: 1
 # CHECK-NEXT:     Info: 1
-# CHECK-NEXT:     AddressAlignment: 1
+# CHECK-NEXT:     AddressAlignment: 8
 # CHECK-NEXT:     EntrySize: 24
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Section {
 # CHECK-NEXT:     Index: 3
 # CHECK-NEXT:     Name: .data
 # CHECK-NEXT:     Type: SHT_PROGBITS (0x1)
 # CHECK-NEXT:     Flags [ (0x3)
 # CHECK-NEXT:       SHF_ALLOC (0x2)

tools/llvm-objcopy/ELF/Object.h

public:
   virtual void visit(const DynamicRelocationSection &Sec) = 0;
   virtual void visit(const GnuDebugLinkSection &Sec) = 0;
   virtual void visit(const GroupSection &Sec) = 0;
   virtual void visit(const SectionIndexSection &Sec) = 0;
   virtual void visit(const CompressedSection &Sec) = 0;
   virtual void visit(const DecompressedSection &Sec) = 0;
 };
 
+class MutableSectionVisitor {
+public:
+  virtual ~MutableSectionVisitor() = default;
+
+  virtual void visit(Section &Sec) = 0;
+  virtual void visit(OwnedDataSection &Sec) = 0;
+  virtual void visit(StringTableSection &Sec) = 0;
+  virtual void visit(SymbolTableSection &Sec) = 0;
+  virtual void visit(RelocationSection &Sec) = 0;
+  virtual void visit(DynamicRelocationSection &Sec) = 0;
+  virtual void visit(GnuDebugLinkSection &Sec) = 0;
+  virtual void visit(GroupSection &Sec) = 0;
+  virtual void visit(SectionIndexSection &Sec) = 0;
+  virtual void visit(CompressedSection &Sec) = 0;
+  virtual void visit(DecompressedSection &Sec) = 0;
+};
+
 class SectionWriter : public SectionVisitor {
 protected:
   Buffer &Out;
 
 public:
   virtual ~SectionWriter(){};
 
   void visit(const Section &Sec) override;
public:
   void visit(const GroupSection &Sec) override;
   void visit(const SectionIndexSection &Sec) override;
   void visit(const CompressedSection &Sec) override;
   void visit(const DecompressedSection &Sec) override;
 
   explicit ELFSectionWriter(Buffer &Buf) : SectionWriter(Buf) {}
 };
 
+template <class ELFT> class ELFSectionSizer : public MutableSectionVisitor {
+private:
+  using Elf_Rel = typename ELFT::Rel;
+  using Elf_Rela = typename ELFT::Rela;
+  using Elf_Shdr = typename ELFT::Shdr;
+  using Elf_Sym = typename ELFT::Sym;
+
+public:
+  ELFSectionSizer() = default;
+  virtual ~ELFSectionSizer() = default;
+  void visit(Section &Sec) override;
+  void visit(OwnedDataSection &Sec) override;
+  void visit(StringTableSection &Sec) override;
+  void visit(DynamicRelocationSection &Sec) override;
+  void visit(SymbolTableSection &Sec) override;
+  void visit(RelocationSection &Sec) override;
+  void visit(GnuDebugLinkSection &Sec) override;
+  void visit(GroupSection &Sec) override;
+  void visit(SectionIndexSection &Sec) override;
+  void visit(CompressedSection &Sec) override;
+  void visit(DecompressedSection &Sec) override;
+};
+
 #define MAKE_SEC_WRITER_FRIEND                                                 \
   friend class SectionWriter;                                                  \
-  template <class ELFT> friend class ELFSectionWriter;
+  template <class ELFT> friend class ELFSectionWriter;                         \
+  template <class ELFT> friend class ELFSectionSizer;
 
 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;
public:
 
   virtual ~SectionBase() = default;
 
   virtual void initialize(SectionTableRef SecTable);
   virtual void finalize();
   virtual void removeSectionReferences(const SectionBase *Sec);
   virtual void removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
   virtual void accept(SectionVisitor &Visitor) const = 0;
+  virtual void accept(MutableSectionVisitor &Visitor) = 0;
   virtual void markSymbols();
 };
 
 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
class Section : public SectionBase {
 
   ArrayRef<uint8_t> Contents;
   SectionBase *LinkSection = nullptr;
 
 public:
   explicit Section(ArrayRef<uint8_t> Data) : Contents(Data) {}
 
   void accept(SectionVisitor &Visitor) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
   void removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
 };
 
 class OwnedDataSection : public SectionBase {
   MAKE_SEC_WRITER_FRIEND
 
   std::vector<uint8_t> Data;
 
 public:
   OwnedDataSection(StringRef SecName, ArrayRef<uint8_t> Data)
       : Data(std::begin(Data), std::end(Data)) {
     Name = SecName.str();
     Type = ELF::SHT_PROGBITS;
     Size = Data.size();
     OriginalOffset = std::numeric_limits<uint64_t>::max();
   }
 
   void accept(SectionVisitor &Sec) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
 };
 
 class CompressedSection : public SectionBase {
   MAKE_SEC_WRITER_FRIEND
 
   DebugCompressionType CompressionType;
   uint64_t DecompressedSize;
   uint64_t DecompressedAlign;
   SmallVector<char, 128> CompressedData;
 
 public:
   CompressedSection(const SectionBase &Sec,
                     DebugCompressionType CompressionType);
   CompressedSection(ArrayRef<uint8_t> CompressedData, uint64_t DecompressedSize,
                     uint64_t DecompressedAlign);
 
   uint64_t getDecompressedSize() const { return DecompressedSize; }
   uint64_t getDecompressedAlign() const { return DecompressedAlign; }
 
   void accept(SectionVisitor &Visitor) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
 
   static bool classof(const SectionBase *S) {
     return (S->Flags & ELF::SHF_COMPRESSED) ||
            (StringRef(S->Name).startswith(".zdebug"));
   }
 };
 
 class DecompressedSection : public SectionBase {
   MAKE_SEC_WRITER_FRIEND
 
 public:
   explicit DecompressedSection(const CompressedSection &Sec)
       : SectionBase(Sec) {
     Size = Sec.getDecompressedSize();
     Align = Sec.getDecompressedAlign();
     Flags = (Flags & ~ELF::SHF_COMPRESSED);
     if (StringRef(Name).startswith(".zdebug"))
       Name = "." + Name.substr(2);
   }
 
   void accept(SectionVisitor &Visitor) const override;
+  void accept(MutableSectionVisitor &Visitor) 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
 
   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 accept(SectionVisitor &Visitor) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
 
   static bool classof(const SectionBase *S) {
     if (S->Flags & ELF::SHF_ALLOC)
       return false;
     return S->Type == ELF::SHT_STRTAB;
   }
 };
 
public:
   void addIndex(uint32_t Index) {
     Indexes.push_back(Index);
     Size += 4;
   }
   void setSymTab(SymbolTableSection *SymTab) { Symbols = SymTab; }
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
   void accept(SectionVisitor &Visitor) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
 
   SectionIndexSection() {
     Name = ".symtab_shndx";
     Align = 4;
     EntrySize = 4;
     Type = ELF::SHT_SYMTAB_SHNDX;
   }
 };
public:
   const Symbol *getSymbolByIndex(uint32_t Index) const;
   Symbol *getSymbolByIndex(uint32_t Index);
   void updateSymbols(function_ref<void(Symbol &)> Callable);
 
   void removeSectionReferences(const SectionBase *Sec) override;
   void initialize(SectionTableRef SecTable) override;
   void finalize() override;
   void accept(SectionVisitor &Visitor) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
   void removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
 
   static bool classof(const SectionBase *S) {
     return S->Type == ELF::SHT_SYMTAB;
   }
 };
 
 struct Relocation {
class RelocationSection
     : public RelocSectionWithSymtabBase<SymbolTableSection> {
   MAKE_SEC_WRITER_FRIEND
 
   std::vector<Relocation> Relocations;
 
 public:
   void addRelocation(Relocation Rel) { Relocations.push_back(Rel); }
   void accept(SectionVisitor &Visitor) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
   void removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
   void markSymbols() 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:
   explicit GroupSection(ArrayRef<uint8_t> Data) : Contents(Data) {}
 
   void setSymTab(const SymbolTableSection *SymTabSec) { SymTab = SymTabSec; }
   void setSymbol(Symbol *S) { Sym = S; }
   void setFlagWord(ELF::Elf32_Word W) { FlagWord = W; }
   void addMember(SectionBase *Sec) { GroupMembers.push_back(Sec); }
 
   void accept(SectionVisitor &) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
   void finalize() override;
   void removeSymbols(function_ref<bool(const Symbol &)> ToRemove) override;
   void markSymbols() override;
 
   static bool classof(const SectionBase *S) {
     return S->Type == ELF::SHT_GROUP;
   }
 };

 
 private:
   ArrayRef<uint8_t> Contents;
 
 public:
   explicit DynamicRelocationSection(ArrayRef<uint8_t> Data) : Contents(Data) {}
 
   void accept(SectionVisitor &) const override;
+  void accept(MutableSectionVisitor &Visitor) 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;
   }
 };
 
 class GnuDebugLinkSection : public SectionBase {
   MAKE_SEC_WRITER_FRIEND
 
 private:
   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.
   explicit GnuDebugLinkSection(StringRef File);
   void accept(SectionVisitor &Visitor) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
 };
 
 class Reader {
 public:
   virtual ~Reader();
   virtual std::unique_ptr<Object> create() const = 0;
 };
 
 using object::Binary;
 using object::ELFFile;
 using object::ELFObjectFile;
 using object::OwningBinary;
 
-template <class ELFT> class BinaryELFBuilder {
-  using Elf_Sym = typename ELFT::Sym;
-
+class BinaryELFBuilder {
   uint16_t EMachine;
   MemoryBuffer *MemBuf;
   std::unique_ptr<Object> Obj;
 
   void initFileHeader();
   void initHeaderSegment();
   StringTableSection *addStrTab();
   SymbolTableSection *addSymTab(StringTableSection *StrTab);

tools/llvm-objcopy/ELF/Object.cpp

template <class ELFT> void ELFWriter<ELFT>::writeShdr(const SectionBase &Sec) {
   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;
 }
 
+template <class ELFT> void ELFSectionSizer<ELFT>::visit(Section &Sec) {}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(OwnedDataSection &Sec) {}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(StringTableSection &Sec) {}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(DynamicRelocationSection &Sec) {}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(SymbolTableSection &Sec) {
+  Sec.EntrySize = sizeof(Elf_Sym);
+  Sec.Size = Sec.Symbols.size() * Sec.EntrySize;
+  Sec.Align = sizeof(Elf_Shdr::sh_addralign);
+}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(RelocationSection &Sec) {
+  Sec.EntrySize = Sec.Type == SHT_REL ? sizeof(Elf_Rel) : sizeof(Elf_Rela);
+  Sec.Size = Sec.Relocations.size() * Sec.EntrySize;
+  Sec.Align = sizeof(Elf_Shdr::sh_addralign);
+}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(GnuDebugLinkSection &Sec) {}
+
+template <class ELFT> void ELFSectionSizer<ELFT>::visit(GroupSection &Sec) {}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(SectionIndexSection &Sec) {}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(CompressedSection &Sec) {}
+
+template <class ELFT>
+void ELFSectionSizer<ELFT>::visit(DecompressedSection &Sec) {}
+
 SectionVisitor::~SectionVisitor() {}
 
 void BinarySectionWriter::visit(const SectionIndexSection &Sec) {
   error("Cannot write symbol section index table '" + Sec.Name + "' ");
 }
 
 void BinarySectionWriter::visit(const SymbolTableSection &Sec) {
   error("Cannot write symbol table '" + Sec.Name + "' out to binary");
void SectionWriter::visit(const Section &Sec) {
   if (Sec.Type == SHT_NOBITS)
     return;
   uint8_t *Buf = Out.getBufferStart() + Sec.Offset;
   llvm::copy(Sec.Contents, Buf);
 }
 
 void Section::accept(SectionVisitor &Visitor) const { Visitor.visit(*this); }
 
+void Section::accept(MutableSectionVisitor &Visitor) { Visitor.visit(*this); }
+
 void SectionWriter::visit(const OwnedDataSection &Sec) {
   uint8_t *Buf = Out.getBufferStart() + Sec.Offset;
   llvm::copy(Sec.Data, Buf);
 }
 
 static const std::vector<uint8_t> ZlibGnuMagic = {'Z', 'L', 'I', 'B'};
 
 static bool isDataGnuCompressed(ArrayRef<uint8_t> Data) {

 void BinarySectionWriter::visit(const DecompressedSection &Sec) {
   error("Cannot write compressed section '" + Sec.Name + "' ");
 }
 
 void DecompressedSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void DecompressedSection::accept(MutableSectionVisitor &Visitor) {
+  Visitor.visit(*this);
+}
+
 void OwnedDataSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void OwnedDataSection::accept(MutableSectionVisitor &Visitor) {
+  Visitor.visit(*this);
+}
+
 void BinarySectionWriter::visit(const CompressedSection &Sec) {
   error("Cannot write compressed section '" + Sec.Name + "' ");
 }
 
 template <class ELFT>
 void ELFSectionWriter<ELFT>::visit(const CompressedSection &Sec) {
   uint8_t *Buf = Out.getBufferStart();
   Buf += Sec.Offset;
: CompressionType(DebugCompressionType::None),
       DecompressedSize(DecompressedSize), DecompressedAlign(DecompressedAlign) {
   OriginalData = CompressedData;
 }
 
 void CompressedSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void CompressedSection::accept(MutableSectionVisitor &Visitor) {
+  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 SectionWriter::visit(const StringTableSection &Sec) {
   Sec.StrTabBuilder.write(Out.getBufferStart() + Sec.Offset);
 }
 
 void StringTableSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void StringTableSection::accept(MutableSectionVisitor &Visitor) {
+  Visitor.visit(*this);
+}
+
 template <class ELFT>
 void ELFSectionWriter<ELFT>::visit(const SectionIndexSection &Sec) {
   uint8_t *Buf = Out.getBufferStart() + Sec.Offset;
   auto *IndexesBuffer = reinterpret_cast<Elf_Word *>(Buf);
   llvm::copy(Sec.Indexes, IndexesBuffer);
 }
 
 void SectionIndexSection::initialize(SectionTableRef SecTable) {
   Size = 0;
   setSymTab(SecTable.getSectionOfType<SymbolTableSection>(
       Link,
       "Link field value " + Twine(Link) + " in section " + Name + " is invalid",
       "Link field value " + Twine(Link) + " in section " + Name +
           " is not a symbol table"));
   Symbols->setShndxTable(this);
 }
 
 void SectionIndexSection::finalize() { Link = Symbols->Index; }
 
 void SectionIndexSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void SectionIndexSection::accept(MutableSectionVisitor &Visitor) {
+  Visitor.visit(*this);
+}
+
 static bool isValidReservedSectionIndex(uint16_t Index, uint16_t Machine) {
   switch (Index) {
   case SHN_ABS:
   case SHN_COMMON:
     return true;
   }
   if (Machine == EM_HEXAGON) {
     switch (Index) {
for (auto &Symbol : Sec.Symbols) {
     ++Sym;
   }
 }
 
 void SymbolTableSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void SymbolTableSection::accept(MutableSectionVisitor &Visitor) {
+  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 " +
void ELFSectionWriter<ELFT>::visit(const RelocationSection &Sec) {
   else
     writeRel(Sec.Relocations, reinterpret_cast<Elf_Rela *>(Buf));
 }
 
 void RelocationSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void RelocationSection::accept(MutableSectionVisitor &Visitor) {
+  Visitor.visit(*this);
+}
+
 void RelocationSection::removeSymbols(
     function_ref<bool(const Symbol &)> ToRemove) {
   for (const Relocation &Reloc : Relocations)
     if (ToRemove(*Reloc.RelocSymbol))
       error("not stripping symbol '" + Reloc.RelocSymbol->Name +
             "' because it is named in a relocation");
 }
 
 void RelocationSection::markSymbols() {
   for (const Relocation &Reloc : Relocations)
     Reloc.RelocSymbol->Referenced = true;
 }
 
 void SectionWriter::visit(const DynamicRelocationSection &Sec) {
   llvm::copy(Sec.Contents,
             Out.getBufferStart() + Sec.Offset);
 }
 
 void DynamicRelocationSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void DynamicRelocationSection::accept(MutableSectionVisitor &Visitor) {
+  Visitor.visit(*this);
+}
+
 void Section::removeSectionReferences(const SectionBase *Sec) {
   if (LinkSection == Sec) {
     error("Section " + LinkSection->Name +
           " cannot be removed because it is "
           "referenced by the section " +
           this->Name);
   }
 }
void ELFSectionWriter<ELFT>::visit(const GnuDebugLinkSection &Sec) {
   *CRC = Sec.CRC32;
   llvm::copy(Sec.FileName, File);
 }
 
 void GnuDebugLinkSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void GnuDebugLinkSection::accept(MutableSectionVisitor &Visitor) {
+  Visitor.visit(*this);
+}
+
 template <class ELFT>
 void ELFSectionWriter<ELFT>::visit(const GroupSection &Sec) {
   ELF::Elf32_Word *Buf =
       reinterpret_cast<ELF::Elf32_Word *>(Out.getBufferStart() + Sec.Offset);
   *Buf++ = Sec.FlagWord;
   for (const auto *S : Sec.GroupMembers)
     support::endian::write32<ELFT::TargetEndianness>(Buf++, S->Index);
 }
 
 void GroupSection::accept(SectionVisitor &Visitor) const {
   Visitor.visit(*this);
 }
 
+void GroupSection::accept(MutableSectionVisitor &Visitor) {
+  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
   // not the first.
   uint64_t SecSize = Section.Size ? Section.Size : 1;

 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 BinaryELFBuilder<ELFT>::initFileHeader() {
+void BinaryELFBuilder::initFileHeader() {
   Obj->Flags = 0x0;
   Obj->Type = ET_REL;
   Obj->OSABI = ELFOSABI_NONE;
   Obj->ABIVersion = 0;
   Obj->Entry = 0x0;
   Obj->Machine = EMachine;
   Obj->Version = 1;
 }
 
-template <class ELFT> void BinaryELFBuilder<ELFT>::initHeaderSegment() {
-  Obj->ElfHdrSegment.Index = 0;
-}
+void BinaryELFBuilder::initHeaderSegment() { Obj->ElfHdrSegment.Index = 0; }
 
-template <class ELFT> StringTableSection *BinaryELFBuilder<ELFT>::addStrTab() {
+StringTableSection *BinaryELFBuilder::addStrTab() {
   auto &StrTab = Obj->addSection<StringTableSection>();
   StrTab.Name = ".strtab";
 
   Obj->SectionNames = &StrTab;
   return &StrTab;
 }
 
-template <class ELFT>
-SymbolTableSection *
-BinaryELFBuilder<ELFT>::addSymTab(StringTableSection *StrTab) {
+SymbolTableSection *BinaryELFBuilder::addSymTab(StringTableSection *StrTab) {
   auto &SymTab = Obj->addSection<SymbolTableSection>();
 
   SymTab.Name = ".symtab";
   SymTab.Link = StrTab->Index;
-  // TODO: Factor out dependence on ElfType here.
-  SymTab.EntrySize = sizeof(Elf_Sym);
 
   // The symbol table always needs a null symbol
   SymTab.addSymbol("", 0, 0, nullptr, 0, 0, 0, 0);
 
   Obj->SymbolTable = &SymTab;
   return &SymTab;
 }
 
-template <class ELFT>
-void BinaryELFBuilder<ELFT>::addData(SymbolTableSection *SymTab) {
+void BinaryELFBuilder::addData(SymbolTableSection *SymTab) {
   auto Data = ArrayRef<uint8_t>(
       reinterpret_cast<const uint8_t *>(MemBuf->getBufferStart()),
       MemBuf->getBufferSize());
   auto &DataSection = Obj->addSection<Section>(Data);
   DataSection.Name = ".data";
   DataSection.Type = ELF::SHT_PROGBITS;
   DataSection.Size = Data.size();
   DataSection.Flags = ELF::SHF_ALLOC | ELF::SHF_WRITE;
 
   std::string SanitizedFilename = MemBuf->getBufferIdentifier().str();
   std::replace_if(std::begin(SanitizedFilename), std::end(SanitizedFilename),
                   [](char C) { return !isalnum(C); }, '_');
   Twine Prefix = Twine("_binary_") + SanitizedFilename;
 
   SymTab->addSymbol(Prefix + "_start", STB_GLOBAL, STT_NOTYPE, &DataSection,
                     /*Value=*/0, STV_DEFAULT, 0, 0);
   SymTab->addSymbol(Prefix + "_end", STB_GLOBAL, STT_NOTYPE, &DataSection,
                     /*Value=*/DataSection.Size, STV_DEFAULT, 0, 0);
   SymTab->addSymbol(Prefix + "_size", STB_GLOBAL, STT_NOTYPE, nullptr,
                     /*Value=*/DataSection.Size, STV_DEFAULT, SHN_ABS, 0);
 }
 
-template <class ELFT> void BinaryELFBuilder<ELFT>::initSections() {
+void BinaryELFBuilder::initSections() {
   for (auto &Section : Obj->sections()) {
     Section.initialize(Obj->sections());
   }
 }
 
-template <class ELFT> std::unique_ptr<Object> BinaryELFBuilder<ELFT>::build() {
+std::unique_ptr<Object> BinaryELFBuilder::build() {
   initFileHeader();
   initHeaderSegment();
   StringTableSection *StrTab = addStrTab();
   SymbolTableSection *SymTab = addSymTab(StrTab);
   initSections();
   addData(SymTab);
 
   return std::move(Obj);
template <class R> size_t size(R &&Range) {
   return static_cast<size_t>(std::end(Range) - std::begin(Range));
 }
 
 Writer::~Writer() {}
 
 Reader::~Reader() {}
 
 std::unique_ptr<Object> BinaryReader::create() const {
-  if (MInfo.Is64Bit)
-    return MInfo.IsLittleEndian
-               ? BinaryELFBuilder<ELF64LE>(MInfo.EMachine, MemBuf).build()
-               : BinaryELFBuilder<ELF64BE>(MInfo.EMachine, MemBuf).build();
-  else
-    return MInfo.IsLittleEndian
-               ? BinaryELFBuilder<ELF32LE>(MInfo.EMachine, MemBuf).build()
-               : BinaryELFBuilder<ELF32BE>(MInfo.EMachine, MemBuf).build();
+  return BinaryELFBuilder(MInfo.EMachine, MemBuf).build();
 }
 
 std::unique_ptr<Object> ELFReader::create() const {
   auto Obj = llvm::make_unique<Object>();
   if (auto *O = dyn_cast<ELFObjectFile<ELF32LE>>(Bin)) {
     ELFBuilder<ELF32LE> Builder(*O, *Obj);
     Builder.build();
     return Obj;
template <class ELFT> void ELFWriter<ELFT>::write() {
   writeSectionData();
   if (WriteSectionHeaders)
     writeShdrs();
   if (auto E = Buf.commit())
     reportError(Buf.getName(), errorToErrorCode(std::move(E)));
 }
 
 template <class ELFT> void ELFWriter<ELFT>::finalize() {
+  // The output arch may not be the same as the input arch, so fix up
+  // size-related fields before doing layout calculations.
+  auto SecSizer = llvm::make_unique<ELFSectionSizer<ELFT>>();
+  for (auto &Sec : Obj.sections())
+    Sec.accept(*SecSizer);
+
   // 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.");
 
   Obj.sortSections();
for (const auto &Section : AllocatedSections) {
     if (Section->Type != SHT_NOBITS)
       TotalSize = std::max(TotalSize, Section->Offset + Section->Size);
   }
 
   Buf.allocate(TotalSize);
   SecWriter = llvm::make_unique<BinarySectionWriter>(Buf);
 }
 
-template class BinaryELFBuilder<ELF64LE>;
-template class BinaryELFBuilder<ELF64BE>;
-template class BinaryELFBuilder<ELF32LE>;
-template class BinaryELFBuilder<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 elf
 } // end namespace objcopy
 } // end namespace llvm