[PATCH] D81585: [AIX][XCOFF] Decode trace back table information for xcoff object file for llvm-objdump -d

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:300
+  unsigned Int_Proc : 1;     ///< Set for internal procedures.
+  unsigned Has_Ctl : 1;      ///< Set if rtn has controlled Automatic Strogare.
+  unsigned TOCLess : 1;      ///< Set if this routine is TOC-less.
----------------
s/Strogare/Storage?


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:335
+  unsigned Fixup : 1;     ///< Set if code is generated fixup code.
+  unsigned FP_Saved : 6;  ///< umber of FPRs saved, max of 32.
+  static constexpr uint8_t Stores_BC_Mask = 0x80;
----------------
nit: miss a `n` for `number` .


================
Comment at: llvm/test/tools/llvm-objdump/XCOFF/disassemble-tracebacktable.test:8
+## Input/xcoff-func-tracebacktable.o Compiled with IBM XL C/C++ for AIX, V16.1.0
+## compiler command: xlc -qtls -o xcoff-section-headers.o -c test.c
+## bash> cat test.c
----------------
Is that supposed to be `xcoff-func-tracebacktable.o` to be consistent with line #7?


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:94
+
+bool objdump::isXCOFFTracebackTableBegin(ArrayRef<uint8_t> Bytes) {
+  // Tableback table begin with sizeof(long) bytes zero.
----------------
nit: 
s/isXCOFFTracebackTableBegin/doesXCOFFTracebackTableBegin ?


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:95
+bool objdump::isXCOFFTracebackTableBegin(ArrayRef<uint8_t> Bytes) {
+  // Tableback table begin with sizeof(long) bytes zero.
+  if (Bytes.size() == 4)
----------------
nit: s/Tableback table/Traceback table?


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:105
+                                         bool OnlyComment) {
+
+  size_t Start = OS.tell();
----------------
nit: extra blank line


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:256
+
+  
+  if (Tb.Int_Handler) {
----------------
nit: extra blank line;


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:289
+      Index += Len;
+    }
+  }
----------------
For line #276 - #289, can we do this?

```
      OS << "# Function Name: ";
      while (NameLen > 0) {
        int Len = NameLen >= 4 ? 4 : NameLen;
        CurBytes = Bytes.slice(Index, Len);
        formatTracebackTableOutput(CurBytes, Address + Index, OS);
        OS << StringRef((const char *)CurBytes.data(), Len) << "\n";
        NameLen -= Len;
        Index += Len;
      }
```


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1461
       bool DumpARMELFData = false;
+      bool IsXCOFFFunctionEntryPointSymbol =
+          Obj->isXCOFF() && Section.isText() && TracebackTable &&
----------------
The queries you use kinda mismatch with `IsXCOFFFunctionEntryPointSymbol `. They are more like checking if we are gonna "DumpTracebackTableForXCOFFFunction"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81585