Skip to content

Commit 3fbc344

Browse files
aheejinaheejin
authored
[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.
1 parent 29e5c1c commit 3fbc344

File tree

2 files changed

+69
-95
lines changed

2 files changed

+69
-95
lines changed

‎clang/lib/Driver/ToolChains/WebAssembly.cpp‎

Lines changed: 39 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -344,44 +344,53 @@ void WebAssembly::addClangTargetOptions(const ArgList &DriverArgs,
344344
}
345345
}
346346

347-
if (DriverArgs.getLastArg(options::OPT_fwasm_exceptions)) {
348-
// '-fwasm-exceptions' is not compatible with '-mno-exception-handling'
347+
// Bans incompatible options for Wasm EH / SjLj. We don't allow using
348+
// different modes for EH and SjLj.
349+
auto BanIncompatibleOptionsForWasmEHSjLj = [&](StringRef CurOption) {
349350
if (DriverArgs.hasFlag(options::OPT_mno_exception_handing,
350351
options::OPT_mexception_handing, false))
351352
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
352-
<< "-fwasm-exceptions"
353-
<< "-mno-exception-handling";
354-
// '-fwasm-exceptions' is not compatible with
355-
// '-mllvm -enable-emscripten-cxx-exceptions'
356-
for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
357-
if (StringRef(A->getValue(0)) == "-enable-emscripten-cxx-exceptions")
358-
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
359-
<< "-fwasm-exceptions"
360-
<< "-mllvm -enable-emscripten-cxx-exceptions";
361-
}
362-
// '-fwasm-exceptions' implies exception-handling feature
363-
CC1Args.push_back("-target-feature");
364-
CC1Args.push_back("+exception-handling");
365-
// Backend needs -wasm-enable-eh to enable Wasm EH
366-
CC1Args.push_back("-mllvm");
367-
CC1Args.push_back("-wasm-enable-eh");
368-
369-
// New Wasm EH spec (adopted in Oct 2023) requires multivalue and
370-
// reference-types.
353+
<< CurOption << "-mno-exception-handling";
354+
// The standardized Wasm EH spec requires multivalue and reference-types.
371355
if (DriverArgs.hasFlag(options::OPT_mno_multivalue,
372-
options::OPT_mmultivalue, false)) {
356+
options::OPT_mmultivalue, false))
373357
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
374-
<< "-fwasm-exceptions" << "-mno-multivalue";
375-
}
358+
<< CurOption << "-mno-multivalue";
376359
if (DriverArgs.hasFlag(options::OPT_mno_reference_types,
377-
options::OPT_mreference_types, false)) {
360+
options::OPT_mreference_types, false))
378361
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
379-
<< "-fwasm-exceptions" << "-mno-reference-types";
362+
<< CurOption << "-mno-reference-types";
363+
364+
for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
365+
for (const auto *Option :
366+
{"-enable-emscripten-cxx-exceptions", "-enable-emscripten-sjlj",
367+
"-emscripten-cxx-exceptions-allowed"}) {
368+
if (StringRef(A->getValue(0)) == Option)
369+
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
370+
<< CurOption << Option;
371+
}
380372
}
373+
};
374+
375+
// Enable necessary features for Wasm EH / SjLj in the backend.
376+
auto EnableFeaturesForWasmEHSjLj = [&]() {
377+
CC1Args.push_back("-target-feature");
378+
CC1Args.push_back("+exception-handling");
379+
// The standardized Wasm EH spec requires multivalue and reference-types.
381380
CC1Args.push_back("-target-feature");
382381
CC1Args.push_back("+multivalue");
383382
CC1Args.push_back("-target-feature");
384383
CC1Args.push_back("+reference-types");
384+
// Backend needs '-exception-model=wasm' to use Wasm EH instructions
385+
CC1Args.push_back("-exception-model=wasm");
386+
};
387+
388+
if (DriverArgs.getLastArg(options::OPT_fwasm_exceptions)) {
389+
BanIncompatibleOptionsForWasmEHSjLj("-fwasm-exceptions");
390+
EnableFeaturesForWasmEHSjLj();
391+
// Backend needs -wasm-enable-eh to enable Wasm EH
392+
CC1Args.push_back("-mllvm");
393+
CC1Args.push_back("-wasm-enable-eh");
385394
}
386395

387396
for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
@@ -413,53 +422,11 @@ void WebAssembly::addClangTargetOptions(const ArgList &DriverArgs,
413422
}
414423
}
415424

416-
if (Opt.starts_with("-wasm-enable-sjlj")) {
417-
// '-mllvm -wasm-enable-sjlj' is not compatible with
418-
// '-mno-exception-handling'
419-
if (DriverArgs.hasFlag(options::OPT_mno_exception_handing,
420-
options::OPT_mexception_handing, false))
421-
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
422-
<< "-mllvm -wasm-enable-sjlj"
423-
<< "-mno-exception-handling";
424-
// '-mllvm -wasm-enable-sjlj' is not compatible with
425-
// '-mllvm -enable-emscripten-cxx-exceptions'
426-
// because we don't allow Emscripten EH + Wasm SjLj
427-
for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
428-
if (StringRef(A->getValue(0)) == "-enable-emscripten-cxx-exceptions")
429-
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
430-
<< "-mllvm -wasm-enable-sjlj"
431-
<< "-mllvm -enable-emscripten-cxx-exceptions";
425+
for (const auto *Option : {"-wasm-enable-eh", "-wasm-enable-sjlj"}) {
426+
if (Opt.starts_with(Option)) {
427+
BanIncompatibleOptionsForWasmEHSjLj(Option);
428+
EnableFeaturesForWasmEHSjLj();
432429
}
433-
// '-mllvm -wasm-enable-sjlj' is not compatible with
434-
// '-mllvm -enable-emscripten-sjlj'
435-
for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
436-
if (StringRef(A->getValue(0)) == "-enable-emscripten-sjlj")
437-
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
438-
<< "-mllvm -wasm-enable-sjlj"
439-
<< "-mllvm -enable-emscripten-sjlj";
440-
}
441-
// '-mllvm -wasm-enable-sjlj' implies exception-handling feature
442-
CC1Args.push_back("-target-feature");
443-
CC1Args.push_back("+exception-handling");
444-
// Backend needs '-exception-model=wasm' to use Wasm EH instructions
445-
CC1Args.push_back("-exception-model=wasm");
446-
447-
// New Wasm EH spec (adopted in Oct 2023) requires multivalue and
448-
// reference-types.
449-
if (DriverArgs.hasFlag(options::OPT_mno_multivalue,
450-
options::OPT_mmultivalue, false)) {
451-
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
452-
<< "-mllvm -wasm-enable-sjlj" << "-mno-multivalue";
453-
}
454-
if (DriverArgs.hasFlag(options::OPT_mno_reference_types,
455-
options::OPT_mreference_types, false)) {
456-
getDriver().Diag(diag::err_drv_argument_not_allowed_with)
457-
<< "-mllvm -wasm-enable-sjlj" << "-mno-reference-types";
458-
}
459-
CC1Args.push_back("-target-feature");
460-
CC1Args.push_back("+multivalue");
461-
CC1Args.push_back("-target-feature");
462-
CC1Args.push_back("+reference-types");
463430
}
464431
}
465432
}

‎clang/test/Driver/wasm-toolchain.c‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -120,26 +120,33 @@
120120
// RUN: | FileCheck -check-prefix=EMSCRIPTEN_EH_ALLOWED_WO_ENABLE %s
121121
// EMSCRIPTEN_EH_ALLOWED_WO_ENABLE: invalid argument '-mllvm -emscripten-cxx-exceptions-allowed' only allowed with '-mllvm -enable-emscripten-cxx-exceptions'
122122

123-
// '-fwasm-exceptions' sets +exception-handling, -multivalue, -reference-types
124-
// and '-mllvm -wasm-enable-eh'
123+
// '-fwasm-exceptions' sets +exception-handling, -multivalue, -reference-types,
124+
// "-exception-model=wasm", and '-mllvm -wasm-enable-eh'
125125
// RUN: %clang -### --target=wasm32-unknown-unknown \
126126
// RUN: --sysroot=/foo %s -fwasm-exceptions 2>&1 \
127127
// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS %s
128-
// WASM_EXCEPTIONS: "-cc1" {{.*}} "-target-feature" "+exception-handling" "-mllvm" "-wasm-enable-eh" "-target-feature" "+multivalue" "-target-feature" "+reference-types"
129-
130-
// '-fwasm-exceptions' not allowed with '-mno-exception-handling'
131-
// RUN: not %clang -### --target=wasm32-unknown-unknown \
132-
// RUN: --sysroot=/foo %s -fwasm-exceptions -mno-exception-handling 2>&1 \
133-
// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS_NO_EH %s
134-
// WASM_EXCEPTIONS_NO_EH: invalid argument '-fwasm-exceptions' not allowed with '-mno-exception-handling'
128+
// WASM_EXCEPTIONS: "-cc1" {{.*}} "-target-feature" "+exception-handling" "-target-feature" "+multivalue" "-target-feature" "+reference-types" "-exception-model=wasm" "-mllvm" "-wasm-enable-eh"
135129

136130
// '-fwasm-exceptions' not allowed with
137131
// '-mllvm -enable-emscripten-cxx-exceptions'
138132
// RUN: not %clang -### --target=wasm32-unknown-unknown \
139133
// RUN: --sysroot=/foo %s -fwasm-exceptions \
140134
// RUN: -mllvm -enable-emscripten-cxx-exceptions 2>&1 \
141135
// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS_EMSCRIPTEN_EH %s
142-
// WASM_EXCEPTIONS_EMSCRIPTEN_EH: invalid argument '-fwasm-exceptions' not allowed with '-mllvm -enable-emscripten-cxx-exceptions'
136+
// WASM_EXCEPTIONS_EMSCRIPTEN_EH: invalid argument '-fwasm-exceptions' not allowed with '-enable-emscripten-cxx-exceptions'
137+
138+
// '-fwasm-exceptions' not allowed with '-mllvm -enable-emscripten-sjlj'
139+
// RUN: not %clang -### --target=wasm32-unknown-unknown \
140+
// RUN: --sysroot=/foo %s -fwasm-exceptions \
141+
// RUN: -mllvm -enable-emscripten-sjlj 2>&1 \
142+
// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS_EMSCRIPTEN_SJLJ %s
143+
// WASM_EXCEPTIONS_EMSCRIPTEN_SJLJ: invalid argument '-fwasm-exceptions' not allowed with '-enable-emscripten-sjlj'
144+
145+
// '-fwasm-exceptions' not allowed with '-mno-exception-handling'
146+
// RUN: not %clang -### --target=wasm32-unknown-unknown \
147+
// RUN: --sysroot=/foo %s -fwasm-exceptions -mno-exception-handling 2>&1 \
148+
// RUN: | FileCheck -check-prefix=WASM_EXCEPTIONS_NO_EH %s
149+
// WASM_EXCEPTIONS_NO_EH: invalid argument '-fwasm-exceptions' not allowed with '-mno-exception-handling'
143150

144151
// '-fwasm-exceptions' not allowed with '-mno-multivalue'
145152
// RUN: not %clang -### --target=wasm32-unknown-unknown \
@@ -154,46 +161,46 @@
154161
// WASM_EXCEPTIONS_NO_REFERENCE_TYPES: invalid argument '-fwasm-exceptions' not allowed with '-mno-reference-types'
155162

156163
// '-mllvm -wasm-enable-sjlj' sets +exception-handling, +multivalue,
157-
// +reference-types and '-exception-model=wasm'
164+
// +reference-types and '-exception-model=wasm'
158165
// RUN: %clang -### --target=wasm32-unknown-unknown \
159166
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj 2>&1 \
160167
// RUN: | FileCheck -check-prefix=WASM_SJLJ %s
161-
// WASM_SJLJ: "-cc1" {{.*}} "-target-feature" "+exception-handling" "-exception-model=wasm" "-target-feature" "+multivalue" "-target-feature" "+reference-types"
162-
163-
// '-mllvm -wasm-enable-sjlj' not allowed with '-mno-exception-handling'
164-
// RUN: not %clang -### --target=wasm32-unknown-unknown \
165-
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj -mno-exception-handling \
166-
// RUN: 2>&1 \
167-
// RUN: | FileCheck -check-prefix=WASM_SJLJ_NO_EH %s
168-
// WASM_SJLJ_NO_EH: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mno-exception-handling'
168+
// WASM_SJLJ: "-cc1" {{.*}} "-target-feature" "+exception-handling" "-target-feature" "+multivalue" "-target-feature" "+reference-types" "-exception-model=wasm"
169169

170170
// '-mllvm -wasm-enable-sjlj' not allowed with
171171
// '-mllvm -enable-emscripten-cxx-exceptions'
172172
// RUN: not %clang -### --target=wasm32-unknown-unknown \
173173
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj \
174174
// RUN: -mllvm -enable-emscripten-cxx-exceptions 2>&1 \
175175
// RUN: | FileCheck -check-prefix=WASM_SJLJ_EMSCRIPTEN_EH %s
176-
// WASM_SJLJ_EMSCRIPTEN_EH: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mllvm -enable-emscripten-cxx-exceptions'
176+
// WASM_SJLJ_EMSCRIPTEN_EH: invalid argument '-wasm-enable-sjlj' not allowed with '-enable-emscripten-cxx-exceptions'
177177

178178
// '-mllvm -wasm-enable-sjlj' not allowed with '-mllvm -enable-emscripten-sjlj'
179179
// RUN: not %clang -### --target=wasm32-unknown-unknown \
180180
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj \
181181
// RUN: -mllvm -enable-emscripten-sjlj 2>&1 \
182182
// RUN: | FileCheck -check-prefix=WASM_SJLJ_EMSCRIPTEN_SJLJ %s
183-
// WASM_SJLJ_EMSCRIPTEN_SJLJ: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mllvm -enable-emscripten-sjlj'
183+
// WASM_SJLJ_EMSCRIPTEN_SJLJ: invalid argument '-wasm-enable-sjlj' not allowed with '-enable-emscripten-sjlj'
184+
185+
// '-mllvm -wasm-enable-sjlj' not allowed with '-mno-exception-handling'
186+
// RUN: not %clang -### --target=wasm32-unknown-unknown \
187+
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj -mno-exception-handling \
188+
// RUN: 2>&1 \
189+
// RUN: | FileCheck -check-prefix=WASM_SJLJ_NO_EH %s
190+
// WASM_SJLJ_NO_EH: invalid argument '-wasm-enable-sjlj' not allowed with '-mno-exception-handling'
184191

185192
// '-mllvm -wasm-enable-sjlj' not allowed with '-mno-multivalue'
186193
// RUN: not %clang -### --target=wasm32-unknown-unknown \
187194
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj -mno-multivalue 2>&1 \
188195
// RUN: | FileCheck -check-prefix=WASM_SJLJ_NO_MULTIVALUE %s
189-
// WASM_SJLJ_NO_MULTIVALUE: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mno-multivalue'
196+
// WASM_SJLJ_NO_MULTIVALUE: invalid argument '-wasm-enable-sjlj' not allowed with '-mno-multivalue'
190197

191198
// '-mllvm -wasm-enable-sjlj' not allowed with '-mno-reference-types'
192199
// RUN: not %clang -### --target=wasm32-unknown-unknown \
193200
// RUN: --sysroot=/foo %s -mllvm -wasm-enable-sjlj \
194201
// RUN: -mno-reference-types 2>&1 \
195202
// RUN: | FileCheck -check-prefix=WASM_SJLJ_NO_REFERENCE_TYPES %s
196-
// WASM_SJLJ_NO_REFERENCE_TYPES: invalid argument '-mllvm -wasm-enable-sjlj' not allowed with '-mno-reference-types'
203+
// WASM_SJLJ_NO_REFERENCE_TYPES: invalid argument '-wasm-enable-sjlj' not allowed with '-mno-reference-types'
197204

198205
// RUN: %clang -### %s -fsanitize=address --target=wasm32-unknown-emscripten 2>&1 | FileCheck -check-prefix=CHECK-ASAN-EMSCRIPTEN %s
199206
// CHECK-ASAN-EMSCRIPTEN: "-fsanitize=address"

0 commit comments

Comments
 (0)