[PATCH] D104639: [AIX][XCOFF] Add support for 64-bit file header and section header writing

[Sean Fertile via Phabricator via llvm-commits]

sfertile added a comment.

In D104639#2954342 <https://reviews.llvm.org/D104639#2954342>, @MaryamBen wrote:

> I tried to make changes you told me to see if I could make it in time. Because tomorow is my last day at work,  I don't have enough time to redo all the patchs especially the tests.

Sorry I couldn't get these reviewed and landed in time before your internship finished. Thank you for all the work you did, It is a pretty impressive the amount you got accomplished.

In D104639#2954342 <https://reviews.llvm.org/D104639#2954342>, @MaryamBen wrote:

> When I applied your recommendations, I got the error "The end of the file was unexpectedly encountered". I think it would require more time to do things properly.

Your right, its because we are writing values into the file-header or section header table that implies there is following info (like a symbol table offset and non-zero count). I've tested setting those fields to 0 (as well as offsets to relocations and section data) and then we can dump the file headers and section header table. For existing lit tests that dump symbol tables or section contents I would expect llvm-readobj to give an error as those parts are missing from the object file.

In D104639#2954342 <https://reviews.llvm.org/D104639#2954342>, @MaryamBen wrote:

> Otherwise, there are tests for the file header and the section header in further patches.

Each patch that adds/alters functionality has to be accompanied by testing of said functionality.  We can add further testing in subsequent patches but only patches that contain no function changes are allowed to be committed without new testing (or updated testing).



================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:58
   uint32_t SymbolTableIndex;
-  uint32_t FixupOffsetInCsect;
+  uint64_t FixupOffsetInCsect;
   uint8_t SignAndSize;
----------------
I don't see any need to update this structure until we start emitting relocations.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:78
   const MCSectionXCOFF *const MCCsect;
-  uint32_t SymbolTableIndex;
-  uint32_t Address;
-  uint32_t Size;
+  uint64_t SymbolTableIndex;
+  uint64_t Address;
----------------
Isn't the 64-bit object format also limited to a 4 byte symbol table index?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104639