[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Please upload patches with full context (-U1000000).



================
Comment at: clang/lib/CodeGen/CGException.cpp:1798
+
+    // if the parent is a _finally, need to retrive Establisher's FP,
+    //  2nd paramenter, saved & named frame_pointer in parent's frame
----------------
Maybe worth expanding this comment a little, to explain that a "finally" should have at most one localescape'ed variable.  (At least, that's my understanding.)


================
Comment at: clang/lib/CodeGen/CGException.cpp:1800
+    //  2nd paramenter, saved & named frame_pointer in parent's frame
+    if (ParentCGF.ParentCGF != NULL) {
+      // Locate and escape Parent's frame_pointer.addr alloca
----------------
nullptr

Do you actually use ParentCGF.ParentCGF anywhere?  If not, do you really need to save it?


================
Comment at: clang/lib/CodeGen/CGException.cpp:1805
+        llvm::AllocaInst *II = dyn_cast<llvm::AllocaInst>(&I);
+        if (II && II->getName().startswith("frame_pointer")) {
+          FramePtrAddrAlloca = II;
----------------
Using the name isn't reliable. You should be using data stored somewhere in the CodeGenFunction or something like that.


================
Comment at: clang/lib/CodeGen/CGException.cpp:1822
+      llvm::Function *FrameRecoverFn = llvm::Intrinsic::getDeclaration(
+          &CGM.getModule(), llvm::Intrinsic::localrecover);
+      llvm::Constant *ParentI8Fn =
----------------
Is there some reason you can't reuse recoverAddrOfEscapedLocal here?


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:370
 
+  // For outliner helper only
+  CodeGenFunction *ParentCGF = nullptr;
----------------
This comment isn't really helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77982