[PATCH] D103202: [AIX] Add traceback ssp canary bit support

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2182
+  // the attributes are on.
+  return true;
+}
----------------
jsji wrote:
> jsji wrote:
> > hubert.reinterpretcast wrote:
> > > A patch like this one would need rather more tests to demonstrate that the insertion of the canary word is in a position that is compatible with where the unwinder and other stack-frame-walking tools would look.
> > I think this is chicken and egg issue: there is no SSP support in current libunwind support on AIX yet.
> > I would prefer we enable this bits first,  we can and will definitely do more test while enable this support in libunwind.
> Also, since there is NO *ABI* definition of SSP canary word position, I don't think we should assume that it is the compiler's job to ensure the compatibility of canary word, libunwind can do a better job in ensuring the compatibility with legacy compiler in such case.
It is the compiler's responsibility to ensure compatibility of the canary word so that the non-Clang-based IBM XL C++ stack unwinder continues to have whatever ability it did to unwind past C code (that just happens to be compiled with LLVM). Advanced debugging tools may also have an expectation about the positioning of the canary word.

As for the ability of libunwind itself to do anything reasonable with this bit, even if the specific positioning is not based on the IBM XL compiler, that there is a concise set of rules for the positioning is necessary. Enabling the bit without identifying such a concise set of rules in prose (that is, improving the documentation of the ABI) seems problematic to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103202