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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-objdump.rst:440
+
+  Decode traceback table for disassembly output.
+
----------------



================
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">;
----------------
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).


================
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;
----------------
"begin" is a verb meaning "to start something". "start" is what you are looking for.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:179
+
+    // Print all remaining zero as ...
+    if (Size - LastNonZero >= 8)
----------------



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


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:283
+
+    assert(FunctionNameLen > 0 &&
+           "The length of the function name must be greater than zero.");
----------------
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?


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:394
+#undef PRINTGET
+#undef SPLIT
+#undef PRINTOPTIONAL
----------------
Redundant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89049