[Windows SEH]: Fix -O2 crash for Windows -EHa

[tentzen]

Summary

This patch fixes a Windows -EHa crash induced by previous commit 797ad701522988e212495285dade8efac41a24d4.
The crash was caused by "LifetimeMarker" scope (with option -O2) that should not be considered as SEH Scope.

This change also turns off -fasync-exceptions by default under -EHa option for now.

[aganea]

Thanks for the quick fix! Would you mind fixing the two failing tests please? (see above)

[zahiraam]

This comment has been deleted.

clang/lib/Driver/ToolChains/Clang.cpp

7155

Not really sure I understand this change. Isn't the case that if I compile with -EHa, I want the -fasync-exceptions option to be added to my options?

[tentzen]

In D103664#2798732, @aganea wrote:

Thanks for the quick fix! Would you mind fixing the two failing tests please? (see above)

Hmm, I cannot repro locally..

@zahiraam, currently -EHa is not completely ready yet. we need a couple of more patches. So turn it off to preserve zero-diff, unless that -fasync-exceptions is explicitly specified.

[zahiraam]

In D103664#2799725, @tentzen wrote:

In D103664#2798732, @aganea wrote:

Thanks for the quick fix! Would you mind fixing the two failing tests please? (see above)

Hmm, I cannot repro locally..

@zahiraam, currently -EHa is not completely ready yet. we need a couple of more patches. So turn it off to preserve zero-diff, unless that -fasync-exceptions is explicitly specified.

Got it. Thanks.
I was able to repro the fails. And this is the fix for it:
diff --git a/clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp b/clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp
index 47117b4a9613..b9195a2920e6 100644

• a/clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp

+++ b/clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp
@@ -1,60 +1,54 @@
// 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: invoke void @llvm.seh.scope.begin()
- CHECK: invoke void @llvm.seh.scope.begin()
- CHECK: invoke void @llvm.seh.scope.begin()
- CHECK: invoke void @llvm.seh.scope.end()
- CHECK: invoke void @llvm.seh.scope.end()
- CHECK: invoke void @llvm.seh.scope.end()
+// RUN: %clang_cc1 -triple x86_64-windows -fasync-exceptions -fcxx-exceptions -fexceptions -fms-extensions -x c++ -Wno-implicit-function-declaration -O2 -S -emit-llvm %s -o - | FileCheck %s

CHECK: invoke void @llvm.seh.try.begin()
- CHECK: %[[src:[0-9-]+]] = load volatile i32, i32* %i
- CHECK-NEXT: invoke void @"?crash@@YAXH@Z"(i32 %[[src]])
+ CHECK: = load volatile i32, i32* %i
+ CHECK-NEXT: invoke void @"?crash@@YAXH@Z"(i32
CHECK: invoke void @llvm.seh.try.end()

I was able to repro the fail and this is the fix for it.

[tentzen]

@zahiraam, are you removing all those CHECKs:

• CHECK: invoke void @llvm.seh.scope.**

those are placed there to ensure SEH scope semantic is preserved for Od..

[zahiraam]

In D103664#2799852, @tentzen wrote:

@zahiraam, are you removing all those CHECKs:

• CHECK: invoke void @llvm.seh.scope.**

those are placed there to ensure SEH scope semantic is preserved for Od..

I was just showing you. The checks generated at O2 are only the ones that I have left. At O0 the checks need to stay. So you need to rewrite the LIT tests so that both are expected for one option and the other.

[tentzen]

since I cannot repro it locally, let's have this patch in to resolve -EHa -O2 crashes for now.
I will add more -O2 tests in following patches.

[zahiraam]

In D103664#2799944, @tentzen wrote:

since I cannot repro it locally, let's have this patch in to resolve -EHa -O2 crashes for now.
I will add more -O2 tests in following patches.

Sounds good to me!

clang/lib/CodeGen/CGCleanup.cpp

void *EHScopeStack::pushCleanup(CleanupKind Kind, size_t Size) {
   if (IsNormalCleanup)
     InnermostNormalCleanup = stable_begin();
   if (IsEHCleanup)
     InnermostEHScope = stable_begin();
   if (IsLifetimeMarker)
     Scope->setLifetimeMarker();
 
   // With Windows -EHa, Invoke llvm.seh.scope.begin() for EHCleanup
-  if (CGF->getLangOpts().EHAsynch && IsEHCleanup &&
+  if (CGF->getLangOpts().EHAsynch && IsEHCleanup && !IsLifetimeMarker &&
       CGF->getTarget().getCXXABI().isMicrosoft())
     CGF->EmitSehCppScopeBegin();
 
   return Scope->getCleanupBuffer();
 }
 
 void EHScopeStack::popCleanup() {
   assert(!empty() && "popping exception stack when not empty");

clang/lib/Driver/ToolChains/Clang.cpp

void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType,
   }
 
   const Driver &D = getToolChain().getDriver();
   EHFlags EH = parseClangCLEHFlags(D, Args);
   if (!isNVPTX && (EH.Synch || EH.Asynch)) {
     if (types::isCXX(InputType))
       CmdArgs.push_back("-fcxx-exceptions");
     CmdArgs.push_back("-fexceptions");
-    if (EH.Asynch)
-      CmdArgs.push_back("-fasync-exceptions");
   }
   if (types::isCXX(InputType) && EH.Synch && EH.NoUnwindC)
     CmdArgs.push_back("-fexternc-nounwind");
 
   // /EP should expand to -E -P.
   if (Args.hasArg(options::OPT__SLASH_EP)) {
     CmdArgs.push_back("-E");
     CmdArgs.push_back("-P");