[clang] Ensure NoTrapAfterNoreturn is false for the wasm backend (PR #65876)

Matt Harding via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 25 09:29:06 PDT 2023


majaha wrote:

> These are just non-wasm options, so they mean nothing to wasm.

Yes, and using them gives no warning. 

> Wasm simply doesn't do anything with respect to these options and wasm is not overriding them.

--trap-unreachable is absolutely overridden, in exactly the same place as --no-trap-after-noreturn. It happens on the previous line of code.

> they wouldn't know that their request was silently rejected/overridden.
> To say it another way, the difference is, Wasm doesn't understand --xcoff-traceback-table=true and will do nothing with respect to it. But Wasm understands --no-trap-after-noreturn and will silently reject it after this PR.

There's no difference between "doing nothing with respect to it" and "silently rejecting", it's the same thing. The wasm backend understands neither --xcoff-traceback-table, --trap-unreachable, *or* --no-trap-after-noreturn (the fact that --no-trap-after-noreturn affects the wasm output despite being conceptually unsupported *is* the bug).
None of those are supported by the wasm backend, and they are all equally silently ignored.

> I have been kind of confused by your characterization of this as "bugfix", given that this does not change anything functionality-wise.
> ...So I'm not sure if I agree that this PR is a bugfix. I consider this as an improvement adding a failsafe mechanism.

It *does* change things functionality wise. [Look at the tests](https://github.com/llvm/llvm-project/pull/65876/commits/7e834694b09e6a7787674879442b5f0d0c92eb22#diff-1b973bc4d02665dc4a2d31916df466a5bf53c64ef70aa701ef579fc67dfbd2e4L96), the output has changed with this pull request from invalid to valid. If that's not a bug fix, I don't know what is.

> What's your distinction criteria for "real contradiction" and "hints or suggestions"?

A real contradiction would be asking for two incompatible target options, like [-exception-model=wasm -enable-emscripten-cxx-exceptions](https://godbolt.org/z/n77WeqaT6). TrapUnreachable is a much more wishy-washy request, it's frequently ignored across the codebase, and [using it doesn't even guarantee that unreachable will produce a trap](https://godbolt.org/z/evdrEvh1h). It's just a code-hardening hint that reduces the effect of runaway code in some instances.

To put it plainly: the --trap-unreachable and --no-trap-after-noreturn options *are not supported* on the wasm backend, and they are silently ignored just like every other unsupported option.
This is consistent with the behaviour both [in the wasm backend](https://github.com/llvm/llvm-project/blob/21f8bc25adba762ab06e26a7dd50da78fcd17528/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp#L129), and across the codebase: [1](https://github.com/llvm/llvm-project/blob/aa8f5e6156821aec1fed595f2e13d69655ec3311/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp#L340), [2](https://github.com/llvm/llvm-project/blob/aa8f5e6156821aec1fed595f2e13d69655ec3311/llvm/lib/Target/ARM/ARMTargetMachine.cpp#L254), [3](https://github.com/llvm/llvm-project/blob/aa8f5e6156821aec1fed595f2e13d69655ec3311/llvm/lib/Target/X86/X86TargetMachine.cpp#L237).

https://github.com/llvm/llvm-project/pull/65876


More information about the cfe-commits mailing list