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

[Jason Liu via Phabricator via llvm-commits]

jasonliu marked 2 inline comments as done.
jasonliu added inline comments.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:185
+  if (Obj.is64Bit()) {
+    if (AuxEntPtr->AuxType != XCOFF::AUX_FILE)
+      W.startLine() << "!Mismatched auxiliary type: ";
----------------
DiggerLin wrote:
> if (AuxEntPtr->AuxType != XCOFF::AUX_FILE ) , it should not be parsed as 
> XCOFF::AUX_FILE
> it may better to print out raw data  in the printSymbol()
I think it's the caller's responsibility to make sure they are passing in the right auxiliary type. So we should assume when we enter this function, we have the right auxiliary type here. So I modified it and made it an assert instead. 


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:233
+    W.printEnum("Auxiliary Type", static_cast<uint8_t>(AuxType),
+                makeArrayRef(SymAuxType));
+  } else {
----------------
DiggerLin wrote:
> if (AuxEntPtr->AuxType != XCOFF::AUX_CSECT) , it should not be parsed as
> XCOFF::AUX_CSECT  above
> it may better to print out raw data in the printSymbol()
Same above. I modified it to be an assertion instead.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:379
     // error information, print out the raw Auxiliary entry from 1st till
     // the last - 1. The last one must be a CSECT Auxiliary Entry.
     for (int i = 1; i < NumberOfAuxEntries; i++) {
----------------
DiggerLin wrote:
> this is only for the 32bits .
> " By convention, the csect auxiliary entry in an XCOFF32 file must be the last auxiliary entry for any external symbol that has more than one auxiliary entry"
> 
> for 64bit, it maybe look for the x_auxtype ==AUX_CSECT 
Good point. Updated the code. 


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

https://reviews.llvm.org/D85774