Skip to content

Commit

Permalink
[WebAssembly] Separate addUndefined into addUndefined{Function,Data,G…
Browse files Browse the repository at this point in the history 
…lobal}.

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

llvm-svn: 326271
  • Loading branch information
@rui314
rui314 committed Feb 28, 2018
1 parent fa2fc90 commit e3498ec
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 128 deletions.
10 changes: 3 additions & 7 deletions lld/wasm/Driver.cpp
View file
Expand Up @@ -214,11 +214,6 @@ static StringRef getEntry(opt::InputArgList &Args, StringRef Default) {
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));
Expand Down Expand Up @@ -311,11 +306,12 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
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);
Expand Down
15 changes: 12 additions & 3 deletions lld/wasm/InputFiles.cpp
View file
Expand Up @@ -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,
Expand Down
236 changes: 125 additions & 111 deletions lld/wasm/SymbolTable.cpp
View file
Expand Up @@ -71,63 +71,56 @@ std::pair<Symbol *, bool> SymbolTable::insert(StringRef Name) {
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");

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);
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));
}
}

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) {
Expand Down Expand Up @@ -159,29 +152,12 @@ DefinedGlobal *SymbolTable::addSyntheticGlobal(StringRef Name, uint32_t Flags,
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;
}

Expand All @@ -192,93 +168,131 @@ static bool shouldReplace(const Symbol &Existing, InputFile *NewFile,
}

// 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");

if (auto *Lazy = dyn_cast<LazySymbol>(S)) {
DEBUG(dbgs() << "resolved by existing lazy\n");
Symbol *S;
bool WasInserted;
std::tie(S, WasInserted) = insert(Name);

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();

Expand All @@ -287,14 +301,14 @@ void SymbolTable::addLazy(ArchiveFile *F, const Archive::Symbol *Sym) {
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);
}
}

Expand Down

0 comments on commit e3498ec

Please sign in to comment.