[llvm-objdump] - Many minor NFC changes to cleanup/improve the code in ELF/Object.cpp.

[grimar]

Summary

The code in ELF/Object.cpp is sometimes a bit hard to read because of
lots of auto used everywhere. The main intention of this patch is
to replace them with the real type for places where it is not obvious.
Also it cleanups few places.

It is NFC change, but I want to be sure that there is no objections to do that since it
is massive.

[rupprecht]

Thanks, looks much more readable

[jakehehrlich]

Solid improvement. LGTM.

BTW the commit tag on this change is "[llvm-objdump]" but it should be "[llvm-objcopy]"

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

 namespace llvm {
 namespace objcopy {
 namespace elf {
 
 using namespace object;
 using namespace ELF;
 
 template <class ELFT> void ELFWriter<ELFT>::writePhdr(const Segment &Seg) {
-  uint8_t *B = Buf.getBufferStart();
-  B += Obj.ProgramHdrSegment.Offset + Seg.Index * sizeof(Elf_Phdr);
+  uint8_t *B = Buf.getBufferStart() + Obj.ProgramHdrSegment.Offset +
+               Seg.Index * sizeof(Elf_Phdr);
   Elf_Phdr &Phdr = *reinterpret_cast<Elf_Phdr *>(B);
   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;

 
 void SectionBase::initialize(SectionTableRef SecTable) {}
 void SectionBase::finalize() {}
 void SectionBase::markSymbols() {}
 void SectionBase::replaceSectionReferences(
     const DenseMap<SectionBase *, SectionBase *> &) {}
 
 template <class ELFT> void ELFWriter<ELFT>::writeShdr(const SectionBase &Sec) {
-  uint8_t *B = Buf.getBufferStart();
-  B += Sec.HeaderOffset;
+  uint8_t *B = Buf.getBufferStart() + Sec.HeaderOffset;
   Elf_Shdr &Shdr = *reinterpret_cast<Elf_Shdr *>(B);
   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;
void BinarySectionWriter::visit(const GnuDebugLinkSection &Sec) {
   error("cannot write '" + Sec.Name + "' out to binary");
 }
 
 void BinarySectionWriter::visit(const GroupSection &Sec) {
   error("cannot write '" + 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);
+  if (Sec.Type != SHT_NOBITS)
+    llvm::copy(Sec.Contents, Out.getBufferStart() + Sec.Offset);
 }
 
 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);
+  llvm::copy(Sec.Data, Out.getBufferStart() + Sec.Offset);
 }
 
 static const std::vector<uint8_t> ZlibGnuMagic = {'Z', 'L', 'I', 'B'};
 
 static bool isDataGnuCompressed(ArrayRef<uint8_t> Data) {
   return Data.size() > ZlibGnuMagic.size() &&
          std::equal(ZlibGnuMagic.begin(), ZlibGnuMagic.end(), Data.data());
 }

 }
 
 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;
-
+  uint8_t *Buf = Out.getBufferStart() + Sec.Offset;
   if (Sec.CompressionType == DebugCompressionType::None) {
     std::copy(Sec.OriginalData.begin(), Sec.OriginalData.end(), Buf);
     return;
   }
 
   if (Sec.CompressionType == DebugCompressionType::GNU) {
     const char *Magic = "ZLIB";
     memcpy(Buf, Magic, strlen(Magic));

 
 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);
+  llvm::copy(Sec.Indexes, reinterpret_cast<Elf_Word *>(Buf));
 }
 
 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 +
setStrTab(SecTable.getSectionOfType<StringTableSection>(
       "Symbol table has link index of " + Twine(Link) +
           " which is not a valid index",
       "Symbol table has link index of " + Twine(Link) +
           " which is not a string table"));
 }
 
 void SymbolTableSection::finalize() {
   uint32_t MaxLocalIndex = 0;
-  for (auto &Sym : Symbols) {
+  for (std::unique_ptr<Symbol> &Sym : Symbols) {
     Sym->NameIndex =
         SymbolNames == nullptr ? 0 : SymbolNames->findIndex(Sym->Name);
     if (Sym->Binding == STB_LOCAL)
       MaxLocalIndex = std::max(MaxLocalIndex, Sym->Index);
   }
   // Now we need to set the Link and Info fields.
   Link = SymbolNames == nullptr ? 0 : SymbolNames->Index;
   Info = MaxLocalIndex + 1;
 }
 
 void SymbolTableSection::prepareForLayout() {
   // Reserve proper amount of space in section index table, so we can
   // layout sections correctly. We will fill the table with correct
   // indexes later in fillShdnxTable.
   if (SectionIndexTable)  
     SectionIndexTable->reserve(Symbols.size());
 
   // Add all of our strings to SymbolNames so that SymbolNames has the right
   // size before layout is decided.
   // If the symbol names section has been removed, don't try to add strings to
   // the table.
   if (SymbolNames != nullptr)
-    for (auto &Sym : Symbols)
+    for (std::unique_ptr<Symbol> &Sym : Symbols)
       SymbolNames->addString(Sym->Name);
 }
 
 void SymbolTableSection::fillShndxTable() {
   if (SectionIndexTable == nullptr)
     return;
   // Fill section index table with real section indexes. This function must
   // be called after assignOffsets.
-  for (const auto &Sym : Symbols) {
+  for (const std::unique_ptr<Symbol> &Sym : Symbols) {
     if (Sym->DefinedIn != nullptr && Sym->DefinedIn->Index >= SHN_LORESERVE)
       SectionIndexTable->addIndex(Sym->DefinedIn->Index);
     else
       SectionIndexTable->addIndex(SHN_UNDEF);
   }
 }
 
 const Symbol *SymbolTableSection::getSymbolByIndex(uint32_t Index) const {
   if (Symbols.size() <= Index)
     error("invalid symbol index: " + Twine(Index));
   return Symbols[Index].get();
 }
 
 Symbol *SymbolTableSection::getSymbolByIndex(uint32_t Index) {
   return const_cast<Symbol *>(
       static_cast<const SymbolTableSection *>(this)->getSymbolByIndex(Index));
 }
 
 template <class ELFT>
 void ELFSectionWriter<ELFT>::visit(const SymbolTableSection &Sec) {
-  uint8_t *Buf = Out.getBufferStart();
-  Buf += Sec.Offset;
-  Elf_Sym *Sym = reinterpret_cast<Elf_Sym *>(Buf);
+  Elf_Sym *Sym = reinterpret_cast<Elf_Sym *>(Out.getBufferStart() + Sec.Offset);
   // Loop though symbols setting each entry of the symbol table.
-  for (auto &Symbol : Sec.Symbols) {
+  for (const std::unique_ptr<Symbol> &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 RelocationSection::replaceSectionReferences(
     const DenseMap<SectionBase *, SectionBase *> &FromTo) {
   // Update the target section if it was replaced.
   if (SectionBase *To = FromTo.lookup(SecToApplyRel))
     SecToApplyRel = To;
 }
 
 void SectionWriter::visit(const DynamicRelocationSection &Sec) {
-  llvm::copy(Sec.Contents,
-            Out.getBufferStart() + Sec.Offset);
+  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);
-}
-
-Error DynamicRelocationSection::removeSectionReferences(
-    bool AllowBrokenLinks, function_ref<bool(const SectionBase *)> ToRemove) {
-  if (ToRemove(Symbols)) {
-    if (!AllowBrokenLinks)
-      return createStringError(
-          llvm::errc::invalid_argument,
+  Visitor.visit(*this);
+}
+
+Error DynamicRelocationSection::removeSectionReferences(
+    bool AllowBrokenLinks, function_ref<bool(const SectionBase *)> ToRemove) {
+  if (ToRemove(Symbols)) {
+    if (!AllowBrokenLinks)
+      return createStringError(
+          llvm::errc::invalid_argument,
           "symbol table '%s' cannot be removed because it is "
           "referenced by the relocation section '%s'",
-          Symbols->Name.data(), this->Name.data());
-    Symbols = nullptr;
-  }
-
-  // SecToApplyRel contains a section referenced by sh_info field. It keeps
-  // a section to which the relocation section applies. When we remove any
-  // sections we also remove their relocation sections. Since we do that much
-  // earlier, this assert should never be triggered.
-  assert(!SecToApplyRel || !ToRemove(SecToApplyRel));
-
-  return Error::success();
-}
-
-Error Section::removeSectionReferences(bool AllowBrokenDependency,
-    function_ref<bool(const SectionBase *)> ToRemove) {
-  if (ToRemove(LinkSection)) {
+          Symbols->Name.data(), this->Name.data());
+    Symbols = nullptr;
+  }
+
+  // SecToApplyRel contains a section referenced by sh_info field. It keeps
+  // a section to which the relocation section applies. When we remove any
+  // sections we also remove their relocation sections. Since we do that much
+  // earlier, this assert should never be triggered.
+  assert(!SecToApplyRel || !ToRemove(SecToApplyRel));
+  return Error::success();
+}
+
+Error Section::removeSectionReferences(bool AllowBrokenDependency,
+    function_ref<bool(const SectionBase *)> ToRemove) {
+  if (ToRemove(LinkSection)) {
     if (!AllowBrokenDependency)
       return createStringError(llvm::errc::invalid_argument,
                                "section '%s' cannot be removed because it is "
                                "referenced by the section '%s'",
                                LinkSection->Name.data(), this->Name.data());
     LinkSection = nullptr;
   }
   return Error::success();

 void GroupSection::replaceSectionReferences(
     const DenseMap<SectionBase *, SectionBase *> &FromTo) {
   for (SectionBase *&Sec : GroupMembers)
     if (SectionBase *To = FromTo.lookup(Sec))
       Sec = To;
 }
 
 void Section::initialize(SectionTableRef SecTable) {
-  if (Link != ELF::SHN_UNDEF) {
-    LinkSection =
-        SecTable.getSection(Link, "Link field value " + Twine(Link) +
-                                      " in section " + Name + " is invalid");
-    if (LinkSection->Type == ELF::SHT_SYMTAB)
-      LinkSection = nullptr;
-  }
-}
+  if (Link == ELF::SHN_UNDEF)
+    return;
 
 void Section::finalize() { this->Link = LinkSection ? LinkSection->Index : 0; }
 
 void GnuDebugLinkSection::init(StringRef File) {
   FileName = sys::path::filename(File);
   // The format for the .gnu_debuglink starts with the 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
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)
+  for (SectionBase *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) {
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);
 }
 
 void BinaryELFBuilder::initSections() {
-  for (auto &Section : Obj->sections()) {
+  for (SectionBase &Section : Obj->sections())
     Section.initialize(Obj->sections());
-  }
-}
 
 std::unique_ptr<Object> BinaryELFBuilder::build() {
   initFileHeader();
   initHeaderSegment();
-  StringTableSection *StrTab = addStrTab();
-  SymbolTableSection *SymTab = addSymTab(StrTab);
+
+  SymbolTableSection *SymTab = addSymTab(addStrTab());
   initSections();
   addData(SymTab);
 
   return std::move(Obj);
 }
 
 template <class ELFT> void ELFBuilder<ELFT>::setParentSegment(Segment &Child) {
-  for (auto &Parent : Obj.segments()) {
+  for (Segment &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)) {
       // We want a canonical "most parental" segment but this requires
       // inspecting the ParentSegment.
       if (compareSegmentsByOffset(&Parent, &Child))
         if (Child.ParentSegment == nullptr ||
             compareSegmentsByOffset(&Parent, Child.ParentSegment)) {
for (const auto &Phdr : unwrapOrError(ElfFile.program_headers())) {
     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 : Obj.sections()) {
+    for (SectionBase &Section : Obj.sections()) {
       if (sectionWithinSegment(Section, Seg)) {
         Seg.addSection(&Section);
         if (!Section.ParentSegment ||
             Section.ParentSegment->Offset > Seg.Offset) {
           Section.ParentSegment = &Seg;
         }
       }
     }
template <class ELFT> void ELFBuilder<ELFT>::readProgramHeaders() {
   PrHdr.PAddr = 0;
   PrHdr.FileSize = PrHdr.MemSize = Ehdr.e_phentsize * Ehdr.e_phnum;
   // The spec requires us to naturally align all the fields.
   PrHdr.Align = sizeof(Elf_Addr);
   PrHdr.Index = Index++;
 
   // Now we do an O(n^2) loop through the segments in order to match up
   // segments.
-  for (auto &Child : Obj.segments())
+  for (Segment &Child : Obj.segments())
     setParentSegment(Child);
   setParentSegment(ElfHdr);
   setParentSegment(PrHdr);
 }
 
 template <class ELFT>
 void ELFBuilder<ELFT>::initGroupSection(GroupSection *GroupSec) {
   if (GroupSec->Align % sizeof(ELF::Elf32_Word) != 0)
     error("invalid alignment " + Twine(GroupSec->Align) + " of group section '" +
           GroupSec->Name + "'");
-  auto SecTable = Obj.sections();
+  SectionTableRef SecTable = Obj.sections();
   auto SymTab = SecTable.template getSectionOfType<SymbolTableSection>(
       GroupSec->Link,
       "link field value '" + Twine(GroupSec->Link) + "' in section '" +
           GroupSec->Name + "' is invalid",
       "link field value '" + Twine(GroupSec->Link) + "' in section '" +
           GroupSec->Name + "' is not a symbol table");
-  auto Sym = SymTab->getSymbolByIndex(GroupSec->Info);
+  Symbol *Sym = SymTab->getSymbolByIndex(GroupSec->Info);
   if (!Sym)
     error("info field value '" + Twine(GroupSec->Info) + "' in section '" +
           GroupSec->Name + "' is not a valid symbol index");
   GroupSec->setSymTab(SymTab);
   GroupSec->setSymbol(Sym);
   if (GroupSec->Contents.size() % sizeof(ELF::Elf32_Word) ||
       GroupSec->Contents.empty())
     error("the content of the section " + GroupSec->Name + " is malformed");
if (auto *O = dyn_cast<ELFObjectFile<ELF32LE>>(Bin)) {
     ELFBuilder<ELF64BE> Builder(*O, *Obj);
     Builder.build();
     return Obj;
   }
   error("invalid file type");
 }
 
 template <class ELFT> void ELFWriter<ELFT>::writeEhdr() {
-  uint8_t *B = Buf.getBufferStart();
-  Elf_Ehdr &Ehdr = *reinterpret_cast<Elf_Ehdr *>(B);
+  Elf_Ehdr &Ehdr = *reinterpret_cast<Elf_Ehdr *>(Buf.getBufferStart());
   std::fill(Ehdr.e_ident, Ehdr.e_ident + 16, 0);
   Ehdr.e_ident[EI_MAG0] = 0x7f;
   Ehdr.e_ident[EI_MAG1] = 'E';
   Ehdr.e_ident[EI_MAG2] = 'L';
   Ehdr.e_ident[EI_MAG3] = 'F';
   Ehdr.e_ident[EI_CLASS] = ELFT::Is64Bits ? ELFCLASS64 : ELFCLASS32;
   Ehdr.e_ident[EI_DATA] =
       ELFT::TargetEndianness == support::big ? ELFDATA2MSB : ELFDATA2LSB;

 }
 
 template <class ELFT> void ELFWriter<ELFT>::writePhdrs() {
   for (auto &Seg : Obj.segments())
     writePhdr(Seg);
 }
 
 template <class ELFT> void ELFWriter<ELFT>::writeShdrs() {
-  uint8_t *B = Buf.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 *>(B);
+  Elf_Shdr &Shdr =
+      *reinterpret_cast<Elf_Shdr *>(Buf.getBufferStart() + Obj.SHOffset);
   Shdr.sh_name = 0;
   Shdr.sh_type = SHT_NULL;
   Shdr.sh_flags = 0;
   Shdr.sh_addr = 0;
   Shdr.sh_offset = 0;
   // See writeEhdr for why we do this.
   uint64_t Shnum = Obj.sections().size() + 1;
   if (Shnum >= SHN_LORESERVE)
     Shdr.sh_size = Shnum;
   else
     Shdr.sh_size = 0;
   // See writeEhdr for why we do this.
   if (Obj.SectionNames != nullptr && Obj.SectionNames->Index >= SHN_LORESERVE)
     Shdr.sh_link = Obj.SectionNames->Index;
   else
     Shdr.sh_link = 0;
   Shdr.sh_info = 0;
   Shdr.sh_addralign = 0;
   Shdr.sh_entsize = 0;
 
-  for (auto &Sec : Obj.sections())
+  for (SectionBase &Sec : Obj.sections())
     writeShdr(Sec);
 }
 
 template <class ELFT> void ELFWriter<ELFT>::writeSectionData() {
-  for (auto &Sec : Obj.sections())
+  for (SectionBase &Sec : Obj.sections())
     // Segments are responsible for writing their contents, so only write the
     // section data if the section is not in a segment. Note that this renders
     // sections in segments effectively immutable.
     if (Sec.ParentSegment == nullptr)
       Sec.accept(*SecWriter);
 }
 
 template <class ELFT> void ELFWriter<ELFT>::writeSegmentData() {
   for (Segment &Seg : Obj.segments()) {
     uint8_t *B = Buf.getBufferStart() + Seg.Offset;
     assert(Seg.FileSize == Seg.getContents().size() &&
            "Segment size must match contents size");
     std::memcpy(B, Seg.getContents().data(), Seg.FileSize);
   }
 
   // Iterate over removed sections and overwrite their old data with zeroes.
   for (auto &Sec : Obj.removedSections()) {
     Segment *Parent = Sec.ParentSegment;
     if (Parent == nullptr || Sec.Type == SHT_NOBITS || Sec.Size == 0)
       continue;
     uint64_t Offset =
         Sec.OriginalOffset - Parent->OriginalOffset + Parent->Offset;
-    uint8_t *B = Buf.getBufferStart();
-    std::memset(B + Offset, 0, Sec.Size);
+    std::memset(Buf.getBufferStart() + Offset, 0, Sec.Size);
   }
 }
 
 template <class ELFT>
 ELFWriter<ELFT>::ELFWriter(Object &Obj, Buffer &Buf, bool WSH)
     : Writer(Obj, Buf), WriteSectionHeaders(WSH && Obj.HadShdrs) {}
 
 Error Object::removeSections(bool AllowBrokenLinks,
static uint64_t layoutSegments(std::vector<Segment *> &Segments,
                                uint64_t Offset) {
   assert(std::is_sorted(std::begin(Segments), std::end(Segments),
                         compareSegmentsByOffset));
   // The only way a segment should move is if a section was between two
   // segments and that section was removed. If that section isn't in a segment
   // then it's acceptable, but not ideal, to simply move it to after the
   // segments. So we can simply layout segments one after the other accounting
   // for alignment.
-  for (auto &Segment : Segments) {
+  for (Segment *Seg : Segments) {
     // We assume that segments have been ordered by OriginalOffset and Index
     // such that a parent segment will always come before a child segment in
     // OrderedSegments. This means that the Offset of the ParentSegment should
     // already be set and we can set our offset relative to it.
-    if (Segment->ParentSegment != nullptr) {
-      auto Parent = Segment->ParentSegment;
-      Segment->Offset =
-          Parent->Offset + Segment->OriginalOffset - Parent->OriginalOffset;
+    if (Seg->ParentSegment != nullptr) {
+      Segment *Parent = Seg->ParentSegment;
+      Seg->Offset =
+          Parent->Offset + Seg->OriginalOffset - Parent->OriginalOffset;
     } else {
-      Offset = alignToAddr(Offset, Segment->VAddr, Segment->Align);
-      Segment->Offset = Offset;
+      Offset = alignToAddr(Offset, Seg->VAddr, Seg->Align);
+      Seg->Offset = Offset;
     }
-    Offset = std::max(Offset, Segment->Offset + Segment->FileSize);
+    Offset = std::max(Offset, Seg->Offset + Seg->FileSize);
   }
   return Offset;
 }
 
 // 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
if (Section.ParentSegment != nullptr) {
       if (Section.Type != SHT_NOBITS)
         Offset += Section.Size;
     }
   }
   return Offset;
 }
 
 template <class ELFT> void ELFWriter<ELFT>::initEhdrSegment() {
-  auto &ElfHdr = Obj.ElfHdrSegment;
+  Segment &ElfHdr = Obj.ElfHdrSegment;
   ElfHdr.Type = PT_PHDR;
   ElfHdr.Flags = 0;
   ElfHdr.OriginalOffset = ElfHdr.Offset = 0;
   ElfHdr.VAddr = 0;
   ElfHdr.PAddr = 0;
   ElfHdr.FileSize = ElfHdr.MemSize = sizeof(Elf_Ehdr);
   ElfHdr.Align = 0;
 }
 
 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 : Obj.segments())
+  for (Segment &Segment : Obj.segments())
     OrderedSegments.push_back(&Segment);
   OrderedSegments.push_back(&Obj.ElfHdrSegment);
   OrderedSegments.push_back(&Obj.ProgramHdrSegment);
   orderSegments(OrderedSegments);
   // Offset is used as the start offset of the first segment to be laid out.
   // Since the ELF Header (ElfHdrSegment) must be at the start of the file,
   // we start at offset 0.
   uint64_t Offset = 0;
template <class ELFT> Error ELFWriter<ELFT>::finalize() {
 
   Obj.sortSections();
 
   // We need to assign indexes before we perform layout because we need to know
   // if we need large indexes or not. We can assign indexes first and check as
   // we go to see if we will actully need large indexes.
   bool NeedsLargeIndexes = false;
   if (Obj.sections().size() >= SHN_LORESERVE) {
-    auto Sections = Obj.sections();
+    SectionTableRef Sections = Obj.sections();
     NeedsLargeIndexes =
         std::any_of(Sections.begin() + SHN_LORESERVE, Sections.end(),
                     [](const SectionBase &Sec) { return Sec.HasSymbol; });
     // TODO: handle case where only one section needs the large index table but
     // only needs it because the large index table hasn't been removed yet.
   }
 
   if (NeedsLargeIndexes) {
template <class ELFT> Error ELFWriter<ELFT>::finalize() {
   // The symbol table does not update all other sections on update. For
   // instance, symbol names are not added as new symbols are added. This means
   // that some sections, like .strtab, don't yet have their final size.
   if (Obj.SymbolTable != nullptr)
     Obj.SymbolTable->prepareForLayout();
 
   // Now that all strings are added we want to finalize string table builders,
   // because that affects section sizes which in turn affects section offsets.
-  for (auto &Sec : Obj.sections())
+  for (SectionBase &Sec : Obj.sections())
     if (auto StrTab = dyn_cast<StringTableSection>(&Sec))
       StrTab->prepareForLayout();
 
   assignOffsets();
 
   // layoutSections could have modified section indexes, so we need
   // to fill the index table after assignOffsets.
   if (Obj.SymbolTable != nullptr)
     Obj.SymbolTable->fillShndxTable();
 
   // Finally now that all offsets and indexes have been set we can finalize any
   // remaining issues.
   uint64_t Offset = Obj.SHOffset + sizeof(Elf_Shdr);
-  for (auto &Section : Obj.sections()) {
+  for (SectionBase &Section : Obj.sections()) {
     Section.HeaderOffset = Offset;
     Offset += sizeof(Elf_Shdr);
     if (WriteSectionHeaders)
       Section.NameIndex = Obj.SectionNames->findIndex(Section.Name);
     Section.finalize();
   }
 
   if (Error E = Buf.allocate(totalSize()))
     return E;
   SecWriter = llvm::make_unique<ELFSectionWriter<ELFT>>(Buf);
   return Error::success();
 }
 
 Error BinaryWriter::write() {
-  for (auto &Section : Obj.sections()) {
-    if ((Section.Flags & SHF_ALLOC) == 0)
-      continue;
+  for (auto &Section : Obj.sections())
+    if (Section.Flags & SHF_ALLOC)
       Section.accept(*SecWriter);
-  }
   return Buf.commit();
 }
 
 Error 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 : Obj.sections()) {
-    if ((Section.Flags & SHF_ALLOC) != 0 && Section.ParentSegment != nullptr) {
+  for (SectionBase &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 (all_of(OrderedSegments,
              [](const Segment *Seg) { return Seg->PAddr == 0; }))
Error BinaryWriter::finalize() {
   OrderedSegments.erase(End, std::end(OrderedSegments));
 
   uint64_t Offset = 0;
 
   // Modify the first segment so that there is no gap at the start. This allows
   // our layout algorithm to proceed as expected while not writing out the gap
   // at the start.
   if (!OrderedSegments.empty()) {
-    auto Seg = OrderedSegments[0];
-    auto Sec = Seg->firstSection();
+    Segment *Seg = OrderedSegments[0];
+    const SectionBase *Sec = Seg->firstSection();
     auto Diff = Sec->OriginalOffset - Seg->OriginalOffset;
     Seg->OriginalOffset += Diff;
     // The size needs to be shrunk as well.
     Seg->FileSize -= Diff;
     // The PAddr needs to be increased to remove the gap before the first
     // section.
     Seg->PAddr += Diff;
     uint64_t LowestPAddr = Seg->PAddr;
-    for (auto &Segment : OrderedSegments) {
+    for (Segment *Segment : OrderedSegments) {
       Segment->Offset = Segment->PAddr - LowestPAddr;
       Offset = std::max(Offset, Segment->Offset + Segment->FileSize);
     }
   }
 
   // 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 : Obj.sections()) {
-    if ((Section.Flags & SHF_ALLOC) == 0)
-      continue;
+  for (SectionBase &Section : Obj.sections())
+    if (Section.Flags & SHF_ALLOC)
       AllocatedSections.push_back(&Section);
-  }
   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) {
+  for (SectionBase *Section : AllocatedSections)
     if (Section->Type != SHT_NOBITS)
       TotalSize = std::max(TotalSize, Section->Offset + Section->Size);
-  }
 
   if (Error E = Buf.allocate(TotalSize))
     return E;
   SecWriter = llvm::make_unique<BinarySectionWriter>(Buf);
   return Error::success();
 }
 
 template class ELFBuilder<ELF64LE>;