[PATCH] D89049: [AIX][XCOFF] print out the traceback info

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/tools/llvm-objdump/ObjdumpOpts.td:71
+def traceback_table : Flag<["--"], "traceback-table">,
+  HelpText<"Decode traceback table for disassembly. This "
+           "option is for XCOFF files only">;
----------------
jhenderson wrote:
> A thought: should `--traceback-table` imply `--disassemble-all`, like e.g. `--source` implies `--disassemble`?
> 
> (If you decide to do this, please make sure to mention it in the documentation and help text).

 good idea.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:143
+  printRawData(Bytes.slice(Index, 4), Address, OS, STI);
+  OS << "\t# Traceback table begin\n";
+  Index += 4;
----------------
jhenderson wrote:
> "begin" is a verb meaning "to start something". "start" is what you are looking for.
I think it maybe better to consistent with https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp#L1991

if you strong suggestion to change to "start" , I can add a NFC for both after the patch commit.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:193
+
+  // Print the first byte of 8 bytes of mandatory fields.
+  PrintBytes(1);
----------------
jhenderson wrote:
> Same below for each of the other comments like this.
thanks.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:283
+
+    assert(FunctionNameLen > 0 &&
+           "The length of the function name must be greater than zero.");
----------------
jhenderson wrote:
> Should this assertion be an actual error? In other words, if the function name length stored was actually 0 (in a malformed object), would this code get here?
I think I can keep  the assert here, If we want to check the "FunctionNameLen==0   and  isFuncNamePresent is true" malform. we can implement it in function XCOFFTracebackTable::XCOFFTracebackTable() as a new patch.(Current patch is big patch now).

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Object/XCOFFObjectFile.cpp#L1443 


  if (Cur && isFuncNamePresent()) {
    uint16_t FunctionNameLen = DE.getU16(Cur);
    if (Cur)
      FunctionName = DE.getBytes(Cur, FunctionNameLen);
  }



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89049