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

Wed Jun 9 19:30:29 PDT 2021

shchenz accepted this revision.
shchenz added a comment.
This revision is now accepted and ready to land.

LGTM, thanks.

================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2181
+  // There are cases that the stack protectors are not really inserted even if
+  // the attributes are on.
+  return true;
----------------
jsji wrote:
> hubert.reinterpretcast wrote:
> > shchenz wrote:
> > > Will it cause some issue in unwinder if we set the ssp canary bit in the traceback table but there is no canary word in the stack?
> > > 
> > > Could you please comment @xingxue @DiggerLin @hubert.reinterpretcast 
> > The `FIXME` indicates a problem (as is appropriate for a `FIXME`). Even if it is possible to recover from this by more complicated logic in the unwinder, the preference should be for it to be resolved by the compiler (pay for it at compile time). The patch does not have to be blocked by the pending `FIXME`, because the bit is only set when an internal/experimental flag is set.
> As I mentioned, there is NO unwinder support yet, so this is chicken and egg issue. We can fix this in a following patch if the unwinder will not able to handle it and complain. 
> 
> Also this is a very corner case -- with legacy XL, we set this bit on whenever the options is on, no checking, and we never hit problem -- so legacy unwinder can tolerate this.
> 
> 
OK, LGTM then.

We can revisit the impact of the redundant ssp canary bit later when unwinder supports this. And also this flag will not be set by default in the compiler.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103202