[clang] [WebAssembly] Refactor Wasm EH/SjLj error checking (PR #122466)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 10 06:47:16 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Heejin Ahn (aheejin)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/122466.diff
2 Files Affected:
- (modified) clang/lib/Driver/ToolChains/WebAssembly.cpp (+39-72)
- (modified) clang/test/Driver/wasm-toolchain.c (+30-23)
``````````diff
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"
``````````
</details>
https://github.com/llvm/llvm-project/pull/122466
More information about the cfe-commits
mailing list