[PATCH] D146339: [StackProtector] attribute __stack_chk_fail as NoReturn

Thu Mar 23 11:12:59 PDT 2023

efriedma added inline comments.

================
Comment at: llvm/lib/CodeGen/StackProtector.cpp:611
+  B.CreateCall(StackChkFail, Args);
   B.CreateUnreachable();
   return FailBB;
----------------
nickdesaulniers wrote:
> @efriedma I wonder if we can avoid this unreachable? (maybe as a follow up commit to this)
> 
> I had tried it and a bunch of stack check related tests crashed; I didn't investigate, but suspect maybe some other code expects there to be an instruction after the call.
> 
> Or can you think of any reason why an unreachable needs to be added here?  Is there something implicit about noreturn functions needing to be followed by unreachable in IR?
> 
> (even if the child patches don't land, D146339 alone is still probably worthwhile to land)
Every basic block must end in a terminator, and call isn't a terminator, so we have to put something after the call.  "unreachable" is the terminator we use when we know control can't actually flow anywhere else in the function.  Not sure what you'd want to replace it with.

Yes, this patch makes sense even without the other changes.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146339