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

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked 4 inline comments as done.
sfertile added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:477
+  if (auto EC = Binary::checkOffset(
+          Obj->Data, reinterpret_cast<uintptr_t>(Obj->base()) + Offset, 4))
+    return XCOFFStringTable{0, nullptr};
----------------
hubert.reinterpretcast wrote:
> I would much prefer to perform the addition before the cast.
Good catch, updated.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:155
+
+    // TODO Need to add overflow handling (when NumberOfX == STYP_OVRFLO).
+    W.printNumber("NumberOfRelocations", Sec.NumberOfRelocations);
----------------
hubert.reinterpretcast wrote:
> s/STYP_OVRFLO/_OVERFLOW_MARKER/;
> 
> There's no `_OVERFLOW_MARKER` in the code currently, so we might want to add an appropriate named constant.
Updated the comment.

 I didn't add the named constant yet because its not used in the code anywhere and I though expressing how there are 32-bit and 64-bit values for `_OVERFLOW_MARKER` with the 64-bit value (IIUC) not actually representing overflow difficult to do without the overflow handling code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63843