[clang] [clang][WebAssembly] Handle casted function pointers with different number of arguments (PR #153168)

[Jorge Zapata via cfe-commits]

turran wrote:

> 
> This is part of what I'm trying to understand about the use case. Namely, exactly who has to know the implications? (equivalently, who would need to enable this flag)? is it just the GLib core developers, i.e. the flag just needs to be enable to build GLib code? Or is it everyone who wants to link against GLib?

This flag needs to be enabled whenever your code does function casting to a different kind of function. In the particular case of GLib (but not only GLIb, but for example GStreamer, or many others), this is needed only when building GLib, so yes, that's for the core developers. Linking to GLib doesn't require the flag, unless your code is also doing the same function casting ABI assumption.

> 
> > The signals are not the problem; is the whole GOBject inheritance model that is based on this ABI assumption. It is difficult for me to find a middle ground here ...
> 
> This phrasing suggests that everyone who wants to use GLib has calls in their code (or at least their object files) that need to use mismatched signatures. The fact that IIUC you were able to make things work with hacks that were localized to GLib means that your changes just leaked into public headers?

The changes we made to make GLib work implied internal code only, not at the headers or public API.

> 
> EMULATE_FUNCTION_POINTER_CASTS (implemented by Binaryen's [fpcast-emu](https://github.com/WebAssembly/binaryen/blob/main/src/passes/FuncCastEmulation.cpp) transformation pass) makes all indirect calls use the same number of arguments, and creates thunks that pass those calls through with the right number of arguments. So it should fix both the number and type of arguments.
> 
> That was actually one question I had for you about the patch you wrote here vs EMULATE_FUNCTION_POINTER_CASTS, namely whether EMULATE_FUNCTION_POINTER_CASTS actually worked and was sufficient for GLib (it was unclear from the discussion; also a side question was if so, how bad was the performance effect?). It should work for both number and type of arguments. IIRC it only works for indirect calls, but we could probably fix that on the Binaryen side.
>

I'm not sure, to be honest. In different posts, it seems that EMULATE_FUNCTION_POINTER_CASTS _used_ to work, but since three years ago, when the `gst.wasm` project started, it has never worked, and thus our own fork was needed. I see that the code in `binaryen` is very small and has received recent changes. I'll need to check if, at least, the PR's examples work properly, but I doubt. Especially for a callback-field kind of cast:

```
void test() {
  StructWithFunctionPointer sfp = {
    FUNCTION_POINTER(fp_less)
  };

  int a1 = sfp.fp(10, 20);
}
```
Here `fp` needs to be traced back to `fp_less` which I doubt the FuncCastEmulation will handle. In a real-life case, they belong to two different modules, being sfp and the call to sfp.fp in GLib, for example, and the casted function in GStreamer. The definition of sfp.fp is a pointer (int) without type information, at least on the IR generated. If the logic has to be done on the call (sfp.fp) and not the cast on sfp's field, I don't know how it is going to be done properly with FuncCastEmulation.

> edit: for that matter, was the old FixFunctionBitcasts pass from before opaque pointers sufficient? I think not, right?

Correct, it wasn't working before either. The first set of changes I made to FixFunctionBitcasts was to handle the particularities of the casts found in the examples. Those were missing. After that, due to the opaque pointer thing, it was impossible to know the source and target function types. That's why I guessed that the EMULATE_FUNCTION_POINTER_CASTS was suffering from the same problem.


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