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

[sbc100]

Summary

Previously we could emit a warning and generate a potentially invalid
wasm module (due to call sites and functions having conflicting
signatures). Now, rather than create invalid binaries we handle such
cases by creating stub functions containing unreachable, effectively
turning these into runtime errors rather than validation failures.

Fixes PR40470

[sbc100]

• add docs

[sbc100]

• rebase

[sbc100]

• comments

[sbc100]

• rebase

[sbc100]

rebase

[sbc100]

• fix after upstream llvm change

[ruiu]

docs/WebAssembly.rst

98 ↗

(On Diff #185872)

Is this a new default behavior? I wonder what is the motivation of the change. I mean, finding an error at static-link-time is almost always better than finding it at run-time, no?

[sbc100]

docs/WebAssembly.rst

98 ↗

(On Diff #185872)

It complicated.

Before this change, we generate a warning and an invalid module.

I've tried in the past to make this a hard error because generating an invalid module seems like always the wrong thing to do.

When I do this lots of things break. The most common failure is CMake checks for whether a function exists. Cmake create a tiny C program with the named symbol and checks that it links. But it doesn't know the signature so I always uses "char <func>()". This is almost always wrong but the expectation is that linking should work.

In you actually tried to execute that at runtime you'd get undefined behaviour, but its not trying to run it.

So, the conclusion was to make this the default behaviour and allow the link to succeed, and have an option for a more strict mode.

We could invert the option... but that wouldn't save us any complexity and I quite the like the default to match the native linker behaviour.

[ruiu]

Ah, thanks for the explanation. I think that makes sense to me.

If you create a module with a invalid function type, what would happen at runtime? Does it fail to load?

[sbc100]

In D57909#1389792, @ruiu wrote:

Ah, thanks for the explanation. I think that makes sense to me.

If you create a module with a invalid function type, what would happen at runtime? Does it fail to load?

Yes, the browser would fail to validate the module.

Now it will validate, and even run, as long as you don't hit that invalid call sites.

[ruiu]

How can you fix the CMake issue you mentioned by this change? If CMake checks only if a source file compiles, then this patch doesn't change the behavior (previously we created a broken module but it at least links). If CMake actually tries to run a function, then previously it fails at load-time, and with this patch it would fail when a program attempt to execute a function. So, looks like the behavior observed by CMake doesn't change that much. What am I missing?

[sbc100]

In D57909#1389855, @ruiu wrote:

How can you fix the CMake issue you mentioned by this change? If CMake checks only if a source file compiles, then this patch doesn't change the behavior (previously we created a broken module but it at least links). If CMake actually tries to run a function, then previously it fails at load-time, and with this patch it would fail when a program attempt to execute a function. So, looks like the behavior observed by CMake doesn't change that much. What am I missing?

You are right, I'm sorry, there is a little more to the story: The emscripten toolchain driver "emcc" runs a post-link tool after lld which will fail (quite rightly) when given an invalid module.

I think its pretty safe to say we should never generate invalid modules anyway, so even without this issue I would say the current state is broken.

[sbc100]

Also, I should have mentioned that we can improve on this change by allowing for more flexible stub functions.

For example, if one translation unit declares a function as returning void but the function as defined returns, say, int32, we can have a stub function that simply drops the return value. We have pass in LLVM that already does this for uses within the same bitcode file https://github.com/llvm-mirror/llvm/blob/master/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp.

[ruiu]

docs/WebAssembly.rst

101–106 ↗

(On Diff #185873)

Could you update this comment a bit to reflect the discussion we've had so far? I think that it should mention the following things:

• The runtime verifier rejects modules if it finds a function type mismatch
• So there's no point of creating a module by ignoring type mismatch errors
• But sometimes link failure as a result of a type mismatch is too strict; you want to create an executable even if it contains a few errors.
• So lld/wasm supports two modes: one is to report an error for type mismatch and exit. The other is to create a stub function for an erroneous function, so that the generated file at least passes the verifier, which effectively turns a type error into a runtime error. The latter is the default mode.

wasm/Options.td

94 ↗

(On Diff #185873)

Remove a space before :. Indent with two spaces.

wasm/SymbolTable.cpp

311 ↗

(On Diff #185873)

created -> Created

410–411 ↗

(On Diff #185873)

else if should be on the same line.

571–572 ↗

(On Diff #185873)

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?

[sbc100]

• feedback + clang-format

wasm/Options.td

94 ↗

(On Diff #185873)

Ha.. I hit my clang format button to get this :)

wasm/SymbolTable.cpp

571–572 ↗

(On Diff #185873)

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.

[ruiu]

wasm/SymbolTable.cpp

571–572 ↗

(On Diff #185873)

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?

[sbc100]

• move to monorepo

[sbc100]

• group functions

[sbc100]

• more testing

[sbc100]

wasm/SymbolTable.cpp

571–572 ↗

(On Diff #185873)

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.

[ruiu]

lld/wasm/SymbolTable.cpp

162–163

Could you add a short comment here saying that if one symbol is a function and the other is not a function, it is always a function type error?

162–163

It looks like this function does two different things:

1. verifying that both symbols are function symbols
2. verifying that the two functions have the same type signature

a boolean return value doesn't make much sense for (1).

I'd remove this part of code from this function and move that error check to the caller.

lld/wasm/Symbols.h

76

At this point we probably should simply make Name a public member, as it has both set and get accessors.

[sbc100]

• feedback

[ruiu]

lld/wasm/Options.td

94 ↗

(On Diff #186568)

A little bikeshedish, but --signature-check=strict or --strict-signature-check feel natural to me, but --signature-check-strict seems a bit odd as a command line option name.

[sbc100]

• Remove variant symbols from the symbol table.

[ruiu]

lld/wasm/Options.td

94 ↗

(On Diff #187283)

Sorry for being nitpicky, but since command line names cannot change once set, so bear with me...

There's no linker command line option that ends with -checking. Perhaps a most similar option is -warn-mismatch and -no-warn-mismatch, which is in the form of -<verb>-... and -no-<verb>-. Maybe we should follow that convention? I.e. -check-signature?

Does anyone have a suggestion?

[sbc100]

• remove setName

[sbc100]

• remove new flag. Just go with --fatal-warnings for now

[ruiu]

LGTM

lld/docs/WebAssembly.rst

 Bahaviour
 ---------
 
 In general, where possible, the WebAssembly linker attempts to emulate the
 behavior of a traditional ELF linker, and in particular the ELF port of lld.
 For more specific details on how this is achieved see the tool conventions on
 linking_.
 
+Function Signatrues
+~~~~~~~~~~~~~~~~~~~
+
+One way in which the WebAssembly linker differs from traditional native linkers
+is that function signature checking is strict in WebAssembly.  It is a
+validation error for a module to contain to call site that doesn't agree with
+the target signature.  Even though this is undefined behavior in C/C++ its not
+uncommon to find this in real world C/C++ programs.  For example, a call site in
+one complication unit which calls a function defined in another complication
+unit but with too many arguments.
+
+In order not to generate such invalid modules lld has two modes of handling such
+mismatches: it can simply error out or it can create stub functions that will
+trap at runtime (functions that contain only an ``unreachable`` instruction)
+and use these stub functions at the otherwise invalid call sites.
+
+The the default befviour is to generate these stub function and to produce
+a warning.  The ``--falal-warnings`` flag can be used to disable this behaviour
+and error out if mismatched are found.
+
 Imports and Exports
 ~~~~~~~~~~~~~~~~~~~
 
 When building a shared library any symbols marked as ``visibility=default`` will
 be exported.  When building an executable, only the entry point and symbols
 flagged as ``WASM_SYMBOL_EXPORTED`` are exported by default.  In LLVM the
 ``WASM_SYMBOL_EXPORTED`` flag is applied to any symbol in the ``llvm.used`` list
 which corresponds to ``__attribute__((used))`` in C/C++ sources.

lld/test/wasm/Inputs/call-ret32.ll

+target triple = "wasm32-unknown-unknown"
+
+@ret32_address = global i32 (float)* @ret32, align 4
+
+define hidden i32* @call_ret32() {
+entry:
+  %call1 = call i32 @ret32(float 0.000000e+00)
+  ret i32* bitcast (i32 (float)** @ret32_address to i32*)
+}
+
+declare i32 @ret32(float)

lld/test/wasm/lto/signature-mismatch.ll

 ; f is defined to take no argument in archive.ll which is compiled to bitcode
 declare void @f(i32);
 
 define void @_start() {
   call void @f(i32 0)
   ret void
 }
 
+; CHECK: error: function signature mismatch: f
 ; CHECK: >>> defined as (i32) -> void in {{.*}}signature-mismatch.ll.tmp1.o
 ; CHECK: >>> defined as () -> void in lto.tmp

lld/test/wasm/signature-mismatch-export.ll

+; RUN: llc -filetype=obj %p/Inputs/ret32.ll -o %t.ret32.o
+; RUN: llc -filetype=obj %s -o %t.main.o
+; RUN: wasm-ld --export=ret32 -o %t.wasm %t.main.o %t.ret32.o
+; RUN: obj2yaml %t.wasm | FileCheck %s
+
+target triple = "wasm32-unknown-unknown"
+
+declare i32 @ret32(i32)
+
+define void @_start() {
+entry:
+  %call1 = call i32 @ret32(i32 0)
+  ret void
+}
+
+; CHECK:        - Type:            EXPORT
+; CHECK:            - Name:            ret32
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           3
+
+; CHECK:        - Type:            CUSTOM
+; CHECK-NEXT:     Name:            name
+; CHECK-NEXT:     FunctionNames:
+; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         Name:            __wasm_call_ctors
+; CHECK-NEXT:       - Index:           1
+; CHECK-NEXT:         Name:            'unreachable:ret32'
+; CHECK-NEXT:       - Index:           2
+; CHECK-NEXT:         Name:            _start
+; CHECK-NEXT:       - Index:           3
+; CHECK-NEXT:         Name:            ret32
+; CHECK-NEXT: ...

lld/test/wasm/signature-mismatch.ll

 ; RUN: llc -filetype=obj %p/Inputs/ret32.ll -o %t.ret32.o
+; RUN: llc -filetype=obj %p/Inputs/call-ret32.ll -o %t.call.o
 ; RUN: llc -filetype=obj %s -o %t.main.o
-; RUN: not wasm-ld --fatal-warnings -o %t.wasm %t.main.o %t.ret32.o 2>&1 | FileCheck %s
-; Run the test again by with the object files in the other order to verify
-; the check works when the undefined symbol is resolved by an existing defined
-; one.
-; RUN: not wasm-ld --fatal-warnings -o %t.wasm %t.ret32.o %t.main.o 2>&1 | FileCheck %s -check-prefix=REVERSE
+; RUN: wasm-ld --export=call_ret32 --export=ret32 -o %t.wasm %t.main.o %t.ret32.o %t.call.o 2>&1 | FileCheck %s -check-prefix=WARN
+; RUN: obj2yaml %t.wasm | FileCheck %s -check-prefix=YAML
+; RUN: not wasm-ld --fatal-warnings -o %t.wasm %t.main.o %t.ret32.o %t.call.o 2>&1 | FileCheck %s -check-prefix=ERROR
 
 target triple = "wasm32-unknown-unknown"
 
+@ret32_address_main = global i32 (i32, i64, i32)* @ret32, align 4
+
 ; Function Attrs: nounwind
-define hidden void @_start() local_unnamed_addr #0 {
+define hidden void @_start() local_unnamed_addr {
 entry:
-  %call = tail call i32 @ret32(i32 1, i64 2, i32 3) #2
+  %call1 = call i32 @ret32(i32 1, i64 2, i32 3)
+  %addr = load i32 (i32, i64, i32)*, i32 (i32, i64, i32)** @ret32_address_main, align 4
+  %call2 = call i32 %addr(i32 1, i64 2, i32 3)
   ret void
 }
 
-declare i32 @ret32(i32, i64, i32) local_unnamed_addr #1
+declare i32 @ret32(i32, i64, i32) local_unnamed_addr
+
+; WARN: warning: function signature mismatch: ret32
+; WARN-NEXT: >>> defined as (i32, i64, i32) -> i32 in {{.*}}.main.o
+; WARN-NEXT: >>> defined as (f32) -> i32 in {{.*}}.ret32.o
+
+; ERROR: error: function signature mismatch: ret32
+; ERROR-NEXT: >>> defined as (i32, i64, i32) -> i32 in {{.*}}.main.o
+; ERROR-NEXT: >>> defined as (f32) -> i32 in {{.*}}.ret32.o
+
+; YAML:        - Type:            EXPORT
+; YAML:           - Name:            ret32
+; YAML-NEXT:        Kind:            FUNCTION
+; YAML-NEXT:        Index:           3
+; YAML-NEXT:      - Name:            call_ret32
+; YAML-NEXT:        Kind:            FUNCTION
+; YAML-NEXT:        Index:           4
+
+; YAML:        - Type:            CUSTOM
+; YAML-NEXT:     Name:            name
+; YAML-NEXT:     FunctionNames:   
+; YAML-NEXT:       - Index:           0
+; YAML-NEXT:         Name:            __wasm_call_ctors
+; YAML-NEXT:       - Index:           1
+; YAML-NEXT:         Name:            'unreachable:ret32'
+; YAML-NEXT:       - Index:           2
+; YAML-NEXT:         Name:            _start
+; YAML-NEXT:       - Index:           3
+; YAML-NEXT:         Name:            ret32
+; YAML-NEXT:       - Index:           4
+; YAML-NEXT:         Name:            call_ret32
+; YAML-NEXT: ...
 
-; CHECK: error: function signature mismatch: ret32
-; CHECK-NEXT: >>> defined as (i32, i64, i32) -> i32 in {{.*}}.main.o
-; CHECK-NEXT: >>> defined as (f32) -> i32 in {{.*}}.ret32.o
-
-; REVERSE: error: function signature mismatch: ret32
-; REVERSE-NEXT: >>> defined as (f32) -> i32 in {{.*}}.ret32.o
-; REVERSE-NEXT: >>> defined as (i32, i64, i32) -> i32 in {{.*}}.main.o

lld/wasm/Driver.cpp

for (InputFile *F : Files)
     Symtab->addFile(F);
   if (errorCount())
     return;
 
   // Handle the `--undefined <sym>` options.
   for (auto *Arg : Args.filtered(OPT_undefined))
     handleUndefined(Arg->getValue());
 
-  // Handle the `--export <sym>` options
-  // This works like --undefined but also exports the symbol if its found
-  for (auto *Arg : Args.filtered(OPT_export)) {
-    Symbol *Sym = handleUndefined(Arg->getValue());
-    if (Sym && Sym->isDefined())
-      Sym->ForceExport = true;
-    else if (!Config->AllowUndefined)
-      error(Twine("symbol exported via --export not found: ") +
-            Arg->getValue());
-  }
-
   Symbol *EntrySym = nullptr;
   if (!Config->Relocatable) {
     if (!Config->Shared && !Config->Entry.empty()) {
       EntrySym = handleUndefined(Config->Entry);
       if (EntrySym && EntrySym->isDefined())
         EntrySym->ForceExport = true;
       else
         error("entry symbol not defined (pass --no-entry to supress): " +
               Config->Entry);
     }
-
-    // Make sure we have resolved all symbols.
-    if (!Config->AllowUndefined)
-      Symtab->reportRemainingUndefines();
   }
 
   if (errorCount())
     return;
 
+  // Handle the `--export <sym>` options
+  // This works like --undefined but also exports the symbol if its found
+  for (auto *Arg : Args.filtered(OPT_export))
+    handleUndefined(Arg->getValue());
+
   // Do link-time optimization if given files are LLVM bitcode files.
   // This compiles bitcode files into real object files.
   Symtab->addCombinedLTOObject();
   if (errorCount())
     return;
 
+  // Resolve any variant symbols that were created due to signature
+  // mismatchs.
+  Symtab->handleSymbolVariants();
+  if (errorCount())
+    return;
+
+  for (auto *Arg : Args.filtered(OPT_export)) {
+    Symbol *Sym = Symtab->find(Arg->getValue());
+    if (Sym && Sym->isDefined())
+      Sym->ForceExport = true;
+    else if (!Config->AllowUndefined)
+      error(Twine("symbol exported via --export not found: ") +
+            Arg->getValue());
+  }
+
+  if (!Config->Relocatable) {
     // Add synthetic dummies for weak undefined functions.  Must happen
     // after LTO otherwise functions may not yet have signatures.
-  if (!Config->Relocatable)
     Symtab->handleWeakUndefines();
 
+    // Make sure we have resolved all symbols.
+    if (!Config->AllowUndefined)
+      Symtab->reportRemainingUndefines();
+  }
+
   if (EntrySym)
     EntrySym->setHidden(false);
 
   if (errorCount())
     return;
 
   // Do size optimizations: garbage collection
   markLive();
 
   // Write the result to the file.
   writeResult();
 }

lld/wasm/SymbolTable.h

public:
   std::vector<InputGlobal *> SyntheticGlobals;
 
   void reportRemainingUndefines();
 
   ArrayRef<Symbol *> getSymbols() const { return SymVector; }
 
   Symbol *find(StringRef Name);
 
+  void replace(StringRef Name, Symbol* Sym);
+
   void trace(StringRef Name);
 
   Symbol *addDefinedFunction(StringRef Name, uint32_t Flags, InputFile *File,
                              InputFunction *Function);
   Symbol *addDefinedData(StringRef Name, uint32_t Flags, InputFile *File,
                          InputSegment *Segment, uint32_t Address,
                          uint32_t Size);
   Symbol *addDefinedGlobal(StringRef Name, uint32_t Flags, InputFile *File,
public:
   bool addComdat(StringRef Name);
 
   DefinedData *addSyntheticDataSymbol(StringRef Name, uint32_t Flags);
   DefinedGlobal *addSyntheticGlobal(StringRef Name, uint32_t Flags,
                                     InputGlobal *Global);
   DefinedFunction *addSyntheticFunction(StringRef Name, uint32_t Flags,
                                         InputFunction *Function);
 
+  void handleSymbolVariants();
   void handleWeakUndefines();
 
 private:
   std::pair<Symbol *, bool> insert(StringRef Name, const InputFile *File);
   std::pair<Symbol *, bool> insertName(StringRef Name);
 
+  bool getFunctionVariant(Symbol* Sym, const WasmSignature *Sig,
+                          const InputFile *File, Symbol **Out);
   InputFunction *replaceWithUnreachable(Symbol *Sym, const WasmSignature &Sig,
                                         StringRef DebugName);
 
   // Maps symbol names to index into the SymVector.  -1 means that symbols
   // is to not yet in the vector but it should have tracing enabled if it is
   // ever added.
   llvm::DenseMap<llvm::CachedHashStringRef, int> SymMap;
   std::vector<Symbol *> SymVector;
 
+  // For certain symbols types, e.g. function symbols, we allow for muliple
+  // variants of the same symbol with different signatures.
+  llvm::DenseMap<llvm::CachedHashStringRef, std::vector<Symbol *>> SymVariants;
+
   llvm::DenseSet<llvm::CachedHashStringRef> Comdats;
 
   // For LTO.
   std::unique_ptr<BitcodeCompiler> LTO;
 };
 
 extern SymbolTable *Symtab;
 
 } // namespace wasm
 } // namespace lld
 
 #endif

lld/wasm/SymbolTable.cpp

 using namespace llvm;
 using namespace llvm::wasm;
 using namespace llvm::object;
 using namespace lld;
 using namespace lld::wasm;
 
 SymbolTable *lld::wasm::Symtab;
 
+static char encodeValType(ValType Type) {
+  switch (Type) {
+  case ValType::I32:
+    return 'i';
+  case ValType::I64:
+    return 'j';
+  case ValType::F32:
+    return 'f';
+  case ValType::F64:
+    return 'd';
+  case ValType::V128:
+    return 'V';
+  case ValType::EXCEPT_REF:
+    return 'e';
+  }
+  llvm_unreachable("invalid wasm type");
+}
+
+static std::string encodeSignature(const WasmSignature &Sig) {
+  std::string S = ":";
+  for (ValType Type : Sig.Returns)
+    S += encodeValType(Type);
+  S += ':';
+  for (ValType Type : Sig.Params)
+    S += encodeValType(Type);
+  return S;
+}
+
+static StringRef getVariantName(StringRef BaseName, const WasmSignature &Sig) {
+  return Saver.save(BaseName + encodeSignature(Sig));
+}
+
 void SymbolTable::addFile(InputFile *File) {
   log("Processing: " + toString(File));
   if (Config->Trace)
     message(toString(File));
   File->parse();
 
   // LLVM bitcode file
   if (auto *F = dyn_cast<BitcodeFile>(File))

 
 Symbol *SymbolTable::find(StringRef Name) {
   auto It = SymMap.find(CachedHashStringRef(Name));
   if (It == SymMap.end() || It->second == -1)
     return nullptr;
   return SymVector[It->second];
 }
 
+void SymbolTable::replace(StringRef Name, Symbol* Sym) {
+  auto It = SymMap.find(CachedHashStringRef(Name));
+  SymVector[It->second] = Sym;
+}
+
 std::pair<Symbol *, bool> SymbolTable::insertName(StringRef Name) {
   bool Trace = false;
   auto P = SymMap.insert({CachedHashStringRef(Name), (int)SymVector.size()});
   int &SymIndex = P.first->second;
   bool IsNew = P.second;
   if (SymIndex == -1) {
     SymIndex = SymVector.size();
     Trace = true;

 static void reportTypeError(const Symbol *Existing, const InputFile *File,
                             llvm::wasm::WasmSymbolType Type) {
   error("symbol type mismatch: " + toString(*Existing) + "\n>>> defined as " +
         toString(Existing->getWasmType()) + " in " +
         toString(Existing->getFile()) + "\n>>> defined as " + toString(Type) +
         " in " + toString(File));
 }
 
 // Check the type of new symbol matches that of the symbol is replacing.
-// For functions this can also involve verifying that the signatures match.
-static void checkFunctionType(Symbol *Existing, const InputFile *File,
-                              const WasmSignature *NewSig) {
-  auto ExistingFunction = dyn_cast<FunctionSymbol>(Existing);
-  if (!ExistingFunction) {
-    reportTypeError(Existing, File, WASM_SYMBOL_TYPE_FUNCTION);
-    return;
-  }
-
+// Returns true if the function types match, false is there is a singature
+// mismatch.
+bool signatureMatches(FunctionSymbol *Existing, const WasmSignature *NewSig) {
   if (!NewSig)
-    return;
+    return true;
 
-  const WasmSignature *OldSig = ExistingFunction->Signature;
+  const WasmSignature *OldSig = Existing->Signature;
   if (!OldSig) {
-    ExistingFunction->Signature = NewSig;
-    return;
+    Existing->Signature = NewSig;
+    return true;
   }
 
-  if (*NewSig != *OldSig)
-    warn("function signature mismatch: " + Existing->getName() +
-         "\n>>> defined as " + toString(*OldSig) + " in " +
-         toString(Existing->getFile()) + "\n>>> defined as " +
-         toString(*NewSig) + " in " + toString(File));
+  return *NewSig == *OldSig;
 }
 
 static void checkGlobalType(const Symbol *Existing, const InputFile *File,
                             const WasmGlobalType *NewType) {
   if (!isa<GlobalSymbol>(Existing)) {
     reportTypeError(Existing, File, WASM_SYMBOL_TYPE_GLOBAL);
     return;
   }
Symbol *SymbolTable::addDefinedFunction(StringRef Name, uint32_t Flags,
                                         InputFunction *Function) {
   LLVM_DEBUG(dbgs() << "addDefinedFunction: " << Name << " ["
                     << (Function ? toString(Function->Signature) : "none")
                     << "]\n");
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, File);
 
+  auto Replace = [&](Symbol* Sym) {
+    // If the new defined function doesn't have signture (i.e. bitcode
+    // functions) but the old symbol does, then preserve the old signature
+    const WasmSignature *OldSig = S->getSignature();
+    auto* NewSym = replaceSymbol<DefinedFunction>(Sym, Name, Flags, File, Function);
+    if (!NewSym->Signature)
+      NewSym->Signature = OldSig;
+  };
+
   if (WasInserted || S->isLazy()) {
-    replaceSymbol<DefinedFunction>(S, Name, Flags, File, Function);
+    Replace(S);
     return S;
   }
 
-  if (Function)
-    checkFunctionType(S, File, &Function->Signature);
+  auto ExistingFunction = dyn_cast<FunctionSymbol>(S);
+  if (!ExistingFunction) {
+    reportTypeError(S, File, WASM_SYMBOL_TYPE_FUNCTION);
+    return S;
+  }
 
-  if (shouldReplace(S, File, Flags)) {
-    // If the new defined function doesn't have signture (i.e. bitcode
-    // functions) but the old symbol does then preserve the old signature
-    const WasmSignature *OldSig = nullptr;
-    if (auto* F = dyn_cast<FunctionSymbol>(S))
-      OldSig = F->Signature;
-    if (auto *L = dyn_cast<LazySymbol>(S))
-      OldSig = L->Signature;
-    auto NewSym = replaceSymbol<DefinedFunction>(S, Name, Flags, File, Function);
-    if (!NewSym->Signature)
-      NewSym->Signature = OldSig;
+  if (Function && !signatureMatches(ExistingFunction, &Function->Signature)) {
+    Symbol* Variant;
+    if (getFunctionVariant(S, &Function->Signature, File, &Variant))
+      // New variant, always replace
+      Replace(Variant);
+    else if (shouldReplace(S, File, Flags))
+      // Variant already exists, replace it after checking shouldReplace
+      Replace(Variant);
+
+    // This variant we found take the place in the symbol table as the primary
+    // variant.
+    replace(Name, Variant);
+    return Variant;
   }
+
+  // Existing function with matching signature.
+  if (shouldReplace(S, File, Flags))
+    Replace(S);
+
   return S;
 }
 
 Symbol *SymbolTable::addDefinedData(StringRef Name, uint32_t Flags,
                                     InputFile *File, InputSegment *Segment,
                                     uint32_t Address, uint32_t Size) {
   LLVM_DEBUG(dbgs() << "addDefinedData:" << Name << " addr:" << Address
                     << "\n");
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, File);
 
-  if (WasInserted || S->isLazy()) {
+  auto Replace = [&]() {
     replaceSymbol<DefinedData>(S, Name, Flags, File, Segment, Address, Size);
+  };
+
+  if (WasInserted || S->isLazy()) {
+    Replace();
     return S;
   }
 
   checkDataType(S, File);
 
   if (shouldReplace(S, File, Flags))
-    replaceSymbol<DefinedData>(S, Name, Flags, File, Segment, Address, Size);
+    Replace();
   return S;
 }
 
 Symbol *SymbolTable::addDefinedGlobal(StringRef Name, uint32_t Flags,
                                       InputFile *File, InputGlobal *Global) {
   LLVM_DEBUG(dbgs() << "addDefinedGlobal:" << Name << "\n");
 
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, File);
 
-  if (WasInserted || S->isLazy()) {
+  auto Replace = [&]() {
     replaceSymbol<DefinedGlobal>(S, Name, Flags, File, Global);
+  };
+
+  if (WasInserted || S->isLazy()) {
+    Replace();
     return S;
   }
 
   checkGlobalType(S, File, &Global->getType());
 
   if (shouldReplace(S, File, Flags))
-    replaceSymbol<DefinedGlobal>(S, Name, Flags, File, Global);
+    Replace();
   return S;
 }
 
 Symbol *SymbolTable::addDefinedEvent(StringRef Name, uint32_t Flags,
                                      InputFile *File, InputEvent *Event) {
   LLVM_DEBUG(dbgs() << "addDefinedEvent:" << Name << "\n");
 
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, File);
 
-  if (WasInserted || S->isLazy()) {
+  auto Replace = [&]() {
     replaceSymbol<DefinedEvent>(S, Name, Flags, File, Event);
+  };
+
+  if (WasInserted || S->isLazy()) {
+    Replace();
     return S;
   }
 
   checkEventType(S, File, &Event->getType(), &Event->Signature);
 
   if (shouldReplace(S, File, Flags))
-    replaceSymbol<DefinedEvent>(S, Name, Flags, File, Event);
+    Replace();
   return S;
 }
 
 Symbol *SymbolTable::addUndefinedFunction(StringRef Name, StringRef ImportName,
                                           StringRef ImportModule,
                                           uint32_t Flags, InputFile *File,
                                           const WasmSignature *Sig) {
   LLVM_DEBUG(dbgs() << "addUndefinedFunction: " << Name <<
              " [" << (Sig ? toString(*Sig) : "none") << "]\n");
 
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, File);
 
-  if (WasInserted)
+  auto Replace = [&]() {
     replaceSymbol<UndefinedFunction>(S, Name, ImportName, ImportModule, Flags,
                                      File, Sig);
+  };
+
+  if (WasInserted)
+    Replace();
   else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     Lazy->fetch();
-  else
-    checkFunctionType(S, File, Sig);
+  else {
+    auto ExistingFunction = dyn_cast<FunctionSymbol>(S);
+    if (!ExistingFunction) {
+      reportTypeError(S, File, WASM_SYMBOL_TYPE_FUNCTION);
+      return S;
+    }
+    if (!signatureMatches(ExistingFunction, Sig))
+      if (getFunctionVariant(S, Sig, File, &S))
+        Replace();
+  }
 
   return S;
 }
 
 Symbol *SymbolTable::addUndefinedData(StringRef Name, uint32_t Flags,
                                       InputFile *File) {
   LLVM_DEBUG(dbgs() << "addUndefinedData: " << Name << "\n");
 
void SymbolTable::addLazy(ArchiveFile *File, const Archive::Symbol *Sym) {
   LLVM_DEBUG(dbgs() << "replacing existing undefined\n");
   File->addMember(Sym);
 }
 
 bool SymbolTable::addComdat(StringRef Name) {
   return Comdats.insert(CachedHashStringRef(Name)).second;
 }
 
+// The new signature doesn't match.  Create a variant to the symbol with the
+// signature encoded in the name and return that instead.  These symbols are
+// then unified later in handleSymbolVariants.
+bool SymbolTable::getFunctionVariant(Symbol* Sym, const WasmSignature *Sig,
+                                     const InputFile *File, Symbol **Out) {
+  StringRef NewName = getVariantName(Sym->getName(), *Sig);
+  LLVM_DEBUG(dbgs() << "getFunctionVariant: " << Sym->getName() << " -> " << NewName
+                    << " " << toString(*Sig) << "\n");
+  Symbol *Variant = nullptr;
+
+  // Linear search through symbol variants.  Should never be more than two
+  // or three entries here.
+  auto &Variants = SymVariants[CachedHashStringRef(Sym->getName())];
+  if (Variants.size() == 0)
+    Variants.push_back(Sym);
+
+  for (Symbol* V : Variants) {
+    if (*V->getSignature() == *Sig) {
+      Variant = V;
+      break;
+    }
+  }
+
+  bool WasAdded = !Variant;
+  if (WasAdded) {
+    // Create a new variant;
+    LLVM_DEBUG(dbgs() << "added new variant\n");
+    Variant = reinterpret_cast<Symbol *>(make<SymbolUnion>());
+    Variants.push_back(Variant);
+  } else {
+    LLVM_DEBUG(dbgs() << "variant already exists: " << toString(*Variant) << "\n");
+    assert(*Variant->getSignature() == *Sig);
+  }
+
+  *Out = Variant;
+  return WasAdded;
+}
+
 // Set a flag for --trace-symbol so that we can print out a log message
 // if a new symbol with the same name is inserted into the symbol table.
 void SymbolTable::trace(StringRef Name) {
   SymMap.insert({CachedHashStringRef(Name), -1});
 }
 
 static const uint8_t UnreachableFn[] = {
     0x03 /* ULEB length */, 0x00 /* ULEB num locals */,
     0x00 /* opcode unreachable */, 0x0b /* opcode end */
 };
 
 // Replace the given symbol body with an unreachable function.
 // This is used by handleWeakUndefines in order to generate a callable
-// equivalent of an undefined function.
+// equivalent of an undefined function and also handleSymbolVariants for
+// undefined functions that don't match the signature of the definition.
 InputFunction *SymbolTable::replaceWithUnreachable(Symbol *Sym,
                                                    const WasmSignature &Sig,
                                                    StringRef DebugName) {
   auto *Func = make<SyntheticFunction>(Sig, Sym->getName(), DebugName);
   Func->setBody(UnreachableFn);
   SyntheticFunctions.emplace_back(Func);
-  replaceSymbol<DefinedFunction>(Sym, Sym->getName(), Sym->getFlags(), nullptr, Func);
+  replaceSymbol<DefinedFunction>(Sym, Sym->getName(), Sym->getFlags(), nullptr,
+                                 Func);
   return Func;
 }
 
 // For weak undefined functions, there may be "call" instructions that reference
 // the symbol. In this case, we need to synthesise a dummy/stub function that
 // will abort at runtime, so that relocations can still provided an operand to
 // the call instruction that passes Wasm validation.
 void SymbolTable::handleWeakUndefines() {
   for (Symbol *Sym : getSymbols()) {
     if (!Sym->isUndefWeak())
       continue;
 
-    const WasmSignature *Sig = nullptr;
-
-    if (auto *FuncSym = dyn_cast<FunctionSymbol>(Sym)) {
+    const WasmSignature *Sig = Sym->getSignature();
+    if (!Sig) {
       // It is possible for undefined functions not to have a signature (eg. if
       // added via "--undefined"), but weak undefined ones do have a signature.
-      assert(FuncSym->Signature);
-      Sig = FuncSym->Signature;
-    } else if (auto *LazySym = dyn_cast<LazySymbol>(Sym)) {
-      // Lazy symbols may not be functions and therefore can have a null
-      // signature.
-      Sig = LazySym->Signature;
-    }
-
-    if (!Sig)
+      // Lazy symbols may not be functions and therefore Sig can still be null
+      // in some circumstantce.
+      assert(!isa<FunctionSymbol>(Sym));
       continue;
+    }
 
     // Add a synthetic dummy for weak undefined functions.  These dummies will
     // be GC'd if not used as the target of any "call" instructions.
     StringRef DebugName = Saver.save("undefined:" + toString(*Sym));
     InputFunction* Func = replaceWithUnreachable(Sym, *Sig, DebugName);
     // Ensure it compares equal to the null pointer, and so that table relocs
     // don't pull in the stub body (only call-operand relocs should do that).
     Func->setTableIndex(0);
     // Hide our dummy to prevent export.
     Sym->setHidden(true);
   }
 }
+
+static void reportFunctionSignatureMismatch(StringRef SymName,
+                                            FunctionSymbol *A,
+                                            FunctionSymbol *B, bool Error) {
+  std::string msg = ("function signature mismatch: " + SymName +
+                     "\n>>> defined as " + toString(*A->Signature) + " in " +
+                     toString(A->getFile()) + "\n>>> defined as " +
+                     toString(*B->Signature) + " in " + toString(B->getFile()))
+                        .str();
+  if (Error)
+    error(msg);
+  else
+    warn(msg);
+}
+
+// Remove any variant symbols that were created due to function signature
+// mismatches.
+void SymbolTable::handleSymbolVariants() {
+  for (auto Pair : SymVariants) {
+    // Push the initial symbol onto the list of variants.
+    StringRef SymName = Pair.first.val();
+    std::vector<Symbol *> &Variants = Pair.second;
+
+#ifndef NDEBUG
+    dbgs() << "symbol with (" << Variants.size()
+                      << ") variants: " << SymName << "\n";
+    for (auto *S: Variants) {
+      auto *F = cast<FunctionSymbol>(S);
+      dbgs() << " variant: " + F->getName() << " " << toString(*F->Signature) << "\n";
+    }
+#endif
+
+    // Find the one definition.
+    DefinedFunction *Defined = nullptr;
+    for (auto *Symbol : Variants) {
+      if (auto F = dyn_cast<DefinedFunction>(Symbol)) {
+        Defined = F;
+        break;
+      }
+    }
+
+    // If there are no definitions, and the undefined symbols disagree on
+    // the signature, there is not we can do since we don't know which one
+    // to use as the signature on the import.
+    if (!Defined) {
+      reportFunctionSignatureMismatch(SymName,
+                                      cast<FunctionSymbol>(Variants[0]),
+                                      cast<FunctionSymbol>(Variants[1]), true);
+      return;
+    }
+
+    for (auto *Symbol : Variants) {
+      if (Symbol != Defined) {
+        auto *F = cast<FunctionSymbol>(Symbol);
+        reportFunctionSignatureMismatch(SymName, F, Defined, false);
+        StringRef DebugName = Saver.save("unreachable:" + toString(*F));
+        replaceWithUnreachable(F, *F->Signature, DebugName);
+      }
+    }
+  }
+}

lld/wasm/Symbols.h

public:
   bool isUndefWeak() const {
     // See comment on lazy symbols for details.
     return isWeak() && (isUndefined() || isLazy());
   }
 
   // Returns the symbol name.
   StringRef getName() const { return Name; }
 
   // Returns the file from which this symbol was created.
   InputFile *getFile() const { return File; }
 
   uint32_t getFlags() const { return Flags; }
 
   InputChunk *getChunk() const;
 
   // Indicates that the section or import for this symbol will be included in
   // the final image.
public:
 
   // True if this symbol was referenced by a regular (non-bitcode) object.
   unsigned IsUsedInRegularObj : 1;
   unsigned ForceExport : 1;
 
   // True if this symbol is specified by --trace-symbol option.
   unsigned Traced : 1;
 
+  const WasmSignature* getSignature() const;
+
 protected:
   Symbol(StringRef Name, Kind K, uint32_t Flags, InputFile *F)
       : IsUsedInRegularObj(false), ForceExport(false), Traced(false),
         Name(Name), SymbolKind(K), Flags(Flags), File(F),
         Referenced(!Config->GcSections) {}
 
   StringRef Name;
   Kind SymbolKind;

lld/wasm/Symbols.cpp

if (isa<GlobalSymbol>(this))
     return WASM_SYMBOL_TYPE_GLOBAL;
   if (isa<EventSymbol>(this))
     return WASM_SYMBOL_TYPE_EVENT;
   if (isa<SectionSymbol>(this))
     return WASM_SYMBOL_TYPE_SECTION;
   llvm_unreachable("invalid symbol kind");
 }
 
+const WasmSignature *Symbol::getSignature() const {
+  if (auto* F = dyn_cast<FunctionSymbol>(this))
+    return F->Signature;
+  if (auto *L = dyn_cast<LazySymbol>(this))
+    return L->Signature;
+  return nullptr;
+}
+
 InputChunk *Symbol::getChunk() const {
   if (auto *F = dyn_cast<DefinedFunction>(this))
     return F->Function;
   if (auto *D = dyn_cast<DefinedData>(this))
     return D->Segment;
   return nullptr;
 }