[PATCH] D124680: [WinEHPrepare] Fix accidental truncation of EH funclets with GNUstep ObjC runtime

[Akira Hatanaka via Phabricator via llvm-commits]

ahatanak added subscribers: rjmccall, pete.
ahatanak added inline comments.


================
Comment at: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp:113
+    if (CI->hasFnAttr(Attribute::NoUnwind))
+      NewCI->addFnAttr(Attribute::NoUnwind);
 
----------------
rnk wrote:
> sgraenitz wrote:
> > efriedma wrote:
> > > Are all these functions actually nounwind?  Destroying objects can call into user code.
> > You mean all intrinsics lowered pre-ISel? Not sure, so far we observed the following ones to trigger truncation and from the IR I looked at they all were all declared nounwind:
> > objc_destroyWeak, objc_release, objc_retain, objc_retainAutoreleasedReturnValue, objc_storeStrong
> > 
> > Yes destruction likely calls back into user code. In C++ throwing from destructors is undefined behavior. Is that the case in ObjC as well?
> I think if the original intrinsic call is nounwind, it is safe to transfer that onto the lowered replacement call regardless of how it interacts with WinEH, other transforms will have already optimized assuming this instruction doesn't throw exceptions.
Retain and release methods under ARC aren't allowed to raise exceptions, so I assume `llvm.objc.retain` and `llvm.objc.release` don't raise exceptions either.

https://clang.llvm.org/docs/AutomaticReferenceCounting.html#retain-count-semantics

I guess the other methods are marked as `nounwind` for similar reasons.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124680