[lld] r326271 - [WebAssembly] Separate addUndefined into addUndefined{Function, Data, Global}.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 27 16:09:23 PST 2018


Author: ruiu
Date: Tue Feb 27 16:09:22 2018
New Revision: 326271

URL: http://llvm.org/viewvc/llvm-project?rev=326271&view=rev
Log:
[WebAssembly] Separate addUndefined into addUndefined{Function,Data,Global}.

Previously, one function adds all types of undefined symbols. That
doesn't fit to the wasm's undefined symbol semantics well because
different types of undefined symbols are very different in wasm.
As a result, separate control flows merge in this addUndefined function
and then separate again for each type. That wasn't easy to read.

This patch separates the function into three functions. Now it is pretty
clear what we are doing for each undefined symbol type.

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

Modified:
    lld/trunk/wasm/Driver.cpp
    lld/trunk/wasm/InputFiles.cpp
    lld/trunk/wasm/SymbolTable.cpp
    lld/trunk/wasm/SymbolTable.h

Modified: lld/trunk/wasm/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/Driver.cpp?rev=326271&r1=326270&r2=326271&view=diff
==============================================================================
--- lld/trunk/wasm/Driver.cpp (original)
+++ lld/trunk/wasm/Driver.cpp Tue Feb 27 16:09:22 2018
@@ -214,11 +214,6 @@ static StringRef getEntry(opt::InputArgL
   return Arg->getValue();
 }
 
-static Symbol *addUndefinedFunction(StringRef Name, const WasmSignature *Type) {
-  return Symtab->addUndefined(Name, WASM_SYMBOL_TYPE_FUNCTION, 0, nullptr,
-                              Type);
-}
-
 void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   WasmOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -311,11 +306,12 @@ void LinkerDriver::link(ArrayRef<const c
     WasmSym::DataEnd = Symtab->addSyntheticDataSymbol("__data_end");
 
     if (!Config->Entry.empty())
-      EntrySym = addUndefinedFunction(Config->Entry, &NullSignature);
+      EntrySym = Symtab->addUndefinedFunction(Config->Entry, 0, nullptr,
+                                              &NullSignature);
 
     // Handle the `--undefined <sym>` options.
     for (auto* Arg : Args.filtered(OPT_undefined))
-      addUndefinedFunction(Arg->getValue(), nullptr);
+      Symtab->addUndefinedFunction(Arg->getValue(), 0, nullptr, nullptr);
   }
 
   createFiles(Args);

Modified: lld/trunk/wasm/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/InputFiles.cpp?rev=326271&r1=326270&r2=326271&view=diff
==============================================================================
--- lld/trunk/wasm/InputFiles.cpp (original)
+++ lld/trunk/wasm/InputFiles.cpp Tue Feb 27 16:09:22 2018
@@ -268,9 +268,18 @@ void ObjFile::initializeSymbols() {
 }
 
 Symbol *ObjFile::createUndefined(const WasmSymbol &Sym) {
-  return Symtab->addUndefined(
-      Sym.Info.Name, static_cast<WasmSymbolType>(Sym.Info.Kind), Sym.Info.Flags,
-      this, Sym.FunctionType, Sym.GlobalType);
+  StringRef Name = Sym.Info.Name;
+  uint32_t Flags = Sym.Info.Flags;
+
+  switch (Sym.Info.Kind) {
+  case WASM_SYMBOL_TYPE_FUNCTION:
+    return Symtab->addUndefinedFunction(Name, Flags, this, Sym.FunctionType);
+  case WASM_SYMBOL_TYPE_DATA:
+    return Symtab->addUndefinedData(Name, Flags, this);
+  case WASM_SYMBOL_TYPE_GLOBAL:
+    return Symtab->addUndefinedGlobal(Name, Flags, this, Sym.GlobalType);
+  }
+  llvm_unreachable("unkown symbol kind");
 }
 
 Symbol *ObjFile::createDefinedFunction(const WasmSymbol &Sym,

Modified: lld/trunk/wasm/SymbolTable.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/SymbolTable.cpp?rev=326271&r1=326270&r2=326271&view=diff
==============================================================================
--- lld/trunk/wasm/SymbolTable.cpp (original)
+++ lld/trunk/wasm/SymbolTable.cpp Tue Feb 27 16:09:22 2018
@@ -71,63 +71,56 @@ std::pair<Symbol *, bool> SymbolTable::i
   return {Sym, true};
 }
 
-// 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,
-                             WasmSymbolType NewType,
-                             const WasmSignature *NewFunctionSig,
-                             const WasmGlobalType *NewGlobalType) {
-  if (Existing.isLazy())
-    return;
-
-  WasmSymbolType ExistingType = Existing.getWasmType();
+static void reportTypeError(const Symbol *Existing, const InputFile *File,
+                            StringRef Type) {
+  error("symbol type mismatch: " + toString(*Existing) + "\n>>> defined as " +
+        toString(Existing->getWasmType()) + " in " +
+        toString(Existing->getFile()) + "\n>>> defined as " + Type + " in " +
+        toString(File));
+}
 
-  // First check the symbol types match (i.e. either both are function
-  // symbols or both are data symbols).
-  if (NewType != ExistingType) {
-    error("symbol type mismatch: " + Existing.getName() + "\n>>> defined as " +
-          toString(ExistingType) + " in " + toString(Existing.getFile()) +
-          "\n>>> defined as " + toString(NewType) + " in " + F.getName());
+static void checkFunctionType(const Symbol *Existing, const InputFile *File,
+                              const WasmSignature *NewSig) {
+  if (!isa<FunctionSymbol>(Existing)) {
+    reportTypeError(Existing, File, "Function");
     return;
   }
 
-  // For function/global symbols, optionally check the type matches too.
-  if (NewType == WASM_SYMBOL_TYPE_DATA || !Config->CheckSignatures)
+  if (!Config->CheckSignatures)
     return;
 
-  DEBUG(dbgs() << "checkSymbolTypes: " << Existing.getName() << "\n");
+  const WasmSignature *OldSig =
+      cast<FunctionSymbol>(Existing)->getFunctionType();
+  if (OldSig && *NewSig != *OldSig) {
+    error("Function type mismatch: " + Existing->getName() +
+          "\n>>> defined as " + toString(*OldSig) + " in " +
+          toString(Existing->getFile()) + "\n>>> defined as " +
+          toString(*NewSig) + " in " + toString(File));
+  }
+}
 
-  auto ReportError = [&](const Twine &Old, const Twine &New) {
-    error(toString(NewType) + " type mismatch: " + Existing.getName() +
-          "\n>>> defined as " + Old + " in " + toString(Existing.getFile()) +
-          "\n>>> defined as " + New + " in " + F.getName());
-  };
-
-  if (NewType == WASM_SYMBOL_TYPE_FUNCTION) {
-    // Skip the signature check if the existing function has no signature (e.g.
-    // if it is an undefined symbol generated by --undefined command line flag).
-    auto &Sym = cast<FunctionSymbol>(Existing);
-    const WasmSignature *OldSig = Sym.getFunctionType();
-    if (!OldSig)
-      return;
-
-    assert(NewFunctionSig);
-    if (*NewFunctionSig == *OldSig)
-      return;
-
-    ReportError(toString(*OldSig), toString(*NewFunctionSig));
-  } else {
-    auto &Sym = cast<GlobalSymbol>(Existing);
-
-    assert(NewGlobalType != nullptr);
-    const WasmGlobalType *OldType = Sym.getGlobalType();
-    if (*NewGlobalType == *OldType)
-      return;
+// 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, "Global");
+    return;
+  }
 
-    ReportError(toString(*OldType), toString(*NewGlobalType));
+  const WasmGlobalType *OldType = cast<GlobalSymbol>(Existing)->getGlobalType();
+  if (*NewType != *OldType) {
+    error("Global type mismatch: " + Existing->getName() + "\n>>> defined as " +
+          toString(*OldType) + " in " + toString(Existing->getFile()) +
+          "\n>>> defined as " + toString(*NewType) + " in " + toString(File));
   }
 }
 
+static void checkDataType(const Symbol *Existing, const InputFile *File) {
+  if (!isa<DataSymbol>(Existing))
+    reportTypeError(Existing, File, "Data");
+}
+
 DefinedFunction *SymbolTable::addSyntheticFunction(StringRef Name,
                                                    const WasmSignature *Type,
                                                    uint32_t Flags) {
@@ -159,29 +152,12 @@ DefinedGlobal *SymbolTable::addSynthetic
   return replaceSymbol<DefinedGlobal>(S, Name, Flags, nullptr, Global);
 }
 
-static bool shouldReplace(const Symbol &Existing, InputFile *NewFile,
-                          WasmSymbolType NewType, uint32_t NewFlags,
-                          const WasmSignature *NewFuncType = nullptr,
-                          const WasmGlobalType *NewGlobalType = nullptr) {
-
-  // If existing symbol is lazy, replace it without checking types since
-  // lazy symbols don't have any type information.
-  if (Existing.isLazy()) {
-    DEBUG(dbgs() << "replacing existing lazy symbol: " << Existing.getName()
-                 << "\n");
-    return true;
-  }
-
-  // Now we have two wasm symbols, and all wasm symbols that have the same
-  // symbol name must have the same type, even if they are undefined. This
-  // is different from ELF because symbol types are not that significant
-  // in ELF, and undefined symbols in ELF don't have type in the first place.
-  checkSymbolTypes(Existing, *NewFile, NewType, NewFuncType, NewGlobalType);
-
+static bool shouldReplace(const Symbol *Existing, InputFile *NewFile,
+                          uint32_t NewFlags) {
   // If existing symbol is undefined, replace it.
-  if (!Existing.isDefined()) {
+  if (!Existing->isDefined()) {
     DEBUG(dbgs() << "resolving existing undefined symbol: "
-                 << Existing.getName() << "\n");
+                 << Existing->getName() << "\n");
     return true;
   }
 
@@ -192,93 +168,131 @@ static bool shouldReplace(const Symbol &
   }
 
   // If the existing symbol is weak, we should replace it.
-  if (Existing.isWeak()) {
+  if (Existing->isWeak()) {
     DEBUG(dbgs() << "replacing existing weak symbol\n");
     return true;
   }
 
   // Neither symbol is week. They conflict.
-  error("duplicate symbol: " + toString(Existing) + "\n>>> defined in " +
-        toString(Existing.getFile()) + "\n>>> defined in " + toString(NewFile));
+  error("duplicate symbol: " + toString(*Existing) + "\n>>> defined in " +
+        toString(Existing->getFile()) + "\n>>> defined in " +
+        toString(NewFile));
   return true;
 }
 
 Symbol *SymbolTable::addDefinedFunction(StringRef Name, uint32_t Flags,
-                                        InputFile *F, InputFunction *Function) {
+                                        InputFile *File,
+                                        InputFunction *Function) {
   DEBUG(dbgs() << "addDefinedFunction: " << Name << "\n");
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
-  if (WasInserted || shouldReplace(*S, F, WASM_SYMBOL_TYPE_FUNCTION, Flags,
-                                   &Function->Signature))
-    replaceSymbol<DefinedFunction>(S, Name, Flags, F, Function);
+
+  if (WasInserted || S->isLazy()) {
+    replaceSymbol<DefinedFunction>(S, Name, Flags, File, Function);
+    return S;
+  }
+
+  checkFunctionType(S, File, &Function->Signature);
+
+  if (shouldReplace(S, File, Flags))
+    replaceSymbol<DefinedFunction>(S, Name, Flags, File, Function);
   return S;
 }
 
 Symbol *SymbolTable::addDefinedData(StringRef Name, uint32_t Flags,
-                                    InputFile *F, InputSegment *Segment,
+                                    InputFile *File, InputSegment *Segment,
                                     uint32_t Address, uint32_t Size) {
   DEBUG(dbgs() << "addDefinedData:" << Name << " addr:" << Address << "\n");
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
-  if (WasInserted || shouldReplace(*S, F, WASM_SYMBOL_TYPE_DATA, Flags))
-    replaceSymbol<DefinedData>(S, Name, Flags, F, Segment, Address, Size);
+
+  if (WasInserted || S->isLazy()) {
+    replaceSymbol<DefinedData>(S, Name, Flags, File, Segment, Address, Size);
+    return S;
+  }
+
+  checkDataType(S, File);
+
+  if (shouldReplace(S, File, Flags))
+    replaceSymbol<DefinedData>(S, Name, Flags, File, Segment, Address, Size);
   return S;
 }
 
 Symbol *SymbolTable::addDefinedGlobal(StringRef Name, uint32_t Flags,
-                                      InputFile *F, InputGlobal *Global) {
+                                      InputFile *File, InputGlobal *Global) {
   DEBUG(dbgs() << "addDefinedGlobal:" << Name << "\n");
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
-  if (WasInserted || shouldReplace(*S, F, WASM_SYMBOL_TYPE_GLOBAL, Flags,
-                                   nullptr, &Global->getType()))
-    replaceSymbol<DefinedGlobal>(S, Name, Flags, F, Global);
+
+  if (WasInserted || S->isLazy()) {
+    replaceSymbol<DefinedGlobal>(S, Name, Flags, File, Global);
+    return S;
+  }
+
+  checkGlobalType(S, File, &Global->getType());
+
+  if (shouldReplace(S, File, Flags))
+    replaceSymbol<DefinedGlobal>(S, Name, Flags, File, Global);
   return S;
 }
 
-Symbol *SymbolTable::addUndefined(StringRef Name, WasmSymbolType Type,
-                                  uint32_t Flags, InputFile *F,
-                                  const WasmSignature *FunctionType,
-                                  const WasmGlobalType *GlobalType) {
-  DEBUG(dbgs() << "addUndefined type=" << Type << ": " << Name << "\n");
+Symbol *SymbolTable::addUndefinedFunction(StringRef Name, uint32_t Flags,
+                                          InputFile *File,
+                                          const WasmSignature *Sig) {
+  DEBUG(dbgs() << "addUndefinedFunction: " << Name << "\n");
 
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
 
-  if (WasInserted) {
-    switch (Type) {
-    case WASM_SYMBOL_TYPE_FUNCTION:
-      replaceSymbol<UndefinedFunction>(S, Name, Flags, F, FunctionType);
-      break;
-    case WASM_SYMBOL_TYPE_GLOBAL:
-      replaceSymbol<UndefinedGlobal>(S, Name, Flags, F, GlobalType);
-      break;
-    case WASM_SYMBOL_TYPE_DATA:
-      replaceSymbol<UndefinedData>(S, Name, Flags, F);
-      break;
-    }
-    return S;
-  }
+  if (WasInserted)
+    replaceSymbol<UndefinedFunction>(S, Name, Flags, File, Sig);
+  else if (auto *Lazy = dyn_cast<LazySymbol>(S))
+    cast<ArchiveFile>(Lazy->getFile())->addMember(&Lazy->getArchiveSymbol());
+  else if (S->isDefined())
+    checkFunctionType(S, File, Sig);
+  return S;
+}
+
+Symbol *SymbolTable::addUndefinedData(StringRef Name, uint32_t Flags,
+                                      InputFile *File) {
+  DEBUG(dbgs() << "addUndefinedData: " << Name << "\n");
+
+  Symbol *S;
+  bool WasInserted;
+  std::tie(S, WasInserted) = insert(Name);
 
-  if (auto *Lazy = dyn_cast<LazySymbol>(S)) {
-    DEBUG(dbgs() << "resolved by existing lazy\n");
+  if (WasInserted)
+    replaceSymbol<UndefinedData>(S, Name, Flags, File);
+  else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     cast<ArchiveFile>(Lazy->getFile())->addMember(&Lazy->getArchiveSymbol());
-    return S;
-  }
+  else if (S->isDefined())
+    checkDataType(S, File);
+  return S;
+}
 
-  if (S->isDefined()) {
-    DEBUG(dbgs() << "resolved by existing\n");
-    checkSymbolTypes(*S, *F, Type, FunctionType, GlobalType);
-  }
+Symbol *SymbolTable::addUndefinedGlobal(StringRef Name, uint32_t Flags,
+                                        InputFile *File,
+                                        const WasmGlobalType *Type) {
+  DEBUG(dbgs() << "addUndefinedGlobal: " << Name << "\n");
+
+  Symbol *S;
+  bool WasInserted;
+  std::tie(S, WasInserted) = insert(Name);
 
+  if (WasInserted)
+    replaceSymbol<UndefinedGlobal>(S, Name, Flags, File, Type);
+  else if (auto *Lazy = dyn_cast<LazySymbol>(S))
+    cast<ArchiveFile>(Lazy->getFile())->addMember(&Lazy->getArchiveSymbol());
+  else if (S->isDefined())
+    checkGlobalType(S, File, Type);
   return S;
 }
 
-void SymbolTable::addLazy(ArchiveFile *F, const Archive::Symbol *Sym) {
+void SymbolTable::addLazy(ArchiveFile *File, const Archive::Symbol *Sym) {
   DEBUG(dbgs() << "addLazy: " << Sym->getName() << "\n");
   StringRef Name = Sym->getName();
 
@@ -287,14 +301,14 @@ void SymbolTable::addLazy(ArchiveFile *F
   std::tie(S, WasInserted) = insert(Name);
 
   if (WasInserted) {
-    replaceSymbol<LazySymbol>(S, Name, F, *Sym);
+    replaceSymbol<LazySymbol>(S, Name, File, *Sym);
     return;
   }
 
   // If there is an existing undefined symbol, load a new one from the archive.
   if (S->isUndefined()) {
     DEBUG(dbgs() << "replacing existing undefined\n");
-    F->addMember(Sym);
+    File->addMember(Sym);
   }
 }
 

Modified: lld/trunk/wasm/SymbolTable.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/SymbolTable.h?rev=326271&r1=326270&r2=326271&view=diff
==============================================================================
--- lld/trunk/wasm/SymbolTable.h (original)
+++ lld/trunk/wasm/SymbolTable.h Tue Feb 27 16:09:22 2018
@@ -48,17 +48,20 @@ public:
   Symbol *find(StringRef Name);
   ObjFile *findComdat(StringRef Name) const;
 
-  Symbol *addDefinedFunction(StringRef Name, uint32_t Flags, InputFile *F,
+  Symbol *addDefinedFunction(StringRef Name, uint32_t Flags, InputFile *File,
                              InputFunction *Function = nullptr);
-  Symbol *addDefinedData(StringRef Name, uint32_t Flags, InputFile *F,
+  Symbol *addDefinedData(StringRef Name, uint32_t Flags, InputFile *File,
                          InputSegment *Segment = nullptr, uint32_t Address = 0,
                          uint32_t Size = 0);
-  Symbol *addDefinedGlobal(StringRef Name, uint32_t Flags, InputFile *F,
+  Symbol *addDefinedGlobal(StringRef Name, uint32_t Flags, InputFile *File,
                            InputGlobal *G);
-  Symbol *addUndefinedFunction(StringRef Name, const WasmSignature *Type);
-  Symbol *addUndefined(StringRef Name, WasmSymbolType Type, uint32_t Flags,
-                       InputFile *F, const WasmSignature *Signature = nullptr,
-                       const WasmGlobalType *GlobalType = nullptr);
+
+  Symbol *addUndefinedFunction(StringRef Name, uint32_t Flags, InputFile *File,
+                               const WasmSignature *Signature);
+  Symbol *addUndefinedData(StringRef Name, uint32_t Flags, InputFile *File);
+  Symbol *addUndefinedGlobal(StringRef Name, uint32_t Flags, InputFile *File,
+                             const WasmGlobalType *Type);
+
   void addLazy(ArchiveFile *F, const Archive::Symbol *Sym);
   bool addComdat(StringRef Name, ObjFile *);
 




More information about the llvm-commits mailing list