[PATCH] D44028: [WebAssembly] Add message for relocation against weak undefined symbol

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added inline comments.


================
Comment at: wasm/InputFiles.cpp:91
+      fatal("Cannot generate relocation against undefined function: " +
+            toString(*Sym));
+    }
----------------
sbc100 wrote:
> i'm curious,  is llc capable of generating such a `call` or will it always using call_indirect in this case?
> 
> if possible I'd rather generate the test inputs from bitcode (or ever better assembly) rather than basically checking in binary code.
Yes, `llc` can generate such a `call`. But I regard that as a bug and I'm going to submit a patch for it when I get round to it!

(I've filed this one in Bugzilla)

If you have code like this, then clang will currently generate an unlinkable object file:

```
void __attribute__((weak)) maybeFn(void);

void callOrSkip() {
  if (maybeFn) maybeFn();
}
```

There's really no other way to use weak undefined symbols! Yet currently we write out Wasm like this:

```
(func callOrSkip
  if (const.i32 @R_TABLE_INDEX_SLEB(maybeFn))
    call @R_FUNCTION_INDEX_SLEB(maybe)
)
```

It can't link because the call won't validate the Wasm type checker....

I reckon that the frontend should use call_indirect for all calls to weak functions that aren't defined in the translation unit, otherwise it's basically unusable.


================
Comment at: wasm/InputFiles.cpp:97
+    Symbol *Sym = getGlobalSymbol(Reloc.Index);
+    if (!Sym->hasOutputIndex()) {
+      // This happens if someone tries to use a `get/set_global` instruction to
----------------
sbc100 wrote:
> Would it make more sense to check !Sym->isDefined()?
You think? We're calling `getOutputIndex` on the line below, it seems like it's the obvious way to check whether that will succeed/fail!


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44028