[PATCH] D81585: [AIX][XCOFF][Patch1] Provide decoding trace back table information API for xcoff object file for llvm-objdump -d

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:942
+#define GETBITWITHMASK(P, X)                                                   \
+  (support::endian::read32be(TBPtr + P) & TracebackTable::X)
+#define GETBITWITHMASKSHIFT(P, X, S)                                           \
----------------
hubert.reinterpretcast wrote:
> Use defensive parentheses for references to macro parameters.
The second set of parentheses I requested has not been added.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:944-945
+#define GETBITWITHMASKSHIFT(P, X, S)                                           \
+  (support::endian::read32be(TBPtr + P) & TracebackTable::X) >>                \
+      TracebackTable::S
+
----------------
hubert.reinterpretcast wrote:
> Use defensive parentheses for references to macro parameters and for the definition of the macro.
I requested three sets of parentheses that are still not present.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:934
+
+  // TODO: Need to parse vector info and extension table if there is.
+
----------------
Minor nit: s/is/is one/;


================
Comment at: llvm/unittests/Object/XCOFFObjectFileTest.cpp:179
+      FailedWithMessage(
+          "unexpected end of data at offset 0x9 while reading [0x8, 0xc)"));
+}
----------------
The `Size` should match the first value, right? Can we check that for each of the truncated cases?


================
Comment at: llvm/unittests/Object/XCOFFObjectFileTest.cpp:234
+
+TEST(XCOFFObjectFileTest, XCOFFTracebackTableTruncatedAtFuncitonName) {
+  uint64_t Size = 36;
----------------
s/Funciton/Function/;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81585