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

[Nicholas Wilson via Phabricator via llvm-commits]

ncw marked 4 inline comments as done.
ncw added inline comments.


================
Comment at: wasm/MarkLive.cpp:78-85
+      Symbol *Sym = C->File->getSymbol(Reloc.Index);
+      // Don't mark functions live if we're taking the address of a function
+      // that won't actually go in the function table (has index zero).  This
+      // is the case for some synthetic functions.
+      if ((Reloc.Type == R_WEBASSEMBLY_TABLE_INDEX_SLEB ||
+           Reloc.Type == R_WEBASSEMBLY_TABLE_INDEX_I32) &&
+          cast<FunctionSymbol>(Sym)->hasNullTableIndex())
----------------
sbc100 wrote:
> ruiu wrote:
> > Adding a special rule for a weak symbol seems a bit odd to me because they are orthogonal in concept. What if you create functions after you garbage-collect symbols? Then you can remove this logic, can't you?
> I also find this part of the change a little bit strange.   And I think the hasNullTableIndex() method helps to describe what is going on here.  However, inlineing hasNullTableIndex() along with a good comment is OK too.
It's not a super-special rule: it's generic in the sense that it would apply to any symbol that had the (weird) property of needing to compare equal to the null pointer. It's not checking for weakness, it's checking something that's actually relevant to the relocation we're considering here. I agree it's not ideal, but I think it's OK (there's already a decent comment above).


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44028