-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Phoebe Wang (phoebewang) ChangesFixes #62449 Full diff: https://github.com/llvm/llvm-project/pull/88101.diff 2 Files Affected:
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{}; }
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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. |
Oh, I think it's just the old |
e272c37 might have fixed this. (Didn't test locally, will check what the bot says.) |
Thanks a lot @nico! I see the bot is still red, but not failed on this test any more. |
Yes, #83124 broke it in the meantime 😅 |
Fixes #62449