[XCOFF] make sure same number of paddings are added

[shchenz]

Summary

This is found in D114419. After changing alignment for DWARF sections from 4 to 32, we found one issue in the XCOFF Object Writer.

One example:
Suppose:
1: the size for all sections(.text, .bss, .data) other than DWARF sections is 52 bytes(already aligned to 4 bytes as required by DefaultSectionAlign):
2: DWARF sections alignment is 32 bytes like in D114419

Step1: In the place where we calculate addresses for DWARF sections, we first align the address of the first DWARF section to new alignment 32, so the start address of DWARF section is 64. We need 64 - 52 = 12 padding bytes here. For this calculation, there are 12 padding bytes before DWARF sections

Step 2: in the place where we calculate FileOffsetToData for each DWARF section. This time, the size of the file header and section header will be considered. In the failure case I meet test/CodeGen/PowerPC/aix-dwarf.ll, the size for XCOFF::FileHeaderSize32 + auxiliaryHeaderSize() + SectionCount * XCOFF::SectionHeaderSize32 is 220, so after adding 52 bytes(sections other than DWARF), the start address of the DWARF section is 272, after 32 bytes align, the start address of DWARF section is 288. And for this calculation, there are 288 - 272 = 16 padding bytes here. And we record FileOffsetToData to the section header assuming that there are 16 padding bytes before DWARF sections.

The mismatch padding bytes for the above two calculations will make XCOFFObjectWriter generate invalid XCOFF object for aix-dwarf.ll

Since currently there is no way to change the alignment for DWARF sections, I left this patch without tests and I guess this patch can be verified together with D114419?

[shchenz]

gentle ping

[Esme]

I don't think a non-NFC patch should be missing tests, how about making D114419 to the parent of this patch and leaving an error check there to be fixed in this patch?

llvm/lib/MC/XCOFFObjectWriter.cpp

1098

simplify the if-else to

if (LastDwarfSection)
  LastDwarfSection->MemorySize =
      DwarfSection.Address - LastDwarfSection->Address;
LastDwarfSection = &DwarfSection;

1101

1103

Why do you want the last Dwarf section to be aligned with the DefaultSectionAlign? It seems to me that it is followed by relocation and not sections, right?

[shchenz]

I don't think a non-NFC patch should be missing tests, how about making D114419 to the parent of this patch and leaving an error check there to be fixed in this patch?

A reasonable concern. I also tried to do this. But while I was adding not to the run command and checking warning or error in the output of objdump, I felt maybe it is not so good. Without this patch, if we commit D114419 first, we certainly introduce regression although it will be fixed in this patch, but we still make some builds in bug state. So I think we should commit this patch first and then commit D114419. Adding an option to change the alignment for DWARF section in this file can let me cook some testcase for the change, but that also does not sound like a good idea.

Let me your thoughts. Thanks.

llvm/lib/MC/XCOFFObjectWriter.cpp

1103

Right. No sections after the dwarf section. But it is better to align the final section as loading contents from aligned address is preferred. This also align with previous behavior.

[shchenz]

address @Esme comments

[Esme]

LGTM. Thanks.
Please state in the summary that D114419 will test this patch when submitting.

llvm/lib/MC/XCOFFObjectWriter.cpp

struct CsectSectionEntry : public SectionEntry {
 
   virtual ~CsectSectionEntry() = default;
 };
 
 struct DwarfSectionEntry : public SectionEntry {
   // For DWARF section entry.
   std::unique_ptr<XCOFFSection> DwarfSect;
 
+  // For DWARF section, we must use real size in the section header. MemorySize
+  // is for the size the DWARF section occupies including paddings.
+  uint32_t MemorySize;
+
   DwarfSectionEntry(StringRef N, int32_t Flags,
                     std::unique_ptr<XCOFFSection> Sect)
-      : SectionEntry(N, Flags | XCOFF::STYP_DWARF), DwarfSect(std::move(Sect)) {
+      : SectionEntry(N, Flags | XCOFF::STYP_DWARF), DwarfSect(std::move(Sect)),
+        MemorySize(0) {
     assert(DwarfSect->MCSec->isDwarfSect() &&
            "This should be a DWARF section!");
     assert(N.size() <= XCOFF::NameSize && "section name too long");
     memcpy(Name, N.data(), N.size());
   }
 
   DwarfSectionEntry(DwarfSectionEntry &&s) = default;
 
void XCOFFObjectWriter::assignAddressesAndIndices(const MCAsmLayout &Layout) {
 
   // The address corrresponds to the address of sections and symbols in the
   // object file. We place the shared address 0 immediately after the
   // section header table.
   uint64_t Address = 0;
   // Section indices are 1-based in XCOFF.
   int32_t SectionIndex = 1;
   bool HasTDataSection = false;
+  uint32_t PaddingsBeforeDwarf = 0;
 
   for (auto *Section : Sections) {
     const bool IsEmpty =
         llvm::all_of(Section->Groups,
                      [](const CsectGroup *Group) { return Group->empty(); });
     if (IsEmpty)
       continue;
 
for (auto *Section : Sections) {
     }
 
     // Make sure the address of the next section aligned to
     // DefaultSectionAlign.
     Address = alignTo(Address, DefaultSectionAlign);
     Section->Size = Address - Section->Address;
   }
 
+  // Start to generate DWARF sections. Sections other than DWARF section use
+  // DefaultSectionAlign as the default alignment, while DWARF sections have
+  // their own alignments. If these two alignments are not the same, we need
+  // some paddings here and record the paddings bytes for FileOffsetToData
+  // calculation.
+  if (!DwarfSections.empty())
+    PaddingsBeforeDwarf =
+        alignTo(Address,
+                (*DwarfSections.begin()).DwarfSect->MCSec->getAlignment()) -
+        Address;
+
+  DwarfSectionEntry *LastDwarfSection = nullptr;
+
   for (auto &DwarfSection : DwarfSections) {
     assert((SectionIndex <= MaxSectionIndex) && "Section index overflow!");
 
     XCOFFSection &DwarfSect = *DwarfSection.DwarfSect;
     const MCSectionXCOFF *MCSec = DwarfSect.MCSec;
 
     // Section index.
     DwarfSection.Index = SectionIndex++;
for (auto &DwarfSection : DwarfSections) {
     // See writeSectionForDwarfSectionEntry().
     DwarfSection.Address = DwarfSect.Address =
         alignTo(Address, MCSec->getAlignment());
 
     // Section size.
     // For DWARF section, we must use the real size which may be not aligned.
     DwarfSection.Size = DwarfSect.Size = Layout.getSectionAddressSize(MCSec);
 
-    // Make the Address align to default alignment for follow section.
-    Address = alignTo(DwarfSect.Address + DwarfSect.Size, DefaultSectionAlign);
+    Address = DwarfSection.Address + DwarfSection.Size;
+
+    if (LastDwarfSection)
+      LastDwarfSection->MemorySize =
+          DwarfSection.Address - LastDwarfSection->Address;
+    LastDwarfSection = &DwarfSection;
+  }
+  if (LastDwarfSection) {
+    // Make the final DWARF section address align to the default section
+    // alignment for follow contents.
+    Address = alignTo(LastDwarfSection->Address + LastDwarfSection->Size,
+                      DefaultSectionAlign);
+    LastDwarfSection->MemorySize = Address - LastDwarfSection->Address;
   }
 
   SymbolTableEntryCount = SymbolTableIndex;
 
   // Calculate the RawPointer value for each section.
   uint64_t RawPointer =
       (is64Bit() ? (XCOFF::FileHeaderSize64 +
                     SectionCount * XCOFF::SectionHeaderSize64)
                  : (XCOFF::FileHeaderSize32 +
                     SectionCount * XCOFF::SectionHeaderSize32)) +
       auxiliaryHeaderSize();
 
   for (auto *Sec : Sections) {
     if (Sec->Index == SectionEntry::UninitializedIndex || Sec->IsVirtual)
       continue;
 
     Sec->FileOffsetToData = RawPointer;
     RawPointer += Sec->Size;
     if (RawPointer > MaxRawDataSize)
       report_fatal_error("Section raw data overflowed this object file.");
   }
 
-  for (auto &DwarfSection : DwarfSections) {
-    // Address of csect sections are always aligned to DefaultSectionAlign, but
-    // address of DWARF section are aligned to Section alignment which may be
-    // bigger than DefaultSectionAlign, need to execlude the padding bits.
-    RawPointer =
-        alignTo(RawPointer, DwarfSection.DwarfSect->MCSec->getAlignment());
+  // Increase the raw pointer for the padding bytes between csect sections and
+  // DWARF sections.
+  if (!DwarfSections.empty())
+    RawPointer += PaddingsBeforeDwarf;
 
+  for (auto &DwarfSection : DwarfSections) {
     DwarfSection.FileOffsetToData = RawPointer;
-    // Some section entries, like DWARF section size is not aligned, so
-    // RawPointer may be not aligned.
-    RawPointer += DwarfSection.Size;
-    // Make sure RawPointer is aligned.
-    RawPointer = alignTo(RawPointer, DefaultSectionAlign);
+
+    RawPointer += DwarfSection.MemorySize;
 
     assert(RawPointer <= MaxRawDataSize &&
            "Section raw data overflowed this object file.");
   }
 
   RelocationEntryOffset = RawPointer;
 }