[PATCH] D137819: [XCOFF] support the overflow section.

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:944
+      // The field value will be 65535 (XCOFF::RelocOverflow).
+      Sec->RelocationCount = XCOFF::RelocOverflow;
+
----------------
nit : suggest moving the line after  the SecEntry.Index=3.  put assign the member of SecEntry together.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:980
+  // Calculate the file offset to the section data.
+  uint64_t CurrAddress = 0;
+  for (auto *Sec : Sections) {
----------------
we do not need to calculate the CurrAddress  if there is not Dwarf section. we can delete  the CurrAddress  and code of calculation of  CurrAddress ,  and get the last element of Sections  when calculating  the  padding before the dwarf section. 

for example :


```
if (!DwarfSections.empty()) {
   CsectSectionEntry * LastCsectSec = Sections.back();
   uint64_t LastCsectSecAddr = LastCsectSec ->Address + LastCsectSet ->Size;
  RawPointer +=
        alignTo(LastCsectSecAddr,
                (*DwarfSections.begin()).DwarfSect->MCSec->getAlignment()) -
        LastCsectSecAddr;
}

```




================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:1027
+          // primary section header.
+          OvrfloSec.FileOffsetToRelocations = Sec->FileOffsetToRelocations;
+        }
----------------
DiggerLin wrote:
> I can not find "the s_relptr must have the same values as in the corresponding primary section header." in the xcoff document.
> 
> s_relptr	Recognized for the .text , .data , .tdata, and STYP_DWARF sections only.
> so the field "s_relptr" in the overflow section should be set to zero?
sorry I found it . please ignore above comment.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:1193
 
-  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.");
-  }
-
-  // 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;
-
-    RawPointer += DwarfSection.MemorySize;
-
-    assert(RawPointer <= MaxRawDataSize &&
-           "Section raw data overflowed this object file.");
-  }
-
-  RelocationEntryOffset = RawPointer;
+  EndOfAllHeadersOffset = RawPointer;
 }
----------------
since you change the caculation logic of  the RawPointer and EndOfAllHeadersOffset in the patch. I suggest to  delete the code from line 1185~1193 and 

change 


```
  uint64_t RawPointer = EndOfAllHeadersOffset;
```

to


```
 uint64_t RawPointer =
      (is64Bit() ? (XCOFF::FileHeaderSize64 +
                    SectionCount * XCOFF::SectionHeaderSize64)
                 : (XCOFF::FileHeaderSize32 +
                    SectionCount * XCOFF::SectionHeaderSize32)) +
      auxiliaryHeaderSize();
```
 
at  the begine of function void XCOFFObjectWriter::finalizeSectionInfo()

and we can delete the variable  "EndOfAllHeadersOffset" from the patch.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137819/new/

https://reviews.llvm.org/D137819