[PATCH] D85774: [XCOFF][AIX] Enable tooling support for 64 bit symbol table parsing

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I took a quick look at the pre-merge output. The message is that the object isn't recognised as a valid object file, not that it wasn't found, which suggests either the object or code is broken in some manner, or you're missing a dependent patch. Does this patch depend on another patch that isn't in main yet?



================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:357
     // If the symbol is C_FILE and has auxiliary entries...
     for (int i = 1; i <= NumberOfAuxEntries; i++) {
+      uintptr_t AuxAddress = XCOFFObjectFile::getAdvancedSymbolEntryAddress(
----------------
`i` -> `I`

(you're changing most of the loop body - you might as well fix this whilst you're here)


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:363
+          *Obj.getSymbolAuxType(AuxAddress) != XCOFF::SymbolAuxType::AUX_FILE) {
+        W.startLine() << "!Unexpected raw auxiliary entry data:\n";
+        W.startLine() << format_bytes(
----------------
Test case?


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:389-391
+    // For 32-bit object, print from 1st till the last - 1. The last one must be
+    // a CSECT Auxiliary Entry.
+    // For 64-bit object, print from 1st till last and skips if SymbolAuxType is
----------------
My English language ping went off at the word "till" in these two sentences. I'd probably change it to "to". Also, use "first" rather than "1st", I suggest in both places.

Also "skips" -> "skip" for grammatical consistency.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:393
+    // AUX_CSECT.
+    for (int i = 1; i <= NumberOfAuxEntries; i++) {
+      if (i == NumberOfAuxEntries && !Obj.is64Bit())
----------------
`i` -> `I`


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

https://reviews.llvm.org/D85774