maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Revert D105519 "[WebAssembly] Deduplicate imports of the same module name, field name, and type" and its followup 

This reverts commit 4ae575b999 and 9b965b37c7.

There is an use-of-uninitialized-value bug in the `else` branch in ImportSection::addImport.
This commit is contained in:
Fangrui Song 2021-07-19 17:09:01 -07:00
parent 762badb0ab
commit 16aac493e5
9 changed files with 41 additions and 370 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
lld/test/wasm/duplicate-function-imports.s
View File

@ -1,57 +0,0 @@
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t.o
# RUN: wasm-ld -o %t1.wasm %t.o
# RUN: obj2yaml %t1.wasm | FileCheck %s
.globl f1
.import_module f1, env
.import_name f1, f
.functype f1 () -> (i32)
# Same import module/name/type as `f1`, should be de-duped.
.globl f2
.import_module f2, env
.import_name f2, f
.functype f2 () -> (i32)
# Same import module/name, but different type. Should not be de-duped.
.globl f3
.import_module f3, env
.import_name f3, f
.functype f3 () -> (f32)
.globl _start
_start:
.functype _start () -> ()
call f1
drop
call f2
drop
call f3
drop
end_function
# CHECK: - Type: TYPE
# CHECK-NEXT: Signatures:
# CHECK-NEXT: - Index: 0
# CHECK-NEXT: ParamTypes: []
# CHECK-NEXT: ReturnTypes:
# CHECK-NEXT: - I32
# CHECK-NEXT: - Index: 1
# CHECK-NEXT: ParamTypes: []
# CHECK-NEXT: ReturnTypes:
# CHECK-NEXT: - F32
# CHECK-NEXT: - Index: 2
# CHECK-NEXT: ParamTypes: []
# CHECK-NEXT: ReturnTypes: []
# CHECK-NEXT: - Type: IMPORT
# CHECK-NEXT: Imports:
# CHECK-NEXT: - Module: env
# CHECK-NEXT: Field: f
# CHECK-NEXT: Kind: FUNCTION
# CHECK-NEXT: SigIndex: 0
# CHECK-NEXT: - Module: env
# CHECK-NEXT: Field: f
# CHECK-NEXT: Kind: FUNCTION
# CHECK-NEXT: SigIndex: 1
# CHECK-NEXT: - Type:

69
lld/test/wasm/duplicate-global-imports.s
View File

@ -1,69 +0,0 @@
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t.o
# RUN: wasm-ld --no-check-features -o %t1.wasm %t.o
# RUN: obj2yaml %t1.wasm | FileCheck %s
.global g1
.import_module g1, env
.import_name g1, g
.globaltype g1, i64, immutable
# Same import module/name/type as `g1`, should be de-duped.
.global g2
.import_module g2, env
.import_name g2, g
.globaltype g2, i64, immutable
# Imported as an i32 instead of i64, so should not be de-duped.
.global g3
.import_module g3, env
.import_name g3, g
.globaltype g3, i32, immutable
# Imported as mutable instead of immutable, so should not be de-duped.
.global g4
.import_module g4, env
.import_name g4, g
.globaltype g4, i64
.globl _start
_start:
.functype _start () -> ()
global.get g1
drop
global.get g2
drop
global.get g3
drop
global.get g4
drop
end_function
# CHECK: - Type: IMPORT
# CHECK-NEXT: Imports:
# CHECK-NEXT: - Module: env
# CHECK-NEXT: Field: g
# CHECK-NEXT: Kind: GLOBAL
# CHECK-NEXT: GlobalType: I64
# CHECK-NEXT: GlobalMutable: false
# CHECK-NEXT: - Module: env
# CHECK-NEXT: Field: g
# CHECK-NEXT: Kind: GLOBAL
# CHECK-NEXT: GlobalType: I32
# CHECK-NEXT: GlobalMutable: false
# CHECK-NEXT: - Module: env
# CHECK-NEXT: Field: g
# CHECK-NEXT: Kind: GLOBAL
# CHECK-NEXT: GlobalType: I64
# CHECK-NEXT: GlobalMutable: true
# CHECK-NEXT: - Type:
# CHECK: GlobalNames:
# CHECK-NEXT: - Index: 0
# CHECK-NEXT: Name: g1
# CHECK-NEXT: - Index: 1
# CHECK-NEXT: Name: g3
# CHECK-NEXT: - Index: 2
# CHECK-NEXT: Name: g4
# CHECK-NEXT: - Index: 3
# CHECK-NEXT: Name: __stack_pointer

75
lld/test/wasm/duplicate-table-imports.s
View File

@ -1,75 +0,0 @@
# RUN: llvm-mc -mattr=+reference-types -triple=wasm32-unknown-unknown -filetype=obj -o %t.o %s
# RUN: wasm-ld --allow-undefined -o %t1.wasm %t.o
# RUN: obj2yaml %t1.wasm | FileCheck %s
.tabletype t1, funcref
.import_module t1, env
.import_name t1, t
.globl t1
# Same import module/name/type as `t1`, should be de-duped.
.tabletype t2, funcref
.import_module t2, env
.import_name t2, t
.globl t2
# Imported as an externref instead of funcref, so should not be de-duped.
.tabletype t3, externref
.import_module t3, env
.import_name t3, t
.globl t3
.globl _start
_start:
.functype _start () -> ()
# Read from `t1`
i32.const 0
table.get t1
drop
# Read from `t2`
i32.const 0
table.get t2
drop
# Read from `t3`
i32.const 0
table.get t3
drop
end_function
# XXX: the second imported table has index 1, not 0. I've verified by hand (with
# `wasm2wat`) that the resulting Wasm file is correct: `t3` does end up at index
# 1 and our `table.get` instructions are using the proper table index
# immediates. This is also asserted (less legibly) in the hexdump of the code
# body below. It looks like there's a bug in how `obj2yaml` disassembles
# multiple table imports.
# CHECK: - Type: IMPORT
# CHECK-NEXT: Imports:
# CHECK-NEXT: - Module: env
# CHECK-NEXT: Field: t
# CHECK-NEXT: Kind: TABLE
# CHECK-NEXT: Table:
# CHECK-NEXT: Index: 0
# CHECK-NEXT: ElemType: FUNCREF
# CHECK-NEXT: Limits:
# CHECK-NEXT: Minimum: 0x0
# CHECK-NEXT: - Module: env
# CHECK-NEXT: Field: t
# CHECK-NEXT: Kind: TABLE
# CHECK-NEXT: Table:
# CHECK-NEXT: Index: 0
# CHECK-NEXT: ElemType: EXTERNREF
# CHECK-NEXT: Limits:
# CHECK-NEXT: Minimum: 0x0
# CHECK-NEXT: - Type:
# CHECK: - Type: CODE
# CHECK-NEXT: Functions:
# CHECK-NEXT: - Index: 0
# CHECK-NEXT: Locals: []
# CHECK-NEXT: Body: 41002580808080001A41002580808080001A41002581808080001A0B
# CHECK-NEXT: - Type:

29
lld/wasm/Symbols.h
View File

@ -172,9 +172,6 @@ public:
bool isStub : 1;
uint32_t flags;
llvm::Optional<StringRef> importName;
llvm::Optional<StringRef> importModule;
};
class FunctionSymbol : public Symbol {
@ -225,15 +222,15 @@ public:
const WasmSignature *type = nullptr,
bool isCalledDirectly = true)
: FunctionSymbol(name, UndefinedFunctionKind, flags, file, type),
isCalledDirectly(isCalledDirectly) {
this->importName = importName;
this->importModule = importModule;
}
importName(importName), importModule(importModule),
isCalledDirectly(isCalledDirectly) {}
static bool classof(const Symbol *s) {
return s->kind() == UndefinedFunctionKind;
}
llvm::Optional<StringRef> importName;
llvm::Optional<StringRef> importModule;
DefinedFunction *stubFunction = nullptr;
bool isCalledDirectly;
};
@ -357,14 +354,15 @@ public:
llvm::Optional<StringRef> importModule, uint32_t flags,
InputFile *file = nullptr,
const WasmGlobalType *type = nullptr)
: GlobalSymbol(name, UndefinedGlobalKind, flags, file, type) {
this->importName = importName;
this->importModule = importModule;
}
: GlobalSymbol(name, UndefinedGlobalKind, flags, file, type),
importName(importName), importModule(importModule) {}
static bool classof(const Symbol *s) {
return s->kind() == UndefinedGlobalKind;
}
llvm::Optional<StringRef> importName;
llvm::Optional<StringRef> importModule;
};
class TableSymbol : public Symbol {
@ -405,14 +403,15 @@ public:
UndefinedTable(StringRef name, llvm::Optional<StringRef> importName,
llvm::Optional<StringRef> importModule, uint32_t flags,
InputFile *file, const WasmTableType *type)
: TableSymbol(name, UndefinedTableKind, flags, file, type) {
this->importName = importName;
this->importModule = importModule;
}
: TableSymbol(name, UndefinedTableKind, flags, file, type),
importName(importName), importModule(importModule) {}
static bool classof(const Symbol *s) {
return s->kind() == UndefinedTableKind;
}
llvm::Optional<StringRef> importName;
llvm::Optional<StringRef> importModule;
};
// A tag is a general format to distinguish typed entities. Each tag has an

58
lld/wasm/SyntheticSections.cpp
View File

@ -109,42 +109,15 @@ void ImportSection::addGOTEntry(Symbol *sym) {
void ImportSection::addImport(Symbol *sym) {
assert(!isSealed);
StringRef module = sym->importModule.getValueOr(defaultModule);
StringRef name = sym->importName.getValueOr(sym->getName());
if (auto *f = dyn_cast<FunctionSymbol>(sym)) {
ImportKey<WasmSignature> key(*(f->getSignature()), module, name);
auto entry = importedFunctions.try_emplace(key, numImportedFunctions);
if (entry.second) {
importedSymbols.emplace_back(sym);
f->setFunctionIndex(numImportedFunctions++);
} else {
f->setFunctionIndex(entry.first->second);
}
} else if (auto *g = dyn_cast<GlobalSymbol>(sym)) {
ImportKey<WasmGlobalType> key(*(g->getGlobalType()), module, name);
auto entry = importedGlobals.try_emplace(key, numImportedGlobals);
if (entry.second) {
importedSymbols.emplace_back(sym);
g->setGlobalIndex(numImportedGlobals++);
} else {
g->setGlobalIndex(entry.first->second);
}
} else if (auto *t = dyn_cast<TagSymbol>(sym)) {
// NB: There's currently only one possible kind of tag, and no
// `UndefinedTag`, so we don't bother de-duplicating tag imports.
importedSymbols.emplace_back(sym);
importedSymbols.emplace_back(sym);
if (auto *f = dyn_cast<FunctionSymbol>(sym))
f->setFunctionIndex(numImportedFunctions++);
else if (auto *g = dyn_cast<GlobalSymbol>(sym))
g->setGlobalIndex(numImportedGlobals++);
else if (auto *t = dyn_cast<TagSymbol>(sym))
t->setTagIndex(numImportedTags++);
} else {
auto *table = cast<TableSymbol>(sym);
ImportKey<WasmTableType> key(*(table->getTableType()), module, name);
auto entry = importedTables.try_emplace(key, numImportedTables);
if (entry.second) {
importedSymbols.emplace_back(sym);
table->setTableNumber(numImportedTables++);
} else {
table->setTableNumber(entry.first->second);
}
}
else
cast<TableSymbol>(sym)->setTableNumber(numImportedTables++);
}
void ImportSection::writeBody() {
@ -174,8 +147,19 @@ void ImportSection::writeBody() {
for (const Symbol *sym : importedSymbols) {
WasmImport import;
import.Field = sym->importName.getValueOr(sym->getName());
import.Module = sym->importModule.getValueOr(defaultModule);
if (auto *f = dyn_cast<UndefinedFunction>(sym)) {
import.Field = f->importName ? *f->importName : sym->getName();
import.Module = f->importModule ? *f->importModule : defaultModule;
} else if (auto *g = dyn_cast<UndefinedGlobal>(sym)) {
import.Field = g->importName ? *g->importName : sym->getName();
import.Module = g->importModule ? *g->importModule : defaultModule;
} else if (auto *t = dyn_cast<UndefinedTable>(sym)) {
import.Field = t->importName ? *t->importName : sym->getName();
import.Module = t->importModule ? *t->importModule : defaultModule;
} else {
import.Field = sym->getName();
import.Module = defaultModule;
}
if (auto *functionSym = dyn_cast<FunctionSymbol>(sym)) {
import.Kind = WASM_EXTERNAL_FUNCTION;

65
lld/wasm/SyntheticSections.h
View File

@ -96,68 +96,6 @@ protected:
llvm::DenseMap<WasmSignature, int32_t> typeIndices;
};
/**
* A key for some kind of imported entity of type `T`.
*
* Used when de-duplicating imports.
*/
template <typename T> struct ImportKey {
public:
enum class State { Plain, Empty, Tombstone };
public:
T type;
llvm::Optional<StringRef> importModule;
llvm::Optional<StringRef> importName;
State state;
public:
ImportKey(T type) : type(type), state(State::Plain) {}
ImportKey(T type, State state) : type(type), state(state) {}
ImportKey(T type, llvm::Optional<StringRef> importModule,
llvm::Optional<StringRef> importName)
: type(type), importModule(importModule), importName(importName),
state(State::Plain) {}
};
template <typename T>
inline bool operator==(const ImportKey<T> &lhs, const ImportKey<T> &rhs) {
return lhs.state == rhs.state && lhs.importModule == rhs.importModule &&
lhs.importName == rhs.importName && lhs.type == rhs.type;
}
} // namespace wasm
} // namespace lld
// `ImportKey<T>` can be used as a key in a `DenseMap` if `T` can be used as a
// key in a `DenseMap`.
template <typename T> struct llvm::DenseMapInfo<lld::wasm::ImportKey<T>> {
static lld::wasm::ImportKey<T> getEmptyKey() {
typename lld::wasm::ImportKey<T> key(llvm::DenseMapInfo<T>::getEmptyKey());
key.state = lld::wasm::ImportKey<T>::State::Empty;
return key;
}
static lld::wasm::ImportKey<T> getTombstoneKey() {
typename lld::wasm::ImportKey<T> key(llvm::DenseMapInfo<T>::getEmptyKey());
key.state = lld::wasm::ImportKey<T>::State::Tombstone;
return key;
}
static unsigned getHashValue(const lld::wasm::ImportKey<T> &key) {
uintptr_t hash = hash_value(key.importModule);
hash = hash_combine(hash, key.importName);
hash = hash_combine(hash, llvm::DenseMapInfo<T>::getHashValue(key.type));
hash = hash_combine(hash, key.state);
return hash;
}
static bool isEqual(const lld::wasm::ImportKey<T> &lhs,
const lld::wasm::ImportKey<T> &rhs) {
return lhs == rhs;
}
};
namespace lld {
namespace wasm {
class ImportSection : public SyntheticSection {
public:
ImportSection() : SyntheticSection(llvm::wasm::WASM_SEC_IMPORT) {}
@ -193,9 +131,6 @@ protected:
unsigned numImportedFunctions = 0;
unsigned numImportedTags = 0;
unsigned numImportedTables = 0;
llvm::DenseMap<ImportKey<WasmGlobalType>, uint32_t> importedGlobals;
llvm::DenseMap<ImportKey<WasmSignature>, uint32_t> importedFunctions;
llvm::DenseMap<ImportKey<WasmTableType>, uint32_t> importedTables;
};
class FunctionSection : public SyntheticSection {

8
lld/wasm/Writer.cpp
View File

@ -562,8 +562,12 @@ static bool shouldImport(Symbol *sym) {
return true;
if (config->allowUndefinedSymbols.count(sym->getName()) != 0)
return true;
if (sym->isUndefined())
return sym->importName.hasValue();
if (auto *g = dyn_cast<UndefinedGlobal>(sym))
return g->importName.hasValue();
if (auto *f = dyn_cast<UndefinedFunction>(sym))
return f->importName.hasValue();
if (auto *t = dyn_cast<UndefinedTable>(sym))
return t->importName.hasValue();
return false;
}

9
llvm/include/llvm/BinaryFormat/Wasm.h
View File

@ -67,20 +67,11 @@ struct WasmLimits {
uint64_t Maximum;
};
inline bool operator==(const WasmLimits &LHS, const WasmLimits &RHS) {
return LHS.Flags == RHS.Flags && LHS.Minimum == RHS.Minimum &&
LHS.Maximum == RHS.Maximum;
}
struct WasmTableType {
uint8_t ElemType;
WasmLimits Limits;
};
inline bool operator==(const WasmTableType &LHS, const WasmTableType &RHS) {
return LHS.ElemType == RHS.ElemType && LHS.Limits == RHS.Limits;
}
struct WasmTable {
uint32_t Index;
WasmTableType Type;

41
llvm/include/llvm/BinaryFormat/WasmTraits.h
View File

@ -63,47 +63,6 @@ template <> struct DenseMapInfo<wasm::WasmGlobalType> {
}
};
// Traits for using WasmLimits in a DenseMap
template <> struct DenseMapInfo<wasm::WasmLimits> {
static wasm::WasmLimits getEmptyKey() {
return wasm::WasmLimits{0xff, 0xff, 0xff};
}
static wasm::WasmLimits getTombstoneKey() {
return wasm::WasmLimits{0xee, 0xee, 0xee};
}
static unsigned getHashValue(const wasm::WasmLimits &Limits) {
unsigned Hash = hash_value(Limits.Flags);
Hash = hash_combine(Hash, Limits.Minimum);
Hash = hash_combine(Hash, Limits.Maximum);
return Hash;
}
static bool isEqual(const wasm::WasmLimits &LHS,
const wasm::WasmLimits &RHS) {
return LHS == RHS;
}
};
// Traits for using WasmTableType in a DenseMap
template <> struct DenseMapInfo<wasm::WasmTableType> {
static wasm::WasmTableType getEmptyKey() {
return wasm::WasmTableType{0,
DenseMapInfo<wasm::WasmLimits>::getEmptyKey()};
}
static wasm::WasmTableType getTombstoneKey() {
return wasm::WasmTableType{
1, DenseMapInfo<wasm::WasmLimits>::getTombstoneKey()};
}
static unsigned getHashValue(const wasm::WasmTableType &TableType) {
return hash_combine(
TableType.ElemType,
DenseMapInfo<wasm::WasmLimits>::getHashValue(TableType.Limits));
}
static bool isEqual(const wasm::WasmTableType &LHS,
const wasm::WasmTableType &RHS) {
return LHS == RHS;
}
};
} // end namespace llvm
#endif // LLVM_BINARYFORMAT_WASMTRAITS_H