[WebAssembly] Allow function signature checking at link time

This change allows checking of function signatures but
does not yes enable it by default.  In this mode, linking
two objects that were compiled with a different signatures
for the same function will produce a link error.

New options for enabling and disabling this feature have been
added: (--check-signatures/--no-check-signatures).

Differential Revision: https://reviews.llvm.org/D40371

llvm-svn: 319396

11 files changed, 178 insertions, 61 deletions

diff --git a/lld/test/wasm/signature-mismatch.ll b/lld/test/wasm/signature-mismatch.ll
new file mode 100644
index 00000000000..ac7fcc8c67d
--- /dev/null
+++ b/lld/test/wasm/signature-mismatch.ll
@@ -0,0 +1,19 @@
+; RUN: llc -filetype=obj %p/Inputs/ret32.ll -o %t.ret32.o
+; RUN: llc -filetype=obj %s -o %t.main.o
+; RUN: not lld -flavor wasm --check-signatures -o %t.wasm %t.main.o %t.ret32.o 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+; Function Attrs: nounwind
+define hidden void @_start() local_unnamed_addr #0 {
+entry:
+  %call = tail call i32 @ret32(i32 1, i64 2, i32 3) #2
+  ret void
+}
+
+declare i32 @ret32(i32, i64, i32) local_unnamed_addr #1
+
+; 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
diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index b933a7ffb88..b5bd4063179 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -23,6 +23,7 @@ namespace wasm {
 
 struct Configuration {
   bool AllowUndefined;
+  bool CheckSignatures;
   bool Demangle;
   bool EmitRelocs;
   bool ImportMemory;
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index a1fe414b7bd..769ef85b6c1 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -153,9 +153,10 @@ static void addSyntheticGlobal(StringRef Name, int32_t Value) {
 
 // Inject a new undefined symbol into the link.  This will cause the link to
 // fail unless this symbol can be found.
-static void addSyntheticUndefinedFunction(StringRef Name) {
+static void addSyntheticUndefinedFunction(StringRef Name,
+                                          const WasmSignature *Type) {
   log("injecting undefined func: " + Name);
-  Symtab->addUndefinedFunction(Name);
+  Symtab->addUndefinedFunction(Name, Type);
 }
 
 static void printHelp(const char *Argv0) {
@@ -243,6 +244,8 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   }
 
   Config->AllowUndefined = Args.hasArg(OPT_allow_undefined);
+  Config->CheckSignatures =
+      Args.hasFlag(OPT_check_signatures, OPT_no_check_signatures, false);
   Config->EmitRelocs = Args.hasArg(OPT_emit_relocs);
   Config->Entry = Args.getLastArgValue(OPT_entry);
   Config->ImportMemory = Args.hasArg(OPT_import_memory);
@@ -278,7 +281,8 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   if (!Config->Relocatable) {
     if (Config->Entry.empty())
       Config->Entry = "_start";
-    addSyntheticUndefinedFunction(Config->Entry);
+    static WasmSignature Signature = { {}, WASM_TYPE_NORESULT };
+    addSyntheticUndefinedFunction(Config->Entry, &Signature);
 
     addSyntheticGlobal("__stack_pointer", 0);
   }
diff --git a/lld/wasm/Options.td b/lld/wasm/Options.td
index 3f81894a65c..12eef9b2b30 100644
--- a/lld/wasm/Options.td
+++ b/lld/wasm/Options.td
@@ -29,11 +29,15 @@ def no_threads: F<"no-threads">,
 def no_color_diagnostics: F<"no-color-diagnostics">,
   HelpText<"Do not use colors in diagnostics">;
 
+def no_check_signatures: F<"no-check-signatures">, HelpText<"Don't check function signatures">;
+
 def o: JoinedOrSeparate<["-"], "o">, MetaVarName<"<path>">,
   HelpText<"Path to file to write output">;
 
 def threads: F<"threads">, HelpText<"Run the linker multi-threaded">;
 
+def check_signatures: F<"check-signatures">, HelpText<"Check function signatures">;
+
 def v: Flag<["-"], "v">, HelpText<"Display the version number">;
 
 def version: F<"version">, HelpText<"Display the version number and exit">;
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index 72f6bdd8e8f..ea74b6584c4 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -11,6 +11,7 @@
 
 #include "Config.h"
 #include "Strings.h"
+#include "WriterUtils.h"
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
 
@@ -76,20 +77,59 @@ void SymbolTable::reportDuplicate(Symbol *Existing, InputFile *NewFile) {
         toString(NewFile));
 }
 
-static void checkSymbolTypes(Symbol *Existing, InputFile *F,
-                             const WasmSymbol *New) {
-  if (Existing->isLazy())
+// Get the signature for a given function symbol, either by looking
+// it up in function sections (for defined functions), of the imports section
+// (for imported functions).
+static const WasmSignature *getFunctionSig(const ObjFile &Obj,
+                                           const WasmSymbol &Sym) {
+  DEBUG(dbgs() << "getFunctionSig: " << Sym.Name << "\n");
+  const WasmObjectFile *WasmObj = Obj.getWasmObj();
+  uint32_t FunctionType;
+  if (Obj.isImportedFunction(Sym.ElementIndex)) {
+    const WasmImport &Import = WasmObj->imports()[Sym.ImportIndex];
+    FunctionType = Import.SigIndex;
+  } else {
+    uint32_t FuntionIndex = Sym.ElementIndex - Obj.NumFunctionImports();
+    FunctionType = WasmObj->functionTypes()[FuntionIndex];
+  }
+  return &WasmObj->types()[FunctionType];
+}
+
+// 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 checkSymbolTypes(const Symbol &Existing, const InputFile &F,
+                             const WasmSymbol &New,
+                             const WasmSignature *NewSig) {
+  if (Existing.isLazy())
+    return;
+
+  bool NewIsFunction = New.Type == WasmSymbol::SymbolType::FUNCTION_EXPORT ||
+                       New.Type == WasmSymbol::SymbolType::FUNCTION_IMPORT;
+
+  // First check the symbol types match (i.e. either both are function
+  // symbols or both are data symbols).
+  if (Existing.isFunction() != NewIsFunction) {
+    error("symbol type mismatch: " + New.Name + "\n>>> defined as " +
+          (Existing.isFunction() ? "Function" : "Global") + " in " +
+          toString(Existing.getFile()) + "\n>>> defined as " +
+          (NewIsFunction ? "Function" : "Global") + " in " + F.getName());
+    return;
+  }
+
+  // For function symbols, optionally check the function signature matches too.
+  if (!NewIsFunction || !Config->CheckSignatures)
     return;
 
-  bool NewIsFunction = New->Type == WasmSymbol::SymbolType::FUNCTION_EXPORT ||
-                       New->Type == WasmSymbol::SymbolType::FUNCTION_IMPORT;
-  if (Existing->isFunction() == NewIsFunction)
+  DEBUG(dbgs() << "checkSymbolTypes: " << New.Name << "\n");
+  assert(NewSig);
+
+  const WasmSignature &OldSig = Existing.getFunctionType();
+  if (*NewSig == OldSig)
     return;
 
-  error("symbol type mismatch: " + New->Name + "\n>>> defined as " +
-        (Existing->isFunction() ? "Function" : "Global") + " in " +
-        toString(Existing->getFile()) + "\n>>> defined as " +
-        (NewIsFunction ? "Function" : "Global") + " in " + F->getName());
+  error("function signature mismatch: " + New.Name + "\n>>> defined as " +
+        toString(OldSig) + " in " + toString(Existing.getFile()) +
+        "\n>>> defined as " + toString(*NewSig) + " in " + F.getName());
 }
 
 Symbol *SymbolTable::addDefinedGlobal(StringRef Name) {
@@ -110,19 +150,27 @@ Symbol *SymbolTable::addDefined(InputFile *F, const WasmSymbol *Sym,
   Symbol *S;
   bool WasInserted;
   Symbol::Kind Kind = Symbol::DefinedFunctionKind;
+  const WasmSignature *NewSig = nullptr;
   if (Sym->Type == WasmSymbol::SymbolType::GLOBAL_EXPORT)
     Kind = Symbol::DefinedGlobalKind;
+  else
+    NewSig = getFunctionSig(*cast<ObjFile>(F), *Sym);
 
   std::tie(S, WasInserted) = insert(Sym->Name);
   if (WasInserted) {
-    S->update(Kind, F, Sym, Segment);
+    S->update(Kind, F, Sym, Segment, NewSig);
+  } else if (S->isLazy()) {
+    // The existing symbol is lazy. Replace it without checking types since
+    // lazy symbols don't have any type information.
+    DEBUG(dbgs() << "replacing existing lazy symbol: " << Sym->Name << "\n");
+    S->update(Kind, F, Sym, Segment, NewSig);
   } else if (!S->isDefined()) {
     // The existing symbol table entry is undefined. The new symbol replaces
-    // it
+    // it, after checkign the type matches
     DEBUG(dbgs() << "resolving existing undefined symbol: " << Sym->Name
                  << "\n");
-    checkSymbolTypes(S, F, Sym);
-    S->update(Kind, F, Sym, Segment);
+    checkSymbolTypes(*S, *F, *Sym, NewSig);
+    S->update(Kind, F, Sym, Segment, NewSig);
   } else if (Sym->isWeak()) {
     // the new symbol is weak we can ignore it
     DEBUG(dbgs() << "existing symbol takes precensence\n");
@@ -130,7 +178,8 @@ Symbol *SymbolTable::addDefined(InputFile *F, const WasmSymbol *Sym,
     // the new symbol is not weak and the existing symbol is, so we replace
     // it
     DEBUG(dbgs() << "replacing existing weak symbol\n");
-    S->update(Kind, F, Sym, Segment);
+    checkSymbolTypes(*S, *F, *Sym, NewSig);
+    S->update(Kind, F, Sym, Segment, NewSig);
   } else {
     // niether symbol is week. They conflict.
     reportDuplicate(S, F);
@@ -138,14 +187,16 @@ Symbol *SymbolTable::addDefined(InputFile *F, const WasmSymbol *Sym,
   return S;
 }
 
-Symbol *SymbolTable::addUndefinedFunction(StringRef Name) {
+Symbol *SymbolTable::addUndefinedFunction(StringRef Name,
+                                          const WasmSignature *Type) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
-  if (WasInserted)
-    S->update(Symbol::UndefinedFunctionKind);
-  else if (!S->isFunction())
+  if (WasInserted) {
+    S->update(Symbol::UndefinedFunctionKind, nullptr, nullptr, nullptr, Type);
+  } else if (!S->isFunction()) {
     error("symbol type mismatch: " + Name);
+  }
   return S;
 }
 
@@ -154,18 +205,21 @@ Symbol *SymbolTable::addUndefined(InputFile *F, const WasmSymbol *Sym) {
   Symbol *S;
   bool WasInserted;
   Symbol::Kind Kind = Symbol::UndefinedFunctionKind;
+  const WasmSignature *NewSig = nullptr;
   if (Sym->Type == WasmSymbol::SymbolType::GLOBAL_IMPORT)
     Kind = Symbol::UndefinedGlobalKind;
+  else
+    NewSig = getFunctionSig(*cast<ObjFile>(F), *Sym);
   std::tie(S, WasInserted) = insert(Sym->Name);
   if (WasInserted) {
-    S->update(Kind, F, Sym);
+    S->update(Kind, F, Sym, nullptr, NewSig);
   } else if (S->isLazy()) {
     DEBUG(dbgs() << "resolved by existing lazy\n");
     auto *AF = cast<ArchiveFile>(S->getFile());
     AF->addMember(&S->getArchiveSymbol());
   } else if (S->isDefined()) {
     DEBUG(dbgs() << "resolved by existing\n");
-    checkSymbolTypes(S, F, Sym);
+    checkSymbolTypes(*S, *F, *Sym, NewSig);
   }
   return S;
 }
diff --git a/lld/wasm/SymbolTable.h b/lld/wasm/SymbolTable.h
index 89450266d89..9dfcb39b5f3 100644
--- a/lld/wasm/SymbolTable.h
+++ b/lld/wasm/SymbolTable.h
@@ -18,6 +18,7 @@
 #include "llvm/Support/raw_ostream.h"
 
 using llvm::object::WasmSymbol;
+using llvm::wasm::WasmSignature;
 
 namespace lld {
 namespace wasm {
@@ -51,7 +52,7 @@ public:
   Symbol *addDefined(InputFile *F, const WasmSymbol *Sym,
                      const InputSegment *Segment = nullptr);
   Symbol *addUndefined(InputFile *F, const WasmSymbol *Sym);
-  Symbol *addUndefinedFunction(StringRef Name);
+  Symbol *addUndefinedFunction(StringRef Name, const WasmSignature *Type);
   Symbol *addDefinedGlobal(StringRef Name);
   void addLazy(ArchiveFile *F, const Archive::Symbol *Sym);
 
diff --git a/lld/wasm/Symbols.cpp b/lld/wasm/Symbols.cpp
index 733fcf6ce4c..78b0de77cf0 100644
--- a/lld/wasm/Symbols.cpp
+++ b/lld/wasm/Symbols.cpp
@@ -31,19 +31,9 @@ uint32_t Symbol::getFunctionIndex() const {
   return Sym->ElementIndex;
 }
 
-uint32_t Symbol::getFunctionTypeIndex() const {
-  assert(Sym->isFunction());
-  ObjFile *Obj = cast<ObjFile>(File);
-  if (Obj->isImportedFunction(Sym->ElementIndex)) {
-    const WasmImport &Import = Obj->getWasmObj()->imports()[Sym->ImportIndex];
-    DEBUG(dbgs() << "getFunctionTypeIndex: import: " << Sym->ImportIndex
-                 << " -> " << Import.SigIndex << "\n");
-    return Import.SigIndex;
-  }
-  DEBUG(dbgs() << "getFunctionTypeIndex: non import: " << Sym->ElementIndex
-               << "\n");
-  uint32_t FuntionIndex = Sym->ElementIndex - Obj->NumFunctionImports();
-  return Obj->getWasmObj()->functionTypes()[FuntionIndex];
+const WasmSignature &Symbol::getFunctionType() const {
+  assert(FunctionType != nullptr);
+  return *FunctionType;
 }
 
 uint32_t Symbol::getVirtualAddress() const {
@@ -74,11 +64,12 @@ void Symbol::setOutputIndex(uint32_t Index) {
 }
 
 void Symbol::update(Kind K, InputFile *F, const WasmSymbol *WasmSym,
-                    const InputSegment *Seg) {
+                    const InputSegment *Seg, const WasmSignature *Sig) {
   SymbolKind = K;
   File = F;
   Sym = WasmSym;
   Segment = Seg;
+  FunctionType = Sig;
 }
 
 bool Symbol::isWeak() const { return Sym && Sym->isWeak(); }
diff --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h
index a642007bf5d..176a80061ac 100644
--- a/lld/wasm/Symbols.h
+++ b/lld/wasm/Symbols.h
@@ -16,6 +16,7 @@
 
 using llvm::object::Archive;
 using llvm::object::WasmSymbol;
+using llvm::wasm::WasmSignature;
 using llvm::wasm::WasmImport;
 using llvm::wasm::WasmExport;
 
@@ -40,7 +41,7 @@ public:
   };
 
   Symbol(StringRef Name, bool IsLocal)
-      : WrittenToSymtab(0), WrittenToNameSec(0), Name(Name), IsLocal(IsLocal) {}
+      : WrittenToSymtab(0), WrittenToNameSec(0), IsLocal(IsLocal), Name(Name) {}
 
   Kind getKind() const { return SymbolKind; }
 
@@ -66,7 +67,8 @@ public:
 
   uint32_t getGlobalIndex() const;
   uint32_t getFunctionIndex() const;
-  uint32_t getFunctionTypeIndex() const;
+
+  const WasmSignature &getFunctionType() const;
   uint32_t getOutputIndex() const;
 
   // Returns the virtual address of a defined global.
@@ -81,7 +83,8 @@ public:
   void setOutputIndex(uint32_t Index);
 
   void update(Kind K, InputFile *F = nullptr, const WasmSymbol *Sym = nullptr,
-              const InputSegment *Segment = nullptr);
+              const InputSegment *Segment = nullptr,
+              const WasmSignature *Sig = nullptr);
 
   void setArchiveSymbol(const Archive::Symbol &Sym) { ArchiveSymbol = Sym; }
   const Archive::Symbol &getArchiveSymbol() { return ArchiveSymbol; }
@@ -92,14 +95,16 @@ public:
   unsigned WrittenToNameSec : 1;
 
 protected:
+  unsigned IsLocal : 1;
+
   StringRef Name;
-  bool IsLocal;
   Archive::Symbol ArchiveSymbol = {nullptr, 0, 0};
   Kind SymbolKind = InvalidKind;
   InputFile *File = nullptr;
   const WasmSymbol *Sym = nullptr;
   const InputSegment *Segment = nullptr;
   llvm::Optional<uint32_t> OutputIndex;
+  const WasmSignature* FunctionType;
 };
 
 } // namespace wasm
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index fa9c0df6a05..169b234abe0 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -35,11 +35,6 @@ static constexpr int kStackAlignment = 16;
 
 namespace {
 
-// Needed for WasmSignatureDenseMapInfo
-bool operator==(const WasmSignature &LHS, const WasmSignature &RHS) {
-  return LHS.ReturnType == RHS.ReturnType && LHS.ParamTypes == RHS.ParamTypes;
-}
-
 // Traits for using WasmSignature in a DenseMap.
 struct WasmSignatureDenseMapInfo {
   static WasmSignature getEmptyKey() {
@@ -72,6 +67,7 @@ public:
 private:
   void openFile();
 
+  uint32_t getTypeIndex(const WasmSignature &Sig);
   void assignSymbolIndexes();
   void calculateImports();
   void calculateOffsets();
@@ -158,8 +154,8 @@ void Writer::createImportSection() {
     Import.Module = "env";
     Import.Field = Sym->getName();
     Import.Kind = WASM_EXTERNAL_FUNCTION;
-    auto *Obj = cast<ObjFile>(Sym->getFile());
-    Import.SigIndex = Obj->relocateTypeIndex(Sym->getFunctionTypeIndex());
+    assert(TypeIndices.count(Sym->getFunctionType()) > 0);
+    Import.SigIndex = TypeIndices.lookup(Sym->getFunctionType());
     writeImport(OS, Import);
   }
 
@@ -179,9 +175,6 @@ void Writer::createImportSection() {
     Import.Field = Sym->getName();
     Import.Kind = WASM_EXTERNAL_GLOBAL;
     Import.Global.Mutable = false;
-    assert(isa<ObjFile>(Sym->getFile()));
-    // TODO(sbc): Set type of this import
-    // ObjFile* Obj = dyn_cast<ObjFile>(Sym->getFile());
     Import.Global.Type = WASM_TYPE_I32; // Sym->getGlobalType();
     writeImport(OS, Import);
   }
@@ -632,17 +625,18 @@ void Writer::calculateImports() {
   }
 }
 
+uint32_t Writer::getTypeIndex(const WasmSignature &Sig) {
+  auto Pair = TypeIndices.insert(std::make_pair(Sig, Types.size()));
+  if (Pair.second)
+    Types.push_back(&Sig);
+  return Pair.first->second;
+}
+
 void Writer::calculateTypes() {
   for (ObjFile *File : Symtab->ObjectFiles) {
     File->TypeMap.reserve(File->getWasmObj()->types().size());
-    for (const WasmSignature &Sig : File->getWasmObj()->types()) {
-      auto Pair = TypeIndices.insert(std::make_pair(Sig, Types.size()));
-      if (Pair.second)
-        Types.push_back(&Sig);
-
-      // Now we map the input files index to the index in the linked output
-      File->TypeMap.push_back(Pair.first->second);
-    }
+    for (const WasmSignature &Sig : File->getWasmObj()->types())
+      File->TypeMap.push_back(getTypeIndex(Sig));
   }
 }
 
diff --git a/lld/wasm/WriterUtils.cpp b/lld/wasm/WriterUtils.cpp
index e3be081b3ad..5bdf0d2e3f6 100644
--- a/lld/wasm/WriterUtils.cpp
+++ b/lld/wasm/WriterUtils.cpp
@@ -184,3 +184,32 @@ void wasm::writeReloc(raw_ostream &OS, const OutputRelocation &Reloc) {
 }
 
 } // namespace lld
+
+std::string lld::toString(ValType Type) {
+  switch (Type) {
+  case ValType::I32:
+    return "I32";
+  case ValType::I64:
+    return "I64";
+  case ValType::F32:
+    return "F32";
+  case ValType::F64:
+    return "F64";
+  }
+  llvm_unreachable("Invalid wasm::ValType");
+}
+
+std::string lld::toString(const WasmSignature &Sig) {
+  SmallString<128> S("(");
+  for (uint32_t Type : Sig.ParamTypes) {
+    if (S.size() != 1)
+      S += ", ";
+    S += toString(static_cast<ValType>(Type));
+  }
+  S += ") -> ";
+  if (Sig.ReturnType == WASM_TYPE_NORESULT)
+    S += "void";
+  else
+    S += toString(static_cast<ValType>(Sig.ReturnType));
+  return S.str();
+}
diff --git a/lld/wasm/WriterUtils.h b/lld/wasm/WriterUtils.h
index 8721e7cd7ce..c1ed90793f7 100644
--- a/lld/wasm/WriterUtils.h
+++ b/lld/wasm/WriterUtils.h
@@ -16,6 +16,17 @@
 
 using llvm::raw_ostream;
 
+// Needed for WasmSignatureDenseMapInfo
+inline bool operator==(const llvm::wasm::WasmSignature &LHS,
+                       const llvm::wasm::WasmSignature &RHS) {
+  return LHS.ReturnType == RHS.ReturnType && LHS.ParamTypes == RHS.ParamTypes;
+}
+
+inline bool operator!=(const llvm::wasm::WasmSignature &LHS,
+                       const llvm::wasm::WasmSignature &RHS) {
+  return !(LHS == RHS);
+}
+
 namespace lld {
 namespace wasm {
 
@@ -58,6 +69,10 @@ void writeExport(raw_ostream &OS, const llvm::wasm::WasmExport &Export);
 void writeReloc(raw_ostream &OS, const OutputRelocation &Reloc);
 
 } // namespace wasm
+
+std::string toString(const llvm::wasm::ValType Type);
+std::string toString(const llvm::wasm::WasmSignature &Sig);
+
 } // namespace lld
 
 #endif // LLD_WASM_WRITERUTILS_H