[WebAssembly] LTO: Fix signatures of bitcode symbols

[sbc100]

Summary

Function symbols that come from bitcode have not signatures.
After LTO when the real symbols are read in we need to make
sure that we set the signature on the existing symbol.
the signature-less undefined functions.

[sbc100]

• fix test

[sbc100]

• cleanup

[ruiu]

LGTM

wasm/Driver.cpp

260–262

Do you need this assert? I think if it is a nullptr, it will crash immediately on the next line, so that assert() doesn't seem that useful.

wasm/Symbols.h

129

nit: insert a blank line before "protected:".

[dexonsmith]

wasm/Driver.cpp

260–262

I don't think it will crash here without some sort of sanitizer. Saving a reference in Sig doesn't load from the pointer.

test/wasm/lto/undef.ll

+; RUN: llvm-as %s -o %t.o
+; RUN: wasm-ld %t.o -o %t.wasm --allow-undefined
+; RUN: obj2yaml %t.wasm | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+declare void @bar()
+
+define void @_start() {
+  call void @bar()
+  ret void
+}
+
+; CHECK:       - Type:            IMPORT
+; CHECK-NEXT:    Imports:         
+; CHECK-NEXT:      - Module:          env
+; CHECK-NEXT:        Field:           bar
+; CHECK-NEXT:        Kind:            FUNCTION
+; CHECK-NEXT:        SigIndex:        0

wasm/Driver.cpp

for (Symbol *Sym : Symtab->getSymbols()) {
     if (!Sym->isUndefined() || !Sym->isWeak())
       continue;
     auto *FuncSym = dyn_cast<FunctionSymbol>(Sym);
     if (!FuncSym)
       continue;
 
     // 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->getFunctionType());
-    const WasmSignature &Sig = *FuncSym->getFunctionType();
+    assert(FuncSym->FunctionType);
+    const WasmSignature &Sig = *FuncSym->FunctionType;
 
     // Add a synthetic dummy for weak undefined functions.  These dummies will
     // be GC'd if not used as the target of any "call" instructions.
     Optional<std::string> SymName = demangleItanium(Sym->getName());
     StringRef DebugName =
         Saver.save("undefined function " +
                    (SymName ? StringRef(*SymName) : Sym->getName()));
     SyntheticFunction *Func =
         make<SyntheticFunction>(Sig, Sym->getName(), DebugName);

wasm/SymbolTable.cpp

 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));
 }
 
-static void checkFunctionType(const Symbol *Existing, const InputFile *File,
+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;
   }
 
-  const WasmSignature *OldSig = ExistingFunction->getFunctionType();
-  if (OldSig && NewSig && *NewSig != *OldSig) {
+  if (!NewSig)
+    return;
+
+  const WasmSignature *OldSig = ExistingFunction->FunctionType;
+  if (!OldSig) {
+    ExistingFunction->FunctionType = NewSig;
+    return;
+  }
+
+  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));
-  }
-}
 
 // 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 checkGlobalType(const Symbol *Existing, const InputFile *File,
                             const WasmGlobalType *NewType) {
   if (!isa<GlobalSymbol>(Existing)) {
     reportTypeError(Existing, File, WASM_SYMBOL_TYPE_GLOBAL);
     return;
Symbol *SymbolTable::addUndefinedFunction(StringRef Name, uint32_t Flags,
 
   if (!File || File->kind() == InputFile::ObjectKind)
     S->IsUsedInRegularObj = true;
 
   if (WasInserted)
     replaceSymbol<UndefinedFunction>(S, Name, Flags, File, Sig);
   else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     Lazy->fetch();
-  else if (S->isDefined())
+  else
     checkFunctionType(S, File, Sig);
+
   return S;
 }
 
 Symbol *SymbolTable::addUndefinedData(StringRef Name, uint32_t Flags,
                                       InputFile *File) {
   LLVM_DEBUG(dbgs() << "addUndefinedData: " << Name << "\n");
 
   Symbol *S;

wasm/Symbols.h

 
 class FunctionSymbol : public Symbol {
 public:
   static bool classof(const Symbol *S) {
     return S->kind() == DefinedFunctionKind ||
            S->kind() == UndefinedFunctionKind;
   }
 
-  const WasmSignature *getFunctionType() const { return FunctionType; }
-
   // Get/set the table index
   void setTableIndex(uint32_t Index);
   uint32_t getTableIndex() const;
   bool hasTableIndex() const;
 
   // Get/set the function index
   uint32_t getFunctionIndex() const;
   void setFunctionIndex(uint32_t Index);
   bool hasFunctionIndex() const;
 
+  const WasmSignature *FunctionType;
+
 protected:
   FunctionSymbol(StringRef Name, Kind K, uint32_t Flags, InputFile *F,
                  const WasmSignature *Type)
       : Symbol(Name, K, Flags, F), FunctionType(Type) {}
 
   uint32_t TableIndex = INVALID_INDEX;
   uint32_t FunctionIndex = INVALID_INDEX;
-
-  const WasmSignature *FunctionType;
 };
 
 class DefinedFunction : public FunctionSymbol {
 public:
   DefinedFunction(StringRef Name, uint32_t Flags, InputFile *F,
                   InputFunction *Function);
 
   static bool classof(const Symbol *S) {

wasm/Writer.cpp

void Writer::createImportSection() {
   }
 
   for (const Symbol *Sym : ImportedSymbols) {
     WasmImport Import;
     Import.Module = "env";
     Import.Field = Sym->getName();
     if (auto *FunctionSym = dyn_cast<FunctionSymbol>(Sym)) {
       Import.Kind = WASM_EXTERNAL_FUNCTION;
-      Import.SigIndex = lookupType(*FunctionSym->getFunctionType());
+      Import.SigIndex = lookupType(*FunctionSym->FunctionType);
     } else {
       auto *GlobalSym = cast<GlobalSymbol>(Sym);
       Import.Kind = WASM_EXTERNAL_GLOBAL;
       Import.Global = *GlobalSym->getGlobalType();
     }
     writeImport(OS, Import);
   }
 }
for (ObjFile *File : Symtab->ObjectFiles) {
     ArrayRef<WasmSignature> Types = File->getWasmObj()->types();
     for (uint32_t I = 0; I < Types.size(); I++)
       if (File->TypeIsUsed[I])
         File->TypeMap[I] = registerType(Types[I]);
   }
 
   for (const Symbol *Sym : ImportedSymbols)
     if (auto *F = dyn_cast<FunctionSymbol>(Sym))
-      registerType(*F->getFunctionType());
+      registerType(*F->FunctionType);
 
   for (const InputFunction *F : InputFunctions)
     registerType(F->Signature);
 }
 
 void Writer::assignIndexes() {
   uint32_t FunctionIndex = NumImportedFunctions + InputFunctions.size();
   auto AddDefinedFunction = [&](InputFunction *Func) {

 // Populate InitFunctions vector with init functions from all input objects.
 // This is then used either when creating the output linking section or to
 // synthesize the "__wasm_call_ctors" function.
 void Writer::calculateInitFunctions() {
   for (ObjFile *File : Symtab->ObjectFiles) {
     const WasmLinkingData &L = File->getWasmObj()->linkingData();
     for (const WasmInitFunc &F : L.InitFunctions) {
       FunctionSymbol *Sym = File->getFunctionSymbol(F.Symbol);
-      if (*Sym->getFunctionType() != WasmSignature{{}, WASM_TYPE_NORESULT})
+      if (*Sym->FunctionType != WasmSignature{{}, WASM_TYPE_NORESULT})
         error("invalid signature for init func: " + toString(*Sym));
       InitFunctions.emplace_back(WasmInitEntry{Sym, F.Priority});
     }
   }
 
   // Sort in order of priority (lowest first) so that they are called
   // in the correct order.
   std::stable_sort(InitFunctions.begin(), InitFunctions.end(),