[PATCH] D143739: [WinEH][NFC] Assert for dynamic alloca in WinEH BBs

[Phoebe Wang via Phabricator via llvm-commits]

pengfei updated this revision to Diff 524076.
pengfei marked an inline comment as done.
pengfei added a comment.

In D143739#4332486 <https://reviews.llvm.org/D143739#4332486>, @rnk wrote:

> Sorry for the delay, I was OOO, and David Majnemer has moved on to other projects. In the future, consider adding @hans for more visibility for WinEH related changes to help find a reviewer.
>
> I think maybe it would be best to handle this in WinEHPrepare, which is where we check if the third argument operand of the `catchpad` instruction is an alloca. We could check there that the alloca is static if it exists, and it might also be healthy to report a backend error if some non-null, non-alloca value exists. Currently we silently ignore the value if it is not an alloca. It would be consistent with our current behavior to silently ignore dynamic allocas there, if you don't want to go through the trouble of reporting an error.
>
> The next best place to report an error would be the verifier, but the validity of the arguments to the catchpad depend on the personality, so that would require teaching the verifier about more WinEH semantics.

Thanks for the guidances! Changed WinEHPrepare to ignore dynamic allocas.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143739

Files:
  llvm/lib/CodeGen/WinEHPrepare.cpp
  llvm/test/CodeGen/X86/win-catchpad.ll


Index: llvm/test/CodeGen/X86/win-catchpad.ll
===================================================================
--- llvm/test/CodeGen/X86/win-catchpad.ll
+++ llvm/test/CodeGen/X86/win-catchpad.ll
@@ -353,3 +353,37 @@
 ; X64-NEXT: .long   -1
 ; X64-NEXT: .long   "?catch$[[catchbb]]@?0?branch_to_normal_dest at 4HA"@IMGREL
 ; X64-NEXT: .long   1
+
+%struct.widget = type { i8**, i8*, [8 x i8] }
+
+ at global = external global %struct.widget
+
+;; Check no crash if catch object is a dynamic alloca.
+define dso_local void @baz() local_unnamed_addr #0 personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+bb:
+  br i1 poison, label %bb1, label %bb7
+
+bb1:                                              ; preds = %bb
+  %tmp = alloca i32, align 4
+  invoke void @foo()
+          to label %bb6 unwind label %bb2
+
+bb2:                                              ; preds = %bb1
+  %tmp3 = catchswitch within none [label %bb4] unwind label %bb8
+
+bb4:                                              ; preds = %bb2
+  %tmp5 = catchpad within %tmp3 [%struct.widget* @global, i32 0, i32* %tmp]
+  unreachable
+
+bb6:                                              ; preds = %bb1
+  unreachable
+
+bb7:                                              ; preds = %bb
+  ret void
+
+bb8:                                              ; preds = %bb2
+  %tmp9 = cleanuppad within none []
+  unreachable
+}
+
+declare dso_local void @foo() local_unnamed_addr
Index: llvm/lib/CodeGen/WinEHPrepare.cpp
===================================================================
--- llvm/lib/CodeGen/WinEHPrepare.cpp
+++ llvm/lib/CodeGen/WinEHPrepare.cpp
@@ -155,11 +155,13 @@
       HT.TypeDescriptor = cast<GlobalVariable>(TypeInfo->stripPointerCasts());
     HT.Adjectives = cast<ConstantInt>(CPI->getArgOperand(1))->getZExtValue();
     HT.Handler = CPI->getParent();
-    if (auto *AI =
-            dyn_cast<AllocaInst>(CPI->getArgOperand(2)->stripPointerCasts()))
+    HT.CatchObj.Alloca = nullptr;
+    Value *V = CPI->getArgOperand(2)->stripPointerCasts();
+    auto *AI = dyn_cast<AllocaInst>(V);
+    assert((AI || isa<ConstantPointerNull>(V)) && "must be alloc or null ptr");
+    // Silently ignore dynamic allocas.
+    if (AI && AI->isStaticAlloca())
       HT.CatchObj.Alloca = AI;
-    else
-      HT.CatchObj.Alloca = nullptr;
     TBME.HandlerArray.push_back(HT);
   }
   FuncInfo.TryBlockMap.push_back(TBME);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D143739.524076.patch
Type: text/x-patch
Size: 2409 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230521/33df5b87/attachment.bin>