[clang] 3fbc344 - [WebAssembly] Refactor Wasm EH/SjLj error checking (#122466)

via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 10 13:06:00 PST 2025


Author: Heejin Ahn
Date: 2025-01-10T13:05:57-08:00
New Revision: 3fbc344b49800bb0f70fd5af46c0a47f6d55bbd1

URL: https://github.com/llvm/llvm-project/commit/3fbc344b49800bb0f70fd5af46c0a47f6d55bbd1
DIFF: https://github.com/llvm/llvm-project/commit/3fbc344b49800bb0f70fd5af46c0a47f6d55bbd1.diff

LOG: [WebAssembly] Refactor Wasm EH/SjLj error checking (#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.

Added: 
    

Modified: 
    clang/lib/Driver/ToolChains/WebAssembly.cpp
    clang/test/Driver/wasm-toolchain.c

Removed: 
    


################################################################################
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
+  // 
diff erent 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