[PATCH] D63843: [Object][XCOFF] Add support for 64-bit file header and section header dumping.

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked 8 inline comments as done and an inline comment as not done.
sfertile added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:125
 void XCOFFObjectFile::moveSymbolNext(DataRefImpl &Symb) const {
+  assert(!is64Bit() && "Symbol table support not implemented for 64-bit.");
   const XCOFFSymbolEntry *SymEntPtr = toSymbolEntry(Symb);
----------------
sfertile wrote:
> DiggerLin wrote:
> > there are assert(!is64Bit() && "Symbol table support not implemented for 64-bit."); in the toSymbolEntry() , I do not think we need  assert(!is64Bit() && "Symbol table support not implemented for 64-bit."); here. 
> In the spirit of trying to keep this patch to a minimum I only implemented section headers and file-header support. I put defensive assert to indicate that I hadn't evaluated any of the symbol related interfaces for 64-bit. While this would technically work for 64-bit, that is only because the 64-bit symbol table entrey happens to also be 18 bytes of data. Casting a 64-bit symbol table entry to an `XCOFFSymbolEntry` is still wrong as they have different layouts.
Sorry Digger you are correct, I don't need to add an assert in any of the interfaces that cast a DataRefImpl to a SymbolEntry as the helper fucntion asserts. Updated.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:478
   uint64_t CurPtr = 0;
-
-  if ((EC = getObject(FileHdrPtr, Data, base() + CurPtr)))
+  if (EC = getObject(FileHeader, Data, base() + CurPtr, getFileHeaderSize()))
     return;
----------------
MaskRay wrote:
> `warning: using the result of an assignment as a condition without parentheses [-Wparentheses]`
> 
> I remember I said in the initial check-in of this file: lib/Object code is moving away from `std::error_code`, `ErrorOr` and `errorCodeToError`. `Error` `Expected` etc should be used instead.
Attempting to clean this up next.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63843