[yaml2obj] Align section content using AddressAlign field's value

[atanasyan]

Summary

Use AddressAlign field's value to properly align sections content in the yaml2obj tool. Before this change the yaml2obj ignored AddressAlign and always aligned section on 16 bytes boundary.

[silvas]

Nice catch. The test seems really brittle though, since it depends on the exact offset. Can you instead ask for an alignment larger than the previous default, then regex check that the offset has sufficient trailing zeros? (hopefully make the alignment large enough that there is a low probability that it could happen by accident, but small enough that the resulting filesize isn't enormous/the test is still fast to execute)

[atanasyan]

Do not stick to the exact offset value in the test. Check number of trailing zeros.

[silvas]

Two nits, otherwise LGTM.

test/Object/yaml2obj-elf-alignment.yaml

2

Typo 'accoun'

test/Object/yaml2obj-elf-section-basic.yaml

55 ↗

(On Diff #29155)

Can you commit a separate patch to just change these tests to regex match this field? I don't think the exact offset matters for what these are testing.

[atanasyan]

• The typo is fixed
• yaml2obj-elf-rel.yaml and yaml2obj-elf-section-basic.yaml fixed by a separate commit rL241669 using regex.

[silvas]

Thanks. Feel free to commit this anytime.

[atanasyan]

rL241674

test/Object/yaml2obj-elf-alignment.yaml

+# Check that yaml2obj takes in account section AddressAlign field.
+
+# RUN: yaml2obj -format=elf %s > %t
+# RUN: llvm-readobj -s %t | FileCheck %s
+
+# CHECK:      Section {
+# CHECK:        Index: 2
+# CHECK-NEXT:   Name: .data
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC
+# CHECK-NEXT:     SHF_WRITE
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address: 0x0
+# CHECK-NEXT:   Offset: 0x{{[0-9A-F]*}}00
+# CHECK-NEXT:   Size: 4
+# CHECK-NEXT:   Link: 0
+# CHECK-NEXT:   Info: 0
+# CHECK-NEXT:   AddressAlignment: 256
+# CHECK-NEXT:   EntrySize: 0
+# CHECK-NEXT: }
+
+---
+FileHeader:
+  Class:    ELFCLASS32
+  Data:     ELFDATA2LSB
+  Type:     ET_REL
+  Machine:  EM_MIPS
+  Flags:    [ EF_MIPS_CPIC, EF_MIPS_ABI_O32, EF_MIPS_ARCH_32 ]
+
+Sections:
+  - Name:          .text
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:  8
+    Size:          4
+  - Name:          .data
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:  256
+    Size:          4
+
+Symbols:
+  Global:
+    - Name:     T0
+      Type:     STT_FUNC
+      Section:  .text
+      Size:     4
+    - Name:     D0
+      Type:     STT_OBJECT
+      Section:  .data
+      Size:     4
+...

tools/yaml2obj/yaml2elf.cpp

 namespace {
 class ContiguousBlobAccumulator {
   const uint64_t InitialOffset;
   SmallVector<char, 128> Buf;
   raw_svector_ostream OS;
 
   /// \returns The new offset.
   uint64_t padToAlignment(unsigned Align) {
+    if (Align == 0)
+      Align = 1;
     uint64_t CurrentOffset = InitialOffset + OS.tell();
     uint64_t AlignedOffset = RoundUpToAlignment(CurrentOffset, Align);
     for (; CurrentOffset != AlignedOffset; ++CurrentOffset)
       OS.write('\0');
     return AlignedOffset; // == CurrentOffset;
   }
 
 public:
   ContiguousBlobAccumulator(uint64_t InitialOffset_)
       : InitialOffset(InitialOffset_), Buf(), OS(Buf) {}
   template <class Integer>
-  raw_ostream &getOSAndAlignedOffset(Integer &Offset, unsigned Align = 16) {
+  raw_ostream &getOSAndAlignedOffset(Integer &Offset, unsigned Align) {
     Offset = padToAlignment(Align);
     return OS;
   }
   void writeBlobToStream(raw_ostream &Out) { Out << OS.str(); }
 };
 } // end anonymous namespace
 
 // Used to keep track of section and symbol names, so that in the YAML file
for (const auto &Sec : Doc.Sections) {
     } else if (auto S = dyn_cast<ELFYAML::MipsABIFlags>(Sec.get())) {
       if (!writeSectionContent(SHeader, *S, CBA))
         return false;
     } else if (auto S = dyn_cast<ELFYAML::NoBitsSection>(Sec.get())) {
       SHeader.sh_entsize = 0;
       SHeader.sh_size = S->Size;
       // SHT_NOBITS section does not have content
       // so just to setup the section offset.
-      CBA.getOSAndAlignedOffset(SHeader.sh_offset);
+      CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
     } else
       llvm_unreachable("Unknown section type");
 
     SHeaders.push_back(SHeader);
   }
   return true;
 }
 
void ELFState<ELFT>::initSymtabSectionHeader(Elf_Shdr &SHeader,
   for (const auto &Sym : Doc.Symbols.Weak)
     DotStrtab.add(Sym.Name);
   DotStrtab.finalize(StringTableBuilder::ELF);
 
   addSymbols(Doc.Symbols.Local, Syms, ELF::STB_LOCAL);
   addSymbols(Doc.Symbols.Global, Syms, ELF::STB_GLOBAL);
   addSymbols(Doc.Symbols.Weak, Syms, ELF::STB_WEAK);
 
-  writeArrayData(CBA.getOSAndAlignedOffset(SHeader.sh_offset),
-                 makeArrayRef(Syms));
+  writeArrayData(
+      CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign),
   SHeader.sh_size = arrayDataSize(makeArrayRef(Syms));
 }
 
 template <class ELFT>
 void ELFState<ELFT>::initStrtabSectionHeader(Elf_Shdr &SHeader, StringRef Name,
                                              StringTableBuilder &STB,
                                              ContiguousBlobAccumulator &CBA) {
   zero(SHeader);
   SHeader.sh_name = DotShStrtab.getOffset(Name);
   SHeader.sh_type = ELF::SHT_STRTAB;
-  CBA.getOSAndAlignedOffset(SHeader.sh_offset) << STB.data();
+  CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign)
+      << STB.data();
   SHeader.sh_size = STB.data().size();
   SHeader.sh_addralign = 1;
 }
 
 template <class ELFT>
 void ELFState<ELFT>::addSymbols(const std::vector<ELFYAML::Symbol> &Symbols,
                                 std::vector<Elf_Sym> &Syms,
                                 unsigned SymbolBinding) {

 
 template <class ELFT>
 void
 ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                     const ELFYAML::RawContentSection &Section,
                                     ContiguousBlobAccumulator &CBA) {
   assert(Section.Size >= Section.Content.binary_size() &&
          "Section size and section content are inconsistent");
-  raw_ostream &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset);
+  raw_ostream &OS =
+      CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
   Section.Content.writeAsBinary(OS);
   for (auto i = Section.Content.binary_size(); i < Section.Size; ++i)
     OS.write(0);
   SHeader.sh_entsize = 0;
   SHeader.sh_size = Section.Size;
 }
 
 static bool isMips64EL(const ELFYAML::Object &Doc) {
ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   assert((Section.Type == llvm::ELF::SHT_REL ||
           Section.Type == llvm::ELF::SHT_RELA) &&
          "Section type is not SHT_REL nor SHT_RELA");
 
   bool IsRela = Section.Type == llvm::ELF::SHT_RELA;
   SHeader.sh_entsize = IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
   SHeader.sh_size = SHeader.sh_entsize * Section.Relocations.size();
 
-  auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset);
+  auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
   for (const auto &Rel : Section.Relocations) {
     unsigned SymIdx = 0;
     // Some special relocation, R_ARM_v4BX for instance, does not have
     // an external reference.  So it ignores the return value of lookup()
     // here.
     SymN2I.lookup(Rel.Symbol, SymIdx);
 
bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          ContiguousBlobAccumulator &CBA) {
   typedef typename object::ELFFile<ELFT>::Elf_Word Elf_Word;
   assert(Section.Type == llvm::ELF::SHT_GROUP &&
          "Section type is not SHT_GROUP");
 
   SHeader.sh_entsize = sizeof(Elf_Word);
   SHeader.sh_size = SHeader.sh_entsize * Section.Members.size();
 
-  auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset);
+  auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
   for (auto member : Section.Members) {
     Elf_Word SIdx;
     unsigned int sectionIndex = 0;
     if (member.sectionNameOrType == "GRP_COMDAT")
       sectionIndex = llvm::ELF::GRP_COMDAT;
     else if (SN2I.lookup(member.sectionNameOrType, sectionIndex)) {
       errs() << "error: Unknown section referenced: '"
bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   assert(Section.Type == llvm::ELF::SHT_MIPS_ABIFLAGS &&
          "Section type is not SHT_MIPS_ABIFLAGS");
 
   object::Elf_Mips_ABIFlags<ELFT> Flags;
   zero(Flags);
   SHeader.sh_entsize = sizeof(Flags);
   SHeader.sh_size = SHeader.sh_entsize;
 
-  auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset);
+  auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
   Flags.version = Section.Version;
   Flags.isa_level = Section.ISALevel;
   Flags.isa_rev = Section.ISARevision;
   Flags.gpr_size = Section.GPRSize;
   Flags.cpr1_size = Section.CPR1Size;
   Flags.cpr2_size = Section.CPR2Size;
   Flags.fp_abi = Section.FpABI;
   Flags.isa_ext = Section.ISAExtension;