[PATCH] D57909: [WebAssembly] Don't generate invalid modules when function signatures mismatch

[Sam Clegg via Phabricator via llvm-commits]

sbc100 marked an inline comment as done.
sbc100 added inline comments.


================
Comment at: wasm/SymbolTable.cpp:571-572
+
+// Remove any variant symbols that were created due to function signature
+// mismatches.
+void SymbolTable::handleSymbolVariants() {
----------------
ruiu wrote:
> sbc100 wrote:
> > ruiu wrote:
> > > It looks like you are trying to reduce the number of stub functions, by generating only one stub for each function type. But is this necessary? Given that no production code will contain a stub, no one cares if a binary with type mismatch contains many duplicated stubs?
> > Yes, I create one stub for each variant of each symbol by encoding the signature into a string and appending it to the symbol name.
> > 
> > Are you suggesting that createFunctionVariant not even use the symbol table at all and just unconditionally create a new Symbol outside the symbol table?  I guess it might work, but I'm not sure how much simpler it would make the code.
> > 
> > Finally I'm not sure this won't end up happening somewhat in production code, especially if we implement the more useful stubs that coerce rather than trap.
> > 
> I don't have a clear idea what I was suggesting, but feels like there's room to simplify the way how we creates stub functions. Currently the code for creating stub functions somewhat scatters over many places in this file.
> 
> I wonder if you can create a vector to which we store all (undefined?) symbols that we find type mismatch, and scan that vector at some point to create stubs?
My intent here is pretty much as you describe.  I have a side data structure `SymVariants` which contains all the symbols have have mismatching symbols.  This structure is empty for normal programs and only becomes populated once we have our first mismatch.

Admittedly this has to be handled in two places (for both defined and undefined functions) and we don't know which one will come first. 

I'm refactored to try to keep the new code together and added more test cases.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57909