[SEH]:Fix assertion when try is used inside catch(...) block with /EHa

[jyu2]

Summary

Current assert wiht /EHa with
A single unwind edge may only enter one EH pad

invoke void @llvm.seh.try.begin()
        to label %invoke.cont1 unwind label %catch.dispatch2

IR:
%1 = catchpad within %0 [ptr null, i32 0, ptr null]

invoke void @llvm.seh.try.begin()
        to label %invoke.cont5 unwind label %catch.dispatch2

The problem is the invoke to llvm.seh.try.begin() missing "funclet"

Accodring: https://llvm.org/docs/LangRef.html#ob-funclet
If any "funclet" EH pads have been entered but not exited (per the
description in the EH doc), it is undefined behavior to execute a
call or invoke.

To fix the problem, when emit seh_try_begin, call EmitSehScope,
instead of calling EmitRuntimeCallOrInvoke for proper "funclet"
gerenration.

[efriedma]

clang/lib/CodeGen/CGException.cpp

650

Do we need to make the same change in EmitSEHTryStmt/ExitSEHTryStmt?

Is there some reason not to just call EmitSehTryScopeBegin here?

[jyu2]

clang/lib/CodeGen/CGException.cpp

650

EmitSehTryScopeBegin: it is emit seh.scope.begin

In here we want to emit seh.try.begin. call EmitSehSCope with different function.

For EmitSEHTryStmt/ExitSEHTryStmt if is for try /except/__finally and it is for C code. I thought about that. And tried some test(BTW, try and __try can not be in same construct), I don't see the problem. So I did not add that.

[efriedma]

clang/lib/CodeGen/CGException.cpp

650

Not sure I understand what you're saying about EmitSehTryScopeBegin. CGException.cpp:45 refers to "llvm.seh.try.begin". CGCleanup.cpp:1370 also refers to "llvm.seh.try.begin"

For EmitSEHTryStmt/ExitSEHTryStmt, I guess __try codegen can't generate a construct quite like the given testcase, so thta's fine.

[jyu2]

Address Eli's comment. Thanks for review!.

:-( Sorry, You are right!! It is seems EmitSehTryScopeBegin is not called anywhere.

Changed. Thanks for the review.

[efriedma]

LGTM... but I don't think the IR we're generating is really correct overall; see https://github.com/llvm/llvm-project/issues/62723

On a side-note, other open issues related to -EHa/__try:

https://github.com/llvm/llvm-project/issues/62606
D124642

[jyu2]

In D150340#4343915, @efriedma wrote:

LGTM... but I don't think the IR we're generating is really correct overall; see https://github.com/llvm/llvm-project/issues/62723

On a side-note, other open issues related to -EHa/__try:

https://github.com/llvm/llvm-project/issues/62606
D124642

Thanks Eli for the code review!! I will take look about missing seh.try.end in c++ try/catch part later. In term of D124642, I'd really want to look at IR for return inside try/finally, but for some reason, I an not build compiler with your patch: no member named 'setHasAddressTaken' in 'llvm::MachineBasicBlock'; did you mean 'hasAddressTaken'. I may missing some thing in my environment.

clang/lib/CodeGen/CGException.cpp

650

:-( Sorry, You are right!! It is seems EmitSehTryScopeBegin is not called anywhere.

Changed. Thanks for the review.

[efriedma]

I'd really want to look at IR for return inside try/finally, but for some reason, I an not build compiler with your patch: no member named 'setHasAddressTaken' in 'llvm::MachineBasicBlock'; did you mean 'hasAddressTaken'. I may missing some thing in my environment.

The patch is old; I was having trouble getting any review, and then I forgot about it. I'll try to find time to rebase.

clang/lib/CodeGen/CGCleanup.cpp

void CodeGenFunction::EmitCXXTemporary(const CXXTemporary *Temporary,
                                        QualType TempType,
                                        Address Ptr) {
   pushDestroy(NormalAndEHCleanup, Ptr, TempType, destroyCXXObject,
               /*useEHCleanup*/ true);
 }
 
 // Need to set "funclet" in OperandBundle properly for noThrow
 //       intrinsic (see CGCall.cpp)
-static void EmitSehScope(CodeGenFunction &CGF,
-                         llvm::FunctionCallee &SehCppScope) {
-  llvm::BasicBlock *InvokeDest = CGF.getInvokeDest();
-  assert(CGF.Builder.GetInsertBlock() && InvokeDest);
-  llvm::BasicBlock *Cont = CGF.createBasicBlock("invoke.cont");
+void CodeGenFunction::EmitSehScope(llvm::FunctionCallee &SehCppScope) {
+  llvm::BasicBlock *InvokeDest = getInvokeDest();
+  assert(Builder.GetInsertBlock() && InvokeDest);
+  llvm::BasicBlock *Cont = createBasicBlock("invoke.cont");
   SmallVector<llvm::OperandBundleDef, 1> BundleList =
-      CGF.getBundlesForFunclet(SehCppScope.getCallee());
-  if (CGF.CurrentFuncletPad)
-    BundleList.emplace_back("funclet", CGF.CurrentFuncletPad);
-  CGF.Builder.CreateInvoke(SehCppScope, Cont, InvokeDest, std::nullopt,
-                           BundleList);
-  CGF.EmitBlock(Cont);
+      getBundlesForFunclet(SehCppScope.getCallee());
+  if (CurrentFuncletPad)
+    BundleList.emplace_back("funclet", CurrentFuncletPad);
+  Builder.CreateInvoke(SehCppScope, Cont, InvokeDest, std::nullopt, BundleList);
+  EmitBlock(Cont);
 }
 
 // Invoke a llvm.seh.scope.begin at the beginning of a CPP scope for -EHa
 void CodeGenFunction::EmitSehCppScopeBegin() {
   assert(getLangOpts().EHAsynch);
   llvm::FunctionType *FTy =
       llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
   llvm::FunctionCallee SehCppScope =
       CGM.CreateRuntimeFunction(FTy, "llvm.seh.scope.begin");
-  EmitSehScope(*this, SehCppScope);
+  EmitSehScope(SehCppScope);
 }
 
 // Invoke a llvm.seh.scope.end at the end of a CPP scope for -EHa
 //   llvm.seh.scope.end is emitted before popCleanup, so it's "invoked"
 void CodeGenFunction::EmitSehCppScopeEnd() {
   assert(getLangOpts().EHAsynch);
   llvm::FunctionType *FTy =
       llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
   llvm::FunctionCallee SehCppScope =
       CGM.CreateRuntimeFunction(FTy, "llvm.seh.scope.end");
-  EmitSehScope(*this, SehCppScope);
+  EmitSehScope(SehCppScope);
 }
 
 // Invoke a llvm.seh.try.begin at the beginning of a SEH scope for -EHa
 void CodeGenFunction::EmitSehTryScopeBegin() {
   assert(getLangOpts().EHAsynch);
   llvm::FunctionType *FTy =
       llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
   llvm::FunctionCallee SehCppScope =
       CGM.CreateRuntimeFunction(FTy, "llvm.seh.try.begin");
-  EmitSehScope(*this, SehCppScope);
+  EmitSehScope(SehCppScope);
 }
 
 // Invoke a llvm.seh.try.end at the end of a SEH scope for -EHa
 void CodeGenFunction::EmitSehTryScopeEnd() {
   assert(getLangOpts().EHAsynch);
   llvm::FunctionType *FTy =
       llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
   llvm::FunctionCallee SehCppScope =
       CGM.CreateRuntimeFunction(FTy, "llvm.seh.try.end");
-  EmitSehScope(*this, SehCppScope);
+  EmitSehScope(SehCppScope);
 }

clang/lib/CodeGen/CGException.cpp

if (C->getExceptionDecl()) {
         TypeInfo = CGM.getCXXABI().getAddrOfCXXCatchHandlerType(
             CaughtType, C->getCaughtType());
       CatchScope->setHandler(I, TypeInfo, Handler);
     } else {
       // No exception decl indicates '...', a catch-all.
       CatchScope->setHandler(I, CGM.getCXXABI().getCatchAllTypeInfo(), Handler);
       // Under async exceptions, catch(...) need to catch HW exception too
       // Mark scope with SehTryBegin as a SEH __try scope
-      if (getLangOpts().EHAsynch)
-        EmitRuntimeCallOrInvoke(getSehTryBeginFn(CGM));
+      if (getLangOpts().EHAsynch) {
+        llvm::FunctionCallee SehCppScope = getSehTryBeginFn(CGM);
+        EmitSehScope(SehCppScope);
+      }
     }
   }
 }
 
 llvm::BasicBlock *
 CodeGenFunction::getEHDispatchBlock(EHScopeStack::stable_iterator si) {
   if (EHPersonality::get(*this).usesFuncletPads())
     return getFuncletEHDispatchBlock(si);

clang/lib/CodeGen/CodeGenFunction.h

public:
 
   void EmitCXXTemporary(const CXXTemporary *Temporary, QualType TempType,
                         Address Ptr);
 
   void EmitSehCppScopeBegin();
   void EmitSehCppScopeEnd();
   void EmitSehTryScopeBegin();
   void EmitSehTryScopeEnd();
+  void EmitSehScope(llvm::FunctionCallee &SehCppScope);
 
   llvm::Value *EmitLifetimeStart(llvm::TypeSize Size, llvm::Value *Addr);
   void EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr);
 
   llvm::Value *EmitCXXNewExpr(const CXXNewExpr *E);
   void EmitCXXDeleteExpr(const CXXDeleteExpr *E);
 
   void EmitDeleteCall(const FunctionDecl *DeleteFD, llvm::Value *Ptr,

clang/test/CodeGen/windows-seh-EHa-CppCatchDotDotDot.cpp

 // RUN: %clang_cc1 -triple x86_64-windows -fasync-exceptions -fcxx-exceptions -fexceptions -fms-extensions -x c++ -Wno-implicit-function-declaration -S -emit-llvm %s -o - | FileCheck %s
 
 // CHECK: define dso_local void @"?crash@@YAXH@Z
 // CHECK: invoke void @llvm.seh.try.begin()
 // CHECK: invoke void @llvm.seh.scope.begin()
 // CHECK: invoke void @llvm.seh.scope.end()
 
 // CHECK: %[[dst:[0-9-]+]] = catchswitch within none [label %catch] unwind to caller
 // CHECK: %[[dst1:[0-9-]+]] = catchpad within %[[dst]] [ptr null, i32 0, ptr null]
 // CHECK: "funclet"(token %[[dst1]])
 
+// CHECK: define dso_local void @"?bar@@YAXXZ
+// CHECK: invoke void @llvm.seh.try.begin()
+// CHECK: invoke void @_CxxThrowException
+// CHECK: %[[dst:[0-9-]+]] = catchpad within %0 [ptr null, i32 0, ptr null]
+// CHECK: invoke void @llvm.seh.try.begin() [ "funclet"(token %[[dst]]) ]
+
 // CHECK: invoke void @llvm.seh.try.begin()
 // CHECK: %[[src:[0-9-]+]] = load volatile i32, ptr %i
 // CHECK-NEXT: invoke void @"?crash@@YAXH@Z"(i32 noundef %[[src]])
 // CHECK: invoke void @llvm.seh.try.end()
 
+// CHECK: invoke void @llvm.seh.try.begin()
+// CHECK: invoke void @"?bar@@YAXXZ"()
+// CHECK: invoke void @llvm.seh.try.end()
+
 // *****************************************************************************
 // Abstract:     Test CPP catch(...) under SEH -EHa option
 
 void printf(...);
 int volatile *NullPtr = 0;
 void foo() {
   *NullPtr = 0;
 }
if (i == 1)
       *NullPtr = 0;
   } catch (...) {
     printf(" in catch(...) funclet \n");
     if (i == 1)
       throw(i);
   }
 }
 
+void bar() {
+  try {
+    throw 1;
+  } catch(...) {
+    try {
+      *NullPtr = 0;
+    } catch (...) {
+       throw 1;
+    }
+  }
+}
+
 int main() {
   for (int i = 0; i < 2; i++) {
     __try {
       crash(i);
     } __except (1) {
       printf(" Test CPP unwind: in except handler i = %d \n", i);
     }
   }
+  __try {
+   bar();
+  } __except (1) {
+    printf("Test CPP unwind: in except handler \n");
+  }
   return 0;
 }