[clang] [WebAssembly] Refactor Wasm EH/SjLj error checking (PR #122466)
Heejin Ahn via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 10 06:46:44 PST 2025
https://github.com/aheejin created https://github.com/llvm/llvm-project/pull/122466
There were many overlaps between error checking and feature enabling routines for Wasm EH and Wasm SjLj. This tries to factor out those common routines in separate lambda functions.
This is not NFC because this ends up disallowing a few new combinations (e.g. `-fwasm-exceptions` and `-emscripten-cxx-exceptions-allowed`), and also deletes `-mllvm` from the error messages to share the same lambda function between options with `-mllvm` and those without it.
This adds a few more tests but does not try to cover every single possible disallowed combination.
>From 859cd4e484ae39e0d044fe8fb894f69f20a80203 Mon Sep 17 00:00:00 2001
From: Heejin Ahn <aheejin at gmail.com>
Date: Fri, 10 Jan 2025 10:53:56 +0000
Subject: [PATCH] [WebAssembly] Refactor Wasm EH/SjLj error checking
There were many overlaps between error checking and feature enabling
routines for Wasm EH and Wasm SjLj. This tries to factor out those
common routines in separate lambda functions.
This is not NFC because this ends up disallowing a few new combinations
(e.g. `-fwasm-exceptions` and `-emscripten-cxx-exceptions-allowed`), and
also deletes `-mllvm` from the error messages to share the same lambda
function between options with `-mllvm` and those without it.
This adds a few more tests but does not try to cover every single
possible disallowed combination.
---
clang/lib/Driver/ToolChains/WebAssembly.cpp | 111 +++++++-------------
clang/test/Driver/wasm-toolchain.c | 53 ++++++----
2 files changed, 69 insertions(+), 95 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 44a6894d30fb29..e338b0d2398e04 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -344,44 +344,53 @@ void WebAssembly::addClangTargetOptions(const ArgList &DriverArgs,
}
}
- if (DriverArgs.getLastArg(options::OPT_fwasm_exceptions)) {
- // '-fwasm-exceptions' is not compatible with '-mno-exception-handling'
+ // Bans incompatible options for Wasm EH / SjLj. We don't allow using
+ // different modes for EH and SjLj.
+ auto BanIncompatibleOptionsForWasmEHSjLj = [&](StringRef CurOption) {
if (DriverArgs.hasFlag(options::OPT_mno_exception_handing,
options::OPT_mexception_handing, false))
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-fwasm-exceptions"
- << "-mno-exception-handling";
- // '-fwasm-exceptions' is not compatible with
- // '-mllvm -enable-emscripten-cxx-exceptions'
- for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
- if (StringRef(A->getValue(0)) == "-enable-emscripten-cxx-exceptions")
- getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-fwasm-exceptions"
- << "-mllvm -enable-emscripten-cxx-exceptions";
- }
- // '-fwasm-exceptions' implies exception-handling feature
- CC1Args.push_back("-target-feature");
- CC1Args.push_back("+exception-handling");
- // Backend needs -wasm-enable-eh to enable Wasm EH
- CC1Args.push_back("-mllvm");
- CC1Args.push_back("-wasm-enable-eh");
-
- // New Wasm EH spec (adopted in Oct 2023) requires multivalue and
- // reference-types.
+ << CurOption << "-mno-exception-handling";
+ // The standardized Wasm EH spec requires multivalue and reference-types.
if (DriverArgs.hasFlag(options::OPT_mno_multivalue,
- options::OPT_mmultivalue, false)) {
+ options::OPT_mmultivalue, false))
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-fwasm-exceptions" << "-mno-multivalue";
- }
+ << CurOption << "-mno-multivalue";
if (DriverArgs.hasFlag(options::OPT_mno_reference_types,
- options::OPT_mreference_types, false)) {
+ options::OPT_mreference_types, false))
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-fwasm-exceptions" << "-mno-reference-types";
+ << CurOption << "-mno-reference-types";
+
+ for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
+ for (const auto *Option :
+ {"-enable-emscripten-cxx-exceptions", "-enable-emscripten-sjlj",
+ "-emscripten-cxx-exceptions-allowed"}) {
+ if (StringRef(A->getValue(0)) == Option)
+ getDriver().Diag(diag::err_drv_argument_not_allowed_with)
+ << CurOption << Option;
+ }
}
+ };
+
+ // Enable necessary features for Wasm EH / SjLj in the backend.
+ auto EnableFeaturesForWasmEHSjLj = [&]() {
+ CC1Args.push_back("-target-feature");
+ CC1Args.push_back("+exception-handling");
+ // The standardized Wasm EH spec requires multivalue and reference-types.
CC1Args.push_back("-target-feature");
CC1Args.push_back("+multivalue");
CC1Args.push_back("-target-feature");
CC1Args.push_back("+reference-types");
+ // Backend needs '-exception-model=wasm' to use Wasm EH instructions
+ CC1Args.push_back("-exception-model=wasm");
+ };
+
+ if (DriverArgs.getLastArg(options::OPT_fwasm_exceptions)) {
+ BanIncompatibleOptionsForWasmEHSjLj("-fwasm-exceptions");
+ EnableFeaturesForWasmEHSjLj();
+ // Backend needs -wasm-enable-eh to enable Wasm EH
+ CC1Args.push_back("-mllvm");
+ CC1Args.push_back("-wasm-enable-eh");
}
for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
@@ -413,53 +422,11 @@ void WebAssembly::addClangTargetOptions(const ArgList &DriverArgs,
}
}
- if (Opt.starts_with("-wasm-enable-sjlj")) {
- // '-mllvm -wasm-enable-sjlj' is not compatible with
- // '-mno-exception-handling'
- if (DriverArgs.hasFlag(options::OPT_mno_exception_handing,
- options::OPT_mexception_handing, false))
- getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-mllvm -wasm-enable-sjlj"
- << "-mno-exception-handling";
- // '-mllvm -wasm-enable-sjlj' is not compatible with
- // '-mllvm -enable-emscripten-cxx-exceptions'
- // because we don't allow Emscripten EH + Wasm SjLj
- for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
- if (StringRef(A->getValue(0)) == "-enable-emscripten-cxx-exceptions")
- getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-mllvm -wasm-enable-sjlj"
- << "-mllvm -enable-emscripten-cxx-exceptions";
+ for (const auto *Option : {"-wasm-enable-eh", "-wasm-enable-sjlj"}) {
+ if (Opt.starts_with(Option)) {
+ BanIncompatibleOptionsForWasmEHSjLj(Option);
+ EnableFeaturesForWasmEHSjLj();
}
- // '-mllvm -wasm-enable-sjlj' is not compatible with
- // '-mllvm -enable-emscripten-sjlj'
- for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
- if (StringRef(A->getValue(0)) == "-enable-emscripten-sjlj")
- getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-mllvm -wasm-enable-sjlj"
- << "-mllvm -enable-emscripten-sjlj";
- }
- // '-mllvm -wasm-enable-sjlj' implies exception-handling feature
- CC1Args.push_back("-target-feature");
- CC1Args.push_back("+exception-handling");
- // Backend needs '-exception-model=wasm' to use Wasm EH instructions
- CC1Args.push_back("-exception-model=wasm");
-
- // New Wasm EH spec (adopted in Oct 2023) requires multivalue and
- // reference-types.
- if (DriverArgs.hasFlag(options::OPT_mno_multivalue,
- options::OPT_mmultivalue, false)) {
- getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-mllvm -wasm-enable-sjlj" << "-mno-multivalue";
- }
- if (DriverArgs.hasFlag(options::OPT_mno_reference_types,
- options::OPT_mreference_types, false)) {
- getDriver().Diag(diag::err_drv_argument_not_allowed_with)
- << "-mllvm -wasm-enable-sjlj" << "-mno-reference-types";
- }
- CC1Args.push_back("-target-feature");
- CC1Args.push_back("+multivalue");
- CC1Args.push_back("-target-feature");
- CC1Args.push_back("+reference-types");
}
}
}
diff --git a/clang/test/Driver/wasm-toolchain.c b/clang/test/Driver/wasm-toolchain.c
index 7c26c2c13c0baf..84c1b4f6efe662 100644
--- a/clang/test/Driver/wasm-toolchain.c
+++ b/clang/test/Driver/wasm-toolchain.c
@@ -120,18 +120,12 @@
// RUN: | FileCheck -check-prefix=EMSCRIPTEN_EH_ALLOWED_WO_ENABLE %s
// EMSCRIPTEN_EH_ALLOWED_WO_ENABLE: invalid argument '-mllvm -emscripten-cxx-exceptions-allowed' only allowed with '-mllvm -enable-emscripten-cxx-exceptions'
-// '-fwasm-exceptions' sets +exception-handling, -multivalue, -reference-types
-// and '-mllvm -wasm-enable-eh'
+// '-fwasm-exceptions' sets +exception-handling, -multivalue, -reference-types,
+// "-exception-model=wasm", and '-mllvm -wasm-enable-eh'
// RUN: %clang -### --target=wasm32-unknown-unknown \
// RUN: --sysroot=/foo %s -fwasm-exceptions 2>&1 \
// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS %s
-// WASM_EXCEPTIONS: "-cc1" {{.*}} "-target-feature" "+exception-handling" "-mllvm" "-wasm-enable-eh" "-target-feature" "+multivalue" "-target-feature" "+reference-types"
-
-// '-fwasm-exceptions' not allowed with '-mno-exception-handling'
-// RUN: not %clang -### --target=wasm32-unknown-unknown \
-// RUN: --sysroot=/foo %s -fwasm-exceptions -mno-exception-handling 2>&1 \
-// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS_NO_EH %s
-// WASM_EXCEPTIONS_NO_EH: invalid argument '-fwasm-exceptions' not allowed with '-mno-exception-handling'
+// WASM_EXCEPTIONS: "-cc1" {{.*}} "-target-feature" "+exception-handling" "-target-feature" "+multivalue" "-target-feature" "+reference-types" "-exception-model=wasm" "-mllvm" "-wasm-enable-eh"
// '-fwasm-exceptions' not allowed with
// '-mllvm -enable-emscripten-cxx-exceptions'
@@ -139,7 +133,20 @@
// RUN: --sysroot=/foo %s -fwasm-exceptions \
// RUN: -mllvm -enable-emscripten-cxx-exceptions 2>&1 \
// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS_EMSCRIPTEN_EH %s
-// WASM_EXCEPTIONS_EMSCRIPTEN_EH: invalid argument '-fwasm-exceptions' not allowed with '-mllvm -enable-emscripten-cxx-exceptions'
+// WASM_EXCEPTIONS_EMSCRIPTEN_EH: invalid argument '-fwasm-exceptions' not allowed with '-enable-emscripten-cxx-exceptions'
+
+// '-fwasm-exceptions' not allowed with '-mllvm -enable-emscripten-sjlj'
+// RUN: not %clang -### --target=wasm32-unknown-unknown \
+// RUN: --sysroot=/foo %s -fwasm-exceptions \
+// RUN: -mllvm -enable-emscripten-sjlj 2>&1 \
+// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS_EMSCRIPTEN_SJLJ %s
+// WASM_EXCEPTIONS_EMSCRIPTEN_SJLJ: invalid argument '-fwasm-exceptions' not allowed with '-enable-emscripten-sjlj'
+
+// '-fwasm-exceptions' not allowed with '-mno-exception-handling'
+// RUN: not %clang -### --target=wasm32-unknown-unknown \
+// RUN: --sysroot=/foo %s -fwasm-exceptions -mno-exception-handling 2>&1 \
+// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS_NO_EH %s
+// WASM_EXCEPTIONS_NO_EH: invalid argument '-fwasm-exceptions' not allowed with '-mno-exception-handling'
// '-fwasm-exceptions' not allowed with '-mno-multivalue'
// RUN: not %clang -### --target=wasm32-unknown-unknown \
@@ -154,18 +161,11 @@
// WASM_EXCEPTIONS_NO_REFERENCE_TYPES: invalid argument '-fwasm-exceptions' not allowed with '-mno-reference-types'
// '-mllvm -wasm-enable-sjlj' sets +exception-handling, +multivalue,
-// +reference-types and '-exception-model=wasm'
+// +reference-types and '-exception-model=wasm'
// RUN: %clang -### --target=wasm32-unknown-unknown \
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj 2>&1 \
// RUN: | FileCheck -check-prefix=WASM_SJLJ %s
-// WASM_SJLJ: "-cc1" {{.*}} "-target-feature" "+exception-handling" "-exception-model=wasm" "-target-feature" "+multivalue" "-target-feature" "+reference-types"
-
-// '-mllvm -wasm-enable-sjlj' not allowed with '-mno-exception-handling'
-// RUN: not %clang -### --target=wasm32-unknown-unknown \
-// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj -mno-exception-handling \
-// RUN: 2>&1 \
-// RUN: | FileCheck -check-prefix=WASM_SJLJ_NO_EH %s
-// WASM_SJLJ_NO_EH: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mno-exception-handling'
+// WASM_SJLJ: "-cc1" {{.*}} "-target-feature" "+exception-handling" "-target-feature" "+multivalue" "-target-feature" "+reference-types" "-exception-model=wasm"
// '-mllvm -wasm-enable-sjlj' not allowed with
// '-mllvm -enable-emscripten-cxx-exceptions'
@@ -173,27 +173,34 @@
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj \
// RUN: -mllvm -enable-emscripten-cxx-exceptions 2>&1 \
// RUN: | FileCheck -check-prefix=WASM_SJLJ_EMSCRIPTEN_EH %s
-// WASM_SJLJ_EMSCRIPTEN_EH: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mllvm -enable-emscripten-cxx-exceptions'
+// WASM_SJLJ_EMSCRIPTEN_EH: invalid argument '-wasm-enable-sjlj' not allowed with '-enable-emscripten-cxx-exceptions'
// '-mllvm -wasm-enable-sjlj' not allowed with '-mllvm -enable-emscripten-sjlj'
// RUN: not %clang -### --target=wasm32-unknown-unknown \
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj \
// RUN: -mllvm -enable-emscripten-sjlj 2>&1 \
// RUN: | FileCheck -check-prefix=WASM_SJLJ_EMSCRIPTEN_SJLJ %s
-// WASM_SJLJ_EMSCRIPTEN_SJLJ: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mllvm -enable-emscripten-sjlj'
+// WASM_SJLJ_EMSCRIPTEN_SJLJ: invalid argument '-wasm-enable-sjlj' not allowed with '-enable-emscripten-sjlj'
+
+// '-mllvm -wasm-enable-sjlj' not allowed with '-mno-exception-handling'
+// RUN: not %clang -### --target=wasm32-unknown-unknown \
+// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj -mno-exception-handling \
+// RUN: 2>&1 \
+// RUN: | FileCheck -check-prefix=WASM_SJLJ_NO_EH %s
+// WASM_SJLJ_NO_EH: invalid argument '-wasm-enable-sjlj' not allowed with '-mno-exception-handling'
// '-mllvm -wasm-enable-sjlj' not allowed with '-mno-multivalue'
// RUN: not %clang -### --target=wasm32-unknown-unknown \
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj -mno-multivalue 2>&1 \
// RUN: | FileCheck -check-prefix=WASM_SJLJ_NO_MULTIVALUE %s
-// WASM_SJLJ_NO_MULTIVALUE: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mno-multivalue'
+// WASM_SJLJ_NO_MULTIVALUE: invalid argument '-wasm-enable-sjlj' not allowed with '-mno-multivalue'
// '-mllvm -wasm-enable-sjlj' not allowed with '-mno-reference-types'
// RUN: not %clang -### --target=wasm32-unknown-unknown \
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj \
// RUN: -mno-reference-types 2>&1 \
// RUN: | FileCheck -check-prefix=WASM_SJLJ_NO_REFERENCE_TYPES %s
-// WASM_SJLJ_NO_REFERENCE_TYPES: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mno-reference-types'
+// WASM_SJLJ_NO_REFERENCE_TYPES: invalid argument '-wasm-enable-sjlj' not allowed with '-mno-reference-types'
// RUN: %clang -### %s -fsanitize=address --target=wasm32-unknown-emscripten 2>&1 | FileCheck -check-prefix=CHECK-ASAN-EMSCRIPTEN %s
// CHECK-ASAN-EMSCRIPTEN: "-fsanitize=address"
More information about the cfe-commits
mailing list