[WebAssembly] Check function signatures at link time

[sbc100]

Summary

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 feaure have been
added: (--check-signatures/--no-check-signatures).

[sbc100]

Remove elf test

[sbc100]

• add unreachable

[sbc100]

• Fix exports
• don't export stack pointer

[sbc100]

• don't export stack pointer
• handle lazy symbols

[ruiu]

What is the motivation to add a new option to control the feature? I mean, is there a reason not to enable this feature by default?

[sbc100]

Its something that I/we would like to experiment with. Turning it on my default would be great, but it would currently break many/most C programs because there are common cases where functions are declare and defined with different signatures. Specifically in the musl C library which we use for a lot of our testing the main(), _init() and _fini() functions all have this issue. I'm trying to find a way to enable to this without have these common cases cause failure. In the mean time it is useful to have this feature behind a flag to that people can experiment with it. For example I imagine the rust folks might be want to turn this on as it can catch errors earlier and give better errors than the runtime errors that result.

[ruiu]

wasm/Config.h

26–33 ↗

(On Diff #124025)

Sort

wasm/Driver.cpp

296–297 ↗

(On Diff #124025)

We usually leave Config members uninitialized if they are overwritten unconditionally (so that we can catch errors using ubsan) and use false or true as the third parameter to hasFlags.

308 ↗

(On Diff #124025)

Is this related change?

wasm/Options.td

32 ↗

(On Diff #124025)

In other ports, we use the exact the same name for variables. I'd name this no_check_signatures.

39 ↗

(On Diff #124025)

Ditto -- check_signatures

wasm/SymbolTable.cpp

80 ↗

(On Diff #124025)

Please add a function comment.

98 ↗

(On Diff #124025)

Please add a function comment.

wasm/SymbolTable.h

56 ↗

(On Diff #124025)

Can you run clang-format-diff on this patch?

58 ↗

(On Diff #124025)

The definition of this function in a .cpp file doesn't have Mutable parameter. Does it compile?

63 ↗

(On Diff #124025)

We named this Symtab in ELF and COFF.

wasm/Symbols.h

106 ↗

(On Diff #124025)

How large is WasmSignature? I wonder if it copying a value is a right approach.

[sbc100]

• clang format
• review feedback

[sbc100]

wasm/Driver.cpp

308 ↗

(On Diff #124025)

I had thought it would make sense for the default value to be the one that the variable itself is initialized to, so I was trying to be consistent, but after your above comment it seems that its more desirable to set the defaults here in the code.

wasm/SymbolTable.h

58 ↗

(On Diff #124025)

Reverted.. this method was never referecnced

63 ↗

(On Diff #124025)

I found this confusing because there is also global with that name. If you agree I could make a separate change to the others too?

wasm/Symbols.h

106 ↗

(On Diff #124025)

Its a uint32_t for the return type and a uint32_t for each param type. The reason I ended up doing it this way is for synthetic symbols (mostly main() I guess) which don't have a signature we can point to in an input object.

Ideally this class would be split into separate subclass and stored as union like we do using the SymbolBody in the ELF linker. That way the we could reduce the size of the each symbol to sizeof the largest symbol type. If its OK with you I'll leave that as a later optimization.

[ruiu]

wasm/SymbolTable.h

63 ↗

(On Diff #124025)

Sure. I just wanted to make all the ports as consistent as possible.

wasm/Symbols.h

106 ↗

(On Diff #124025)

Then does it make sense to make it a pointer to WasmSignature (and represent "no value" as a nullptr)? You are handling pointers to WasmSignature in many places in this patch and copy it only when storing them to this variable, which seems a bit odd to me. If you can handle them as pointers, you can store them as pointers, no?

[sbc100]

• rebase on top of split out changes

[sbc100]

• Don't store sigs in symbols

[ruiu]

wasm/Driver.cpp

207 ↗

(On Diff #124495)

Can you add a brief comment about this? I think this is dummy data, and this code is not technically correct, as synthetic undefined symbols should have expected type, right?

wasm/SymbolTable.cpp

130–131 ↗

(On Diff #124495)

Why is this an exception?

[sbc100]

• review feedback

[sbc100]

wasm/SymbolTable.cpp

130–131 ↗

(On Diff #124495)

This was part of an experiment. Reverted.

[ruiu]

wasm/SymbolTable.cpp

130–131 ↗

(On Diff #124495)

Did you forgot to actually delete the code?

[sbc100]

• remove exception for main

[ruiu]

LGTM

wasm/Symbols.h

106 ↗

(On Diff #124594)

Please run clang-format-diff on this patch.

wasm/Writer.cpp

293 ↗

(On Diff #124594)

be exported because it's

300–306 ↗

(On Diff #124594)

If you flip the innermost if condition, you could write this as

if (ExportFuncs)
  for (Symbol *S : Symtab->getSymbols())
    if (S->isFunction() && !S->isWeak() && S->isDefined())
      ++NumExports;

341 ↗

(On Diff #124594)

Ditto

599–600 ↗

(On Diff #124594)

These two ifs are the same as one if (WasmFile->memories().size() > 1)

[sbc100]

• rebase

[sbc100]

• split out that other part of this change regarding exporting by default

[sbc100]

This new version of this change is a strict subset of the one you approved. Sorry for the churn, but I thought it better to go with this smaller change.

[ruiu]

Please go ahead and submit.

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

+; 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

lld/trunk/wasm/Config.h

 
 using llvm::wasm::WasmGlobal;
 
 namespace lld {
 namespace wasm {
 
 struct Configuration {
   bool AllowUndefined;
+  bool CheckSignatures;
   bool Demangle;
   bool EmitRelocs;
   bool ImportMemory;
   bool Relocatable;
   bool StripAll;
   bool StripDebug;
   uint32_t GlobalBase;
   uint32_t InitialMemory;

lld/trunk/wasm/Driver.cpp

static void addSyntheticGlobal(StringRef Name, int32_t Value) {
   Global.Type = WASM_TYPE_I32;
   Global.InitExpr.Opcode = WASM_OPCODE_I32_CONST;
   Global.InitExpr.Value.Int32 = Value;
   Config->SyntheticGlobals.emplace_back(S, Global);
 }
 
 // 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) {
   WasmOptTable Table;
   Table.PrintHelp(outs(), Argv0, "LLVM Linker", false);
 }
 
 WasmOptTable::WasmOptTable() : OptTable(OptInfo) {}
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   errorHandler().ErrorLimit = args::getInteger(Args, OPT_error_limit, 20);
 
   if (Args.hasArg(OPT_version) || Args.hasArg(OPT_v)) {
     outs() << getLLDVersion() << "\n";
     return;
   }
 
   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);
   Config->OutputFile = Args.getLastArgValue(OPT_o);
   Config->Relocatable = Args.hasArg(OPT_relocatable);
   Config->SearchPaths = args::getStrings(Args, OPT_L);
   Config->StripAll = Args.hasArg(OPT_strip_all);
   Config->StripDebug = Args.hasArg(OPT_strip_debug);
if (!Args.hasArg(OPT_INPUT))
     error("no input files");
 
   if (Config->Relocatable && !Config->Entry.empty())
     error("entry point specified for relocatable output file");
 
   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);
   }
 
   createFiles(Args);
   if (errorCount())
     return;
 

lld/trunk/wasm/Options.td

 def mllvm: S<"mllvm">, HelpText<"Options to pass to LLVM">;
 
 def no_threads: F<"no-threads">,
   HelpText<"Do not run the linker multi-threaded">;
 
 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">;
 
 def verbose: F<"verbose">, HelpText<"Verbose mode">;
 
 def relocatable: F<"relocatable">, HelpText<"Create relocatable object file">;
 

lld/trunk/wasm/SymbolTable.h

 #include "InputFiles.h"
 #include "Symbols.h"
 
 #include "llvm/ADT/CachedHashString.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/raw_ostream.h"
 
 using llvm::object::WasmSymbol;
+using llvm::wasm::WasmSignature;
 
 namespace lld {
 namespace wasm {
 
 class InputSegment;
 
 // SymbolTable is a bucket of all known symbols, including defined,
 // undefined, or lazy symbols (the last one is symbols in archive
public:
   void reportDuplicate(Symbol *Existing, InputFile *NewFile);
   void reportRemainingUndefines();
 
   Symbol *find(StringRef Name);
 
   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);
 
 private:
   std::pair<Symbol *, bool> insert(StringRef Name);
 
   llvm::DenseMap<llvm::CachedHashStringRef, Symbol *> SymMap;
 };
 
 extern SymbolTable *Symtab;
 
 } // namespace wasm
 } // namespace lld
 
 #endif

lld/trunk/wasm/SymbolTable.cpp

 //===- SymbolTable.cpp ----------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolTable.h"
 
 #include "Config.h"
 #include "Strings.h"
+#include "WriterUtils.h"
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
 
 #include <unordered_set>
 
 #define DEBUG_TYPE "lld"
 
 using namespace llvm;

 }
 
 void SymbolTable::reportDuplicate(Symbol *Existing, InputFile *NewFile) {
   error("duplicate symbol: " + toString(*Existing) + "\n>>> defined in " +
         toString(Existing->getFile()) + "\n>>> defined in " +
         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) {
   DEBUG(dbgs() << "addDefinedGlobal: " << Name << "\n");
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
   if (WasInserted)
     S->update(Symbol::DefinedGlobalKind);
   else if (!S->isGlobal())
     error("symbol type mismatch: " + Name);
   return S;
 }
 
 Symbol *SymbolTable::addDefined(InputFile *F, const WasmSymbol *Sym,
                                 const InputSegment *Segment) {
   DEBUG(dbgs() << "addDefined: " << Sym->Name << "\n");
   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");
   } else if (S->isWeak()) {
     // 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);
   }
   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;
 }
 
 Symbol *SymbolTable::addUndefined(InputFile *F, const WasmSymbol *Sym) {
   DEBUG(dbgs() << "addUndefined: " << displayName(Sym->Name) << "\n");
   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;
 }
 
 void SymbolTable::addLazy(ArchiveFile *F, const Archive::Symbol *Sym) {
   DEBUG(dbgs() << "addLazy: " << displayName(Sym->getName()) << "\n");
   StringRef Name = Sym->getName();
   Symbol *S;

lld/trunk/wasm/Symbols.h

 #define LLD_WASM_SYMBOLS_H
 
 #include "lld/Common/LLVM.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/Wasm.h"
 
 using llvm::object::Archive;
 using llvm::object::WasmSymbol;
+using llvm::wasm::WasmSignature;
 using llvm::wasm::WasmImport;
 using llvm::wasm::WasmExport;
 
 namespace lld {
 namespace wasm {
 
 class InputFile;
 class InputSegment;
 
 class Symbol {
 public:
   enum Kind {
     DefinedFunctionKind,
     DefinedGlobalKind,
 
     LazyKind,
     UndefinedFunctionKind,
     UndefinedGlobalKind,
 
     LastDefinedKind = DefinedGlobalKind,
     InvalidKind,
   };
 
   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; }
 
   bool isLazy() const { return SymbolKind == LazyKind; }
   bool isDefined() const { return SymbolKind <= LastDefinedKind; }
   bool isUndefined() const {
     return SymbolKind == UndefinedGlobalKind ||
            SymbolKind == UndefinedFunctionKind;
public:
   // 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 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.
   // Only works for globals, not functions.
   uint32_t getVirtualAddress() const;
 
   // Returns true if an output index has been set for this symbol
   bool hasOutputIndex() { return OutputIndex.hasValue(); }
 
   // Set the output index of the symbol (in the function or global index
   // space of the output object.
   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; }
 
   // This bit is used by Writer::writeNameSection() to prevent
   // symbols from being written to the symbol table more than once.
   unsigned WrittenToSymtab : 1;
   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
 
 // Returns a symbol name for an error message.
 std::string toString(wasm::Symbol &Sym);
 std::string toString(wasm::Symbol::Kind &Kind);
 
 } // namespace lld
 
 #endif

lld/trunk/wasm/Symbols.cpp

uint32_t Symbol::getGlobalIndex() const {
   return Sym->ElementIndex;
 }
 
 uint32_t Symbol::getFunctionIndex() const {
   assert(Sym->isFunction());
   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 {
   assert(isGlobal());
   DEBUG(dbgs() << "getVirtualAddress: " << getName() << "\n");
   if (isUndefined())
     return UINT32_MAX;
 

 
 void Symbol::setOutputIndex(uint32_t Index) {
   DEBUG(dbgs() << "setOutputIndex " << Name << " -> " << Index << "\n");
   assert(!hasOutputIndex());
   OutputIndex = 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(); }
 
 std::string lld::toString(wasm::Symbol &Sym) {
   return wasm::displayName(Sym.getName());
 }
 

lld/trunk/wasm/Writer.cpp

 using namespace llvm::wasm;
 using namespace lld;
 using namespace lld::wasm;
 
 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() {
     WasmSignature Sig;
     Sig.ReturnType = 1;
     return Sig;
   }
   static WasmSignature getTombstoneKey() {

 // The writer writes a SymbolTable result to a file.
 class Writer {
 public:
   void run();
 
 private:
   void openFile();
 
+  uint32_t getTypeIndex(const WasmSignature &Sig);
   void assignSymbolIndexes();
   void calculateImports();
   void calculateOffsets();
   void calculateTypes();
   void createOutputSegments();
   void layoutMemory();
   void createHeader();
   void createSections();
void Writer::createImportSection() {
 
   writeUleb128(OS, NumImports, "import count");
 
   for (Symbol *Sym : FunctionImports) {
     WasmImport Import;
     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);
   }
 
   if (Config->ImportMemory) {
     WasmImport Import;
     Import.Module = "env";
     Import.Field = "memory";
     Import.Kind = WASM_EXTERNAL_MEMORY;
     Import.Memory.Flags = 0;
     Import.Memory.Initial = NumMemoryPages;
     writeImport(OS, Import);
   }
 
   for (Symbol *Sym : GlobalImports) {
     WasmImport Import;
     Import.Module = "env";
     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);
   }
 }
 
 void Writer::createTypeSection() {
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_TYPE);
   raw_ostream &OS = Section->getStream();
for (Symbol *Sym : File->getSymbols()) {
       } else {
         Sym->setOutputIndex(GlobalImports.size());
         GlobalImports.push_back(Sym);
       }
     }
   }
 }
 
-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);
-    }
+uint32_t Writer::getTypeIndex(const WasmSignature &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())
+      File->TypeMap.push_back(getTypeIndex(Sig));
   }
 }
 
 void Writer::assignSymbolIndexes() {
   for (ObjFile *File : Symtab->ObjectFiles) {
     DEBUG(dbgs() << "assignSymbolIndexes: " << File->getName() << "\n");
     for (Symbol *Sym : File->getSymbols()) {
       if (Sym->hasOutputIndex() || !Sym->isDefined())

lld/trunk/wasm/WriterUtils.h

 #define LLD_WASM_WRITERUTILS_H
 
 #include "llvm/ADT/Twine.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/raw_ostream.h"
 
 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 {
 
 struct OutputRelocation {
   llvm::wasm::WasmRelocation Reloc;
   uint32_t NewIndex;
   uint32_t Value;
 };

 
 void writeImport(raw_ostream &OS, const llvm::wasm::WasmImport &Import);
 
 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

lld/trunk/wasm/WriterUtils.cpp

case R_WEBASSEMBLY_MEMORY_ADDR_I32:
     writeUleb128(OS, Reloc.Reloc.Addend, "reloc addend");
     break;
   default:
     break;
   }
 }
 
 } // 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();
+}