[PATCH] D44028: [WebAssembly] Handle weak undefined functions with a synthetic stub

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

In https://reviews.llvm.org/D44028#1030026, @ruiu wrote:

> I have a few questions because I don't know much about this wasm feature...
>
> 1. Let's say `WeakFunc` is a weak function symbol that is not defined at link time. Then `if (WeakFunc)` should be evaluated to false... but how does it actually be translated to wasm instructions? Is it compiled to a indirect call with function index 0?


The address of a function is its table index in the indirection function call table.  For weak undefined functions (or null functions) this will be zero.  However the following call is *direct* call, not a call_indirect.  And direct calls are more efficient we do want continue to use direct calls wherever we can.

> 2. I'm not 100% sure why we need to provide a dummy function for an unresolved weak undefined symbols. Is it for catching an error when a program accidentally calls an undefined weak symbol at runtime? Then, I wonder if we want a function that prints out a better error message (e.g. "unresolved weak function `foo` is invoked`"), instead of just killing itself with a UB instruction.

Yes, its for catching that specific case.   In this case the linker needs insert some valid function index into the direct call instruction.  It it doesn't insert a valid function index with the correct signature, the module will fail to validate (i.e. will fail to load).   We could generate a better synthetic function with a nice error message.   However, that would add complexity to the code generation, and I think we are already on shakey ground adding any kind of code generation to the linker.  Also, simply crashing is what existing platforms do, so I think that can/should be expected.  At least, I don't think we should block this change on making an nice error message, unless you want to push the code generation part into the compiler instead?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44028