[llvm] 5be42f3 - [WebAssembly][MC] Fix leak of std::string members in MCSymbolWasm
Sam Clegg via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 7 10:39:14 PDT 2020
Author: Sam Clegg
Date: 2020-04-07T10:38:43-07:00
New Revision: 5be42f36f5685f76d705c2faa1b1368c48aa2316
URL: https://github.com/llvm/llvm-project/commit/5be42f36f5685f76d705c2faa1b1368c48aa2316
DIFF: https://github.com/llvm/llvm-project/commit/5be42f36f5685f76d705c2faa1b1368c48aa2316.diff
LOG: [WebAssembly][MC] Fix leak of std::string members in MCSymbolWasm
Summary: Fixes: https://bugs.llvm.org/show_bug.cgi?id=45452
Subscribers: dschuff, jgravelle-google, hiraditya, aheejin, sunfish, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D77627
Added:
Modified:
llvm/include/llvm/MC/MCSymbolWasm.h
llvm/lib/MC/MCContext.cpp
llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCSymbolWasm.h b/llvm/include/llvm/MC/MCSymbolWasm.h
index 7f83e93a1d27..e365e079cd8f 100644
--- a/llvm/include/llvm/MC/MCSymbolWasm.h
+++ b/llvm/include/llvm/MC/MCSymbolWasm.h
@@ -19,13 +19,15 @@ class MCSymbolWasm : public MCSymbol {
bool IsHidden = false;
bool IsComdat = false;
mutable bool IsUsedInGOT = false;
- Optional<std::string> ImportModule;
- Optional<std::string> ImportName;
- Optional<std::string> ExportName;
- wasm::WasmSignature *Signature = nullptr;
Optional<wasm::WasmGlobalType> GlobalType;
Optional<wasm::WasmEventType> EventType;
+ // Non-owning pointers since MCSymbol must be trivially destructible.
+ std::string *ImportModule = nullptr;
+ std::string *ImportName = nullptr;
+ std::string *ExportName = nullptr;
+ wasm::WasmSignature *Signature = nullptr;
+
/// An expression describing how to calculate the size of a symbol. If a
/// symbol has no size this field will be NULL.
const MCExpr *SymbolSize = nullptr;
@@ -69,37 +71,32 @@ class MCSymbolWasm : public MCSymbol {
bool isComdat() const { return IsComdat; }
void setComdat(bool isComdat) { IsComdat = isComdat; }
- bool hasImportModule() const { return ImportModule.hasValue(); }
+ bool hasImportModule() const { return ImportModule != nullptr; }
const StringRef getImportModule() const {
- if (ImportModule.hasValue()) {
- return ImportModule.getValue();
- }
- // Use a default module name of "env" for now, for compatibility with
- // existing tools.
- // TODO(sbc): Find a way to specify a default value in the object format
- // without picking a hardcoded value like this.
- return "env";
- }
- void setImportModule(StringRef Name) {
- ImportModule = std::string(std::string(Name));
+ if (ImportModule)
+ return StringRef(*ImportModule);
+ // Use a default module name of "env" for now, for compatibility with
+ // existing tools.
+ // TODO(sbc): Find a way to specify a default value in the object format
+ // without picking a hardcoded value like this.
+ return "env";
}
+ void setImportModule(std::string *Name) { ImportModule = Name; }
- bool hasImportName() const { return ImportName.hasValue(); }
+ bool hasImportName() const { return ImportName != nullptr; }
const StringRef getImportName() const {
- if (ImportName.hasValue()) {
- return ImportName.getValue();
- }
- return getName();
- }
- void setImportName(StringRef Name) {
- ImportName = std::string(std::string(Name));
+ if (ImportName)
+ return StringRef(*ImportName);
+ return getName();
}
+ void setImportName(std::string *Name) { ImportName = Name; }
- bool hasExportName() const { return ExportName.hasValue(); }
- const StringRef getExportName() const { return ExportName.getValue(); }
- void setExportName(StringRef Name) {
- ExportName = std::string(std::string(Name));
+ bool hasExportName() const { return ExportName != nullptr; }
+ const StringRef getExportName() const {
+ assert(ExportName);
+ return StringRef(*ExportName);
}
+ void setExportName(std::string *Name) { ExportName = Name; }
void setUsedInGOT() const { IsUsedInGOT = true; }
bool isUsedInGOT() const { return IsUsedInGOT; }
diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index 524ef44d1863..41ff30aa45be 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -158,6 +158,16 @@ MCSymbol *MCContext::getOrCreateLSDASymbol(StringRef FuncName) {
MCSymbol *MCContext::createSymbolImpl(const StringMapEntry<bool> *Name,
bool IsTemporary) {
+ static_assert(std::is_trivially_destructible<MCSymbolCOFF>(),
+ "MCSymbol classes must be trivially destructible");
+ static_assert(std::is_trivially_destructible<MCSymbolELF>(),
+ "MCSymbol classes must be trivially destructible");
+ static_assert(std::is_trivially_destructible<MCSymbolMachO>(),
+ "MCSymbol classes must be trivially destructible");
+ static_assert(std::is_trivially_destructible<MCSymbolWasm>(),
+ "MCSymbol classes must be trivially destructible");
+ static_assert(std::is_trivially_destructible<MCSymbolXCOFF>(),
+ "MCSymbol classes must be trivially destructible");
if (MOFI) {
switch (MOFI->getObjectFileType()) {
case MCObjectFileInfo::IsCOFF:
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index d0e9eb1fc9bc..68f0cbde8224 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -164,6 +164,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
// Much like WebAssemblyAsmPrinter in the backend, we have to own these.
std::vector<std::unique_ptr<wasm::WasmSignature>> Signatures;
+ std::vector<std::unique_ptr<std::string>> Names;
// Order of labels, directives and instructions in a .s file have no
// syntactical enforcement. This class is a callback from the actual parser,
@@ -232,6 +233,12 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
Signatures.push_back(std::move(Sig));
}
+ std::string *storeName(StringRef Name) {
+ std::unique_ptr<std::string> N = std::make_unique<std::string>(Name);
+ Names.push_back(std::move(N));
+ return Names.back().get();
+ }
+
std::pair<StringRef, StringRef> nestingString(NestingType NT) {
switch (NT) {
case Function:
@@ -725,7 +732,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
return true;
auto ExportName = expectIdent();
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
- WasmSym->setExportName(ExportName);
+ WasmSym->setExportName(storeName(ExportName));
TOut.emitExportName(WasmSym, ExportName);
}
@@ -737,7 +744,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
return true;
auto ImportModule = expectIdent();
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
- WasmSym->setImportModule(ImportModule);
+ WasmSym->setImportModule(storeName(ImportModule));
TOut.emitImportModule(WasmSym, ImportModule);
}
@@ -749,7 +756,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
return true;
auto ImportName = expectIdent();
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
- WasmSym->setImportName(ImportName);
+ WasmSym->setImportName(storeName(ImportName));
TOut.emitImportName(WasmSym, ImportName);
}
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
index a9842a1f72bd..c701e715152a 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
@@ -122,14 +122,14 @@ void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
F.hasFnAttribute("wasm-import-module")) {
StringRef Name =
F.getFnAttribute("wasm-import-module").getValueAsString();
- Sym->setImportModule(Name);
+ Sym->setImportModule(storeName(Name));
getTargetStreamer()->emitImportModule(Sym, Name);
}
if (TM.getTargetTriple().isOSBinFormatWasm() &&
F.hasFnAttribute("wasm-import-name")) {
StringRef Name =
F.getFnAttribute("wasm-import-name").getValueAsString();
- Sym->setImportName(Name);
+ Sym->setImportName(storeName(Name));
getTargetStreamer()->emitImportName(Sym, Name);
}
}
@@ -137,7 +137,7 @@ void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
if (F.hasFnAttribute("wasm-export-name")) {
auto *Sym = cast<MCSymbolWasm>(getSymbol(&F));
StringRef Name = F.getFnAttribute("wasm-export-name").getValueAsString();
- Sym->setExportName(Name);
+ Sym->setExportName(storeName(Name));
getTargetStreamer()->emitExportName(Sym, Name);
}
}
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
index 883320806d3e..5863f556ca33 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h
@@ -26,6 +26,13 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyAsmPrinter final : public AsmPrinter {
WebAssemblyFunctionInfo *MFI;
// TODO: Do the uniquing of Signatures here instead of ObjectFileWriter?
std::vector<std::unique_ptr<wasm::WasmSignature>> Signatures;
+ std::vector<std::unique_ptr<std::string>> Names;
+
+ std::string *storeName(StringRef Name) {
+ std::unique_ptr<std::string> N = std::make_unique<std::string>(Name);
+ Names.push_back(std::move(N));
+ return Names.back().get();
+ }
public:
explicit WebAssemblyAsmPrinter(TargetMachine &TM,
More information about the llvm-commits
mailing list