llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.97k stars 11.54k forks source link

[wasm] Feature Request: Please add a warning/error diagnostic option specifically for Wasm ABI function pointer casts #64526

Open juj opened 1 year ago

juj commented 1 year ago

When running code compiled to WebAssembly, invoking a function pointer via an incompatible signature will cause a trap, e.g.:

RuntimeError: null function or function signature mismatch

LLVM supports two command line diagnostics to capture warning/error messages with respect to unsafe function pointer signature casts: -Wbad-function-cast and -Wcast-function-type to help detect such errors at compile-time.

However, these diagnostic flags are not designed to be accurate with respect to WebAssembly function dispatch signature rules. For example, the following program

void foo(int x) { }

int main()
{
  reinterpret_cast<void(*)(unsigned int)>(foo)(0); // (A)
  reinterpret_cast<void(*)(float)>(foo)(0.f);      // (B)
  return 0;
}

will pass line A, but crash on line B when run in WebAssembly.

If the program is built with -Wbad-function-cast, maybe surprisingly, no diagnostic will be given. If the program is built with -Wcast-function-type, a diagnostic will be given of both lines A and B, which can be counterproductive and overwhelming.

In WebAssembly, the function pointer call signature checks are performed with respect to only the primitive types that Wasm recognizes: i32, i64, f32, f64 and v128. Hence under those rules, line A above is ok, but line B above is not.

In our scenario, when working on the Unity3D engine codebase, on the order of ~millions of code lines, enabling -Wcast-function-type will produce thousands of lines of diagnostics, of which most (all?) are false positives. Whereas enabling -Wbad-function-cast does not report a single diagnostic warning.

It would be extremely helpful to have a command line flag, which would only diagnose those function signature casts that precisely will violate the WebAssembly signature check rules, i.e. casts between the Wasm primitive types, and nothing else.

With such a flag, it would be more feasible to audit through our millions of lines of code, to focus on call sites that will be known at compile time to run into trouble. Would it be possible to add such a -Wbad-wasm-function-cast flag for example?

llvmbot commented 1 year ago

@llvm/issue-subscribers-backend-webassembly

juj commented 1 year ago

CC @dschuff @sbc100 @tlively @kripken

sbc100 commented 1 year ago

I think this tricky to do in the general case because IIRC in most cases the usage of the pointer and the original cast are not together like this. Often/normally one stores the function pointer in memory somewhere (e.g. some void* location), then at some later points we ready the pointer back and down cast it to a specific and possibly incorrect/inconsistent function type.

When casting a function pointer from a memory location like this I don't think there is any mechanism by which llvm/clang can know the original type of the thing stored in memory. We would need some kind of runtime check I think.

juj commented 1 year ago

in most cases the usage of the pointer and the original cast

My intention was that the usage site of the pointer could be ignored, just like -Wbad-function-cast and -Wcast-function-type warnings do: they also look just the cast site. I think that would be fine for this kind of a flag.

I don't think there is any mechanism by which llvm/clang can know the original type

Hmm, doesn't the implementation of -Wbad-function-cast and/or -Wcast-function-type need to analyse both the from -> to signatures of the cast? A -Wbad-wasm-function-cast flag could look just into those same signatures, but just interpret these differently according to Wasm rules?

dschuff commented 1 year ago

As Sam points out it will always be possible to evade this warning (e.g. by loading a value as a void* and then casting it to a function pointer and then calling it). But I think it does seem possible at least in theory to have a warning that will trigger only when a cast would change the wasm signature (or perhaps to augment -Wbad-function-cast to fire in such cases). I expect it might require some refactoring though. The logic for determining the wasm signature from the LLVM signature is currently buried deep in the backend (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp#L1299) and we would have to make that somehow available to clang (which I think would mean it would need to be available even when compiling without the wasm backend). I'm not sure how hard that would be.

aheejin commented 1 year ago

TIL there's a sanitizer option called -fsanitize=function, and it looks like for detecting this kind of errors, even though wasm doesn't support it yet. Supporting it in emscripten will require a separate runtime library support.

juj commented 1 year ago

A runtime sanitizer option is not particularly interesting here. After all, all wasm runtimes already strictly abort when such a bad function pointer call is done, so having a sanitizer tell you that the code will abort right before it would abort anyway does not change things.

The interest is specifically in a compile-time warning and/or error that we could enable as a -Werror= option so that the hundreds of engineers that author code to our repository will not be able to land a cast into the codebase that would be a problem for wasm.