Skip to content

[SEH] Ignore async exception flag when the environment is not MSVC #88101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

phoebewang
Copy link
Contributor

Fixes #62449

@phoebewang phoebewang requested a review from rnk April 9, 2024 08:24
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Apr 9, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Phoebe Wang (phoebewang)

Changes

Fixes #62449


Full diff: https://github.com/llvm/llvm-project/pull/88101.diff

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+20-8)
  • (added) clang/test/Driver/windows-seh-async-verify.cpp (+24)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 766a9b91e3c0ad..c1ed4bd2dcda06 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -346,11 +346,14 @@ static bool addExceptionArgs(const ArgList &Args, types::ID InputType,
   bool EH = Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions,
                          false);
 
-  bool EHa = Args.hasFlag(options::OPT_fasync_exceptions,
-                          options::OPT_fno_async_exceptions, false);
-  if (EHa) {
-    CmdArgs.push_back("-fasync-exceptions");
-    EH = true;
+  // Async exceptions are Windows MSVC only.
+  if (Triple.isWindowsMSVCEnvironment()) {
+    bool EHa = Args.hasFlag(options::OPT_fasync_exceptions,
+                            options::OPT_fno_async_exceptions, false);
+    if (EHa) {
+      CmdArgs.push_back("-fasync-exceptions");
+      EH = true;
+    }
   }
 
   // Obj-C exceptions are enabled by default, regardless of -fexceptions. This
@@ -8084,7 +8087,8 @@ struct EHFlags {
 ///      The 'a' modifier is unimplemented and fundamentally hard in LLVM IR.
 /// - c: Assume that extern "C" functions are implicitly nounwind.
 /// The default is /EHs-c-, meaning cleanups are disabled.
-static EHFlags parseClangCLEHFlags(const Driver &D, const ArgList &Args) {
+static EHFlags parseClangCLEHFlags(const Driver &D, const ArgList &Args,
+                                   bool isWindowsMSVC) {
   EHFlags EH;
 
   std::vector<std::string> EHArgs =
@@ -8094,8 +8098,15 @@ static EHFlags parseClangCLEHFlags(const Driver &D, const ArgList &Args) {
       switch (EHVal[I]) {
       case 'a':
         EH.Asynch = maybeConsumeDash(EHVal, I);
-        if (EH.Asynch)
+        if (EH.Asynch) {
+          // Async exceptions are Windows MSVC only.
+          if (!isWindowsMSVC) {
+            EH.Asynch = false;
+            D.Diag(clang::diag::warn_drv_unused_argument) << "/EHa" << EHVal;
+            continue;
+          }
           EH.Synch = false;
+        }
         continue;
       case 'c':
         EH.NoUnwindC = maybeConsumeDash(EHVal, I);
@@ -8159,7 +8170,8 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType,
 
   const Driver &D = getToolChain().getDriver();
 
-  EHFlags EH = parseClangCLEHFlags(D, Args);
+  bool IsWindowsMSVC = getToolChain().getTriple().isWindowsMSVCEnvironment();
+  EHFlags EH = parseClangCLEHFlags(D, Args, IsWindowsMSVC);
   if (!isNVPTX && (EH.Synch || EH.Asynch)) {
     if (types::isCXX(InputType))
       CmdArgs.push_back("-fcxx-exceptions");
diff --git a/clang/test/Driver/windows-seh-async-verify.cpp b/clang/test/Driver/windows-seh-async-verify.cpp
new file mode 100644
index 00000000000000..5fda6a77dba049
--- /dev/null
+++ b/clang/test/Driver/windows-seh-async-verify.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang --target=x86_64-pc-windows -fasync-exceptions -fsyntax-only %s -### 2>&1 | FileCheck %s
+// RUN: %clang_cl --target=x86_64-pc-windows /EHa -fsyntax-only %s -### 2>&1 | FileCheck %s
+// RUN: %clang --target=x86_64-pc-windows-gnu -fasync-exceptions -fsyntax-only %s -### 2>&1 | FileCheck %s --check-prefixes=GNU-ALL,GNU
+// RUN: %clang_cl --target=x86_64-pc-windows-gnu /EHa -fsyntax-only %s -### 2>&1 | FileCheck %s --check-prefixes=GNU-ALL,CL-GNU
+
+// CHECK-NOT: warning
+// GNU: warning: argument unused during compilation: '-fasync-exceptions' [-Wunused-command-line-argument]
+// CL-GNU: warning: argument unused during compilation: '/EHa' [-Wunused-command-line-argument]
+
+// CHECK: -fasync-exceptions
+// GNU-ALL-NOT: -fasync-exceptions
+struct S {
+    union _Un {
+        ~_Un() {}
+        char _Buf[12];
+    };
+    _Un _un;
+};
+
+struct Embed {
+    S v2;
+};
+
+void PR62449() { Embed v{}; }

Copy link
Contributor

@MaxEW707 MaxEW707 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@phoebewang phoebewang merged commit b0194d2 into llvm:main Apr 16, 2024
@phoebewang phoebewang deleted the seh branch April 16, 2024 10:19
@nico
Copy link
Contributor

nico commented Apr 16, 2024

Looks like breaks check-clang on macOS: http://45.33.8.238/macm1/83520/step_6.txt

Please take a look and revert for now if it takes a while to fix.

@nico
Copy link
Contributor

nico commented Apr 16, 2024

Oh, I think it's just the old clang: warning: '/Users/thakis/src/llvm-project/clang/test/Driver/windows-seh-async-verify.cpp' treated as the '/U' option [-Wslash-u-filename]. I'll try pushing a fix.

@nico
Copy link
Contributor

nico commented Apr 16, 2024

e272c37 might have fixed this. (Didn't test locally, will check what the bot says.)

@phoebewang
Copy link
Contributor Author

Thanks a lot @nico! I see the bot is still red, but not failed on this test any more.

@nico
Copy link
Contributor

nico commented Apr 16, 2024

Yes, #83124 broke it in the meantime 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"parser at end of file" crash with -fasync-exceptions
5 participants