[PATCH] D68575: implement parsing overflow section header.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/xcoff-overflow-section.test:37
+# SECOVERFLOW-NEXT:     Name: .ovrflo
+# SECOVERFLOW-NEXT:     NumberOfRelocations: 0x3
+# SECOVERFLOW-NEXT:     NumberOfLineNumbers: 0x0
----------------
The same information, when conveyed not via an overflow header, is expressed in decimal format (and not hexadecimal).


================
Comment at: llvm/test/tools/llvm-readobj/xcoff-overflow-section.test:38
+# SECOVERFLOW-NEXT:     NumberOfRelocations: 0x3
+# SECOVERFLOW-NEXT:     NumberOfLineNumbers: 0x0
+# SECOVERFLOW-NEXT:     Size: 0x0
----------------
Ditto.


================
Comment at: llvm/test/tools/llvm-readobj/xcoff-overflow-section.test:41
+# SECOVERFLOW-NEXT:     RawDataOffset: 0x0
+# SECOVERFLOW-NEXT:     RelocationPointer: 0x0
+# SECOVERFLOW-NEXT:     LineNumberPointer: 0x0
----------------
According to the XCOFF documentation: The `s_relptr` and `s_lnnoptr` fields must have the same values as in the corresponding primary section header.


================
Comment at: llvm/test/tools/llvm-readobj/xcoff-overflow-section.test:42
+# SECOVERFLOW-NEXT:     RelocationPointer: 0x0
+# SECOVERFLOW-NEXT:     LineNumberPointer: 0x0
+# SECOVERFLOW-NEXT:     IndexOfSectionOverflowed: 2
----------------
See above.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:437
 
+    uint16_t Flags = Sec.Flags & 0xffffu;
+    switch (Flags) {
----------------
Can we encode this into a function (`getSectionType(const T &Section)`) or constant (`SectionFlagsTypeMask`) with the comment:
The low order 16 bits of s_flags denotes the section type.

Also, the variable should be named `SectionType` or similar.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:441
+      if (Obj.is64Bit())
+        W.printString(
+            "64 bits XCOFF objectfile should not have overflow section!");
----------------
I think we should not be printing a "warning" to the same stream as the output.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:442
+        W.printString(
+            "64 bits XCOFF objectfile should not have overflow section!");
+      printOverflowSectionHeader(Sec);
----------------
"64-bit XCOFF object file".


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:448
+    case XCOFF::STYP_TYPCHK:
+      // TODO : The interpret of loader, exception, type check section header
+      // are different from generic section header. We will implement them
----------------
s/TODO : /TODO /;
s/interpret/interpretation/;
s/, type check section header/, and type check section headers/;
s/from/from that of/;
s/generic section header/generic section headers/;
s/generic seciton header now/generic section headers for now/;


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:456
     // The most significant 16-bits represent the DWARF section subtype. For
     // now we just dump the section type flags.
     if (Flags & SectionFlagsReservedMask)
----------------
Since we removed the context of the code line from here, the comment is ambiguous. This should say that, for now, we dump only the section type (low order 16 bits).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68575