[PATCH] D97843: [lld][WebAssembly] -Bsymbolic creates indirect function table if needed

Andy Wingo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 4 00:23:23 PST 2021


wingo marked 6 inline comments as done.
wingo added inline comments.


================
Comment at: lld/wasm/Writer.cpp:748
+  if (!indirectFunctionTablePresentWhenComputingImports &&
+      shouldImport(WasmSym::indirectFunctionTable)) {
+    // Processing -Bsymbolic relocations resulted in a late requirement that the
----------------
sbc100 wrote:
> sbc100 wrote:
> > This kind of ordering violation kind of makes me sad, but I'm not sure I have any better ideas.
> Can this be replaced by:
> 
> ```
> // In some cases indirectFunctionTable is added during scanRelocations and therefore was not
> // added to the import list at the normal time. 
> if (shouldImport(WasmSym::indirectFunctionTable) && !WasmSym::indirectFunctionTable.hasTableNumber()) {
>     ...
> ````
> 
> This would avoid the extra `indirectFunctionTablePresentWhenComputingImports` variable which would simplify the patch a little I think (you could revert the conversion of finalizeIndirectFunctionTable to a member for example).
> 
> Its still ugly but less intrusive maybe.
Yeah I feel the pain here -- I did try to add a clause in the earlier `resolveIndirectFunctionTable` routine, but at that point it's too hard to know if there are GOT function pointers.  Sad indeed.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97843



More information about the llvm-commits mailing list