[PATCH] D100132: [libunwind][AIX] implementation of the unwinder for AIX

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm added inline comments.


================
Comment at: libunwind/src/UnwindCursor.hpp:2080
+    if (TBTable->tb.name_present)
+      charPtr = charPtr + *((uint16_t *)charPtr) + sizeof(uint16_t);
+
----------------
Why is the logic for skipping the name here not the same as above?  The handling of XL frames bumps the pointer by the name_len.


================
Comment at: libunwind/src/UnwindCursor.hpp:2088
+    if (TBTable->tb.has_vec)
+      // Note struct vec_ext does exist at this point because whether the
+      // ordering of longtbtable and has_vec bits is correct or not, both
----------------
I don't think this comment is necessary because this branch is for non XL tbtables, which should not mix up the has_vec and longtbtable bits.


================
Comment at: libunwind/src/UnwindCursor.hpp:2099
+    if ((*charPtr & xTBTableMask::ehInfoBit) &&
+        !(*charPtr & xTBTableMask::reservedBit)) {
+      // eh_info is available.
----------------
I think it's unnecessary to check the reservedBit here because this code should only be handling clang frames at this point that don't mix up the vec_ext and longtbtable bits.

The details of the has_vec and longtbtable bit mix up by the XL compiler can then be pushed into the v0 XL personality routine where the complexity belongs.  I don't think the check or comments are necessary here.


================
Comment at: libunwind/src/UnwindCursor.hpp:2324
+                               (void *)newRegisters.getRegister(2));
+    if (firstInstruction == loadTOCRegInst) {
+      _LIBUNWIND_TRACE_UNWINDING("Set gpr2=%p from frame\n",
----------------
Just for my understanding, is this the first instruction of the glink forwarding stub that the linker inserts for toc save/restore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100132