[PATCH] D96770: [lld][WebAssembly] Fix resolveIndirectFunctionTable for relocatable output

Wed Feb 17 01:32:16 PST 2021

wingo added inline comments.

================
Comment at: lld/wasm/Driver.cpp:832

-  if (config->importTable) {
+  if (config->importTable || (config->relocatable && existing)) {
     if (existing)
----------------
wingo wrote:
> sbc100 wrote:
> > Would it be better to ensure that `importTable` is always set when `config->relocatable`.   Anything else would be nonsensical I think.
> > 
> > We should also error out if `-r` is used with any of the table control options (`--export-table`, `--import-table`, `--growable-table` all make no sense with `--relocatable`.
> > 
> > Also why the extra `existing` here?   In `--relocatable` mode we never want to end up calling createDefinedIndirectFunctionTable so we need to make sure that this branch of the `if` I think, no?
> I can make this change, I just thought you wanted a precise mapping between command-line args and the importTable config flag.
> 
> The extra "existing" is because both `importTable` and `exportTable` force the table to be present, even if not needed.  That's not what we want with `relocatable`.
Sorry, following up here -- can't just set `importTable` when `relocatable` because that could cause the function table to be created when not needed.  Advice welcome; as is I don't see an obvious change to make.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96770/new/

https://reviews.llvm.org/D96770