[PATCH] D66969: Output XCOFF object text section

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:382
+  // n_type
+  W.write<uint16_t>(0);
+  // n_sclass
----------------
Please add a comment about this value being always zero (for now). Again, this was better in the existing code before this patch.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:395
+  // Symbol type.
+  W.write<int8_t>(getEncodedType(CSectionRef.MCCsect));
+  // Storage mapping class.
----------------
Unsigned.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:397
+  // Storage mapping class.
+  W.write<int8_t>(CSectionRef.MCCsect->getMappingClass());
+  // Reserved (x_stab).
----------------
Unsigned.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:399
+  // Reserved (x_stab).
+  W.write<int32_t>(0);
+  // Reserved (x_snstab).
----------------
Unsigned.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:401
+  // Reserved (x_snstab).
+  W.write<int16_t>(0);
+}
----------------
Unsigned.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:488
+    Text.Index = SectionIndex++;
+    assert(Text.Index <= INT16_MAX && "Text section index overflow!");
+    uint32_t StartAddress = Address;
----------------
Technically this assert is too late. `INT_MAX` is allowed to be the same as `INT16_MAX`, so the addition may overflow even in the promoted type.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:506
+    assert(alignTo(Address, DefaultSectionAlign) == Address &&
+           "Improperly aligned address for section.");
+    Text.Size = Address - StartAddress;
----------------
The string text should probably be about the size not being properly padded to a multiple of the section alignment.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:516
     BSS.Index = SectionIndex++;
-    assert(alignTo(Address, DefaultSectionAlign) == Address &&
-           "Improperly aligned address for section.");
+    assert(BSS.Index <= INT16_MAX && "BSS section index overflow!");
     uint32_t StartAddress = Address;
----------------
Ditto regarding the assertion being too late.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66969