Skip to content

Commit 5be42f3

Browse files
sbc100sbc100
committed
[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
1 parent 12a3243 commit 5be42f3

File tree

5 files changed

+55
-34
lines changed

5 files changed

+55
-34
lines changed

‎llvm/include/llvm/MC/MCSymbolWasm.h

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@ class MCSymbolWasm : public MCSymbol {
1919
bool IsHidden = false;
2020
bool IsComdat = false;
2121
mutable bool IsUsedInGOT = false;
22-
Optional<std::string> ImportModule;
23-
Optional<std::string> ImportName;
24-
Optional<std::string> ExportName;
25-
wasm::WasmSignature *Signature = nullptr;
2622
Optional<wasm::WasmGlobalType> GlobalType;
2723
Optional<wasm::WasmEventType> EventType;
2824

25+
// Non-owning pointers since MCSymbol must be trivially destructible.
26+
std::string *ImportModule = nullptr;
27+
std::string *ImportName = nullptr;
28+
std::string *ExportName = nullptr;
29+
wasm::WasmSignature *Signature = nullptr;
30+
2931
/// An expression describing how to calculate the size of a symbol. If a
3032
/// symbol has no size this field will be NULL.
3133
const MCExpr *SymbolSize = nullptr;
@@ -69,37 +71,32 @@ class MCSymbolWasm : public MCSymbol {
6971
bool isComdat() const { return IsComdat; }
7072
void setComdat(bool isComdat) { IsComdat = isComdat; }
7173

72-
bool hasImportModule() const { return ImportModule.hasValue(); }
74+
bool hasImportModule() const { return ImportModule != nullptr; }
7375
const StringRef getImportModule() const {
74-
if (ImportModule.hasValue()) {
75-
return ImportModule.getValue();
76-
}
77-
// Use a default module name of "env" for now, for compatibility with
78-
// existing tools.
79-
// TODO(sbc): Find a way to specify a default value in the object format
80-
// without picking a hardcoded value like this.
81-
return "env";
82-
}
83-
void setImportModule(StringRef Name) {
84-
ImportModule = std::string(std::string(Name));
76+
if (ImportModule)
77+
return StringRef(*ImportModule);
78+
// Use a default module name of "env" for now, for compatibility with
79+
// existing tools.
80+
// TODO(sbc): Find a way to specify a default value in the object format
81+
// without picking a hardcoded value like this.
82+
return "env";
8583
}
84+
void setImportModule(std::string *Name) { ImportModule = Name; }
8685

87-
bool hasImportName() const { return ImportName.hasValue(); }
86+
bool hasImportName() const { return ImportName != nullptr; }
8887
const StringRef getImportName() const {
89-
if (ImportName.hasValue()) {
90-
return ImportName.getValue();
91-
}
92-
return getName();
93-
}
94-
void setImportName(StringRef Name) {
95-
ImportName = std::string(std::string(Name));
88+
if (ImportName)
89+
return StringRef(*ImportName);
90+
return getName();
9691
}
92+
void setImportName(std::string *Name) { ImportName = Name; }
9793

98-
bool hasExportName() const { return ExportName.hasValue(); }
99-
const StringRef getExportName() const { return ExportName.getValue(); }
100-
void setExportName(StringRef Name) {
101-
ExportName = std::string(std::string(Name));
94+
bool hasExportName() const { return ExportName != nullptr; }
95+
const StringRef getExportName() const {
96+
assert(ExportName);
97+
return StringRef(*ExportName);
10298
}
99+
void setExportName(std::string *Name) { ExportName = Name; }
103100

104101
void setUsedInGOT() const { IsUsedInGOT = true; }
105102
bool isUsedInGOT() const { return IsUsedInGOT; }

‎llvm/lib/MC/MCContext.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,16 @@ MCSymbol *MCContext::getOrCreateLSDASymbol(StringRef FuncName) {
158158

159159
MCSymbol *MCContext::createSymbolImpl(const StringMapEntry<bool> *Name,
160160
bool IsTemporary) {
161+
static_assert(std::is_trivially_destructible<MCSymbolCOFF>(),
162+
"MCSymbol classes must be trivially destructible");
163+
static_assert(std::is_trivially_destructible<MCSymbolELF>(),
164+
"MCSymbol classes must be trivially destructible");
165+
static_assert(std::is_trivially_destructible<MCSymbolMachO>(),
166+
"MCSymbol classes must be trivially destructible");
167+
static_assert(std::is_trivially_destructible<MCSymbolWasm>(),
168+
"MCSymbol classes must be trivially destructible");
169+
static_assert(std::is_trivially_destructible<MCSymbolXCOFF>(),
170+
"MCSymbol classes must be trivially destructible");
161171
if (MOFI) {
162172
switch (MOFI->getObjectFileType()) {
163173
case MCObjectFileInfo::IsCOFF:

‎llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
164164

165165
// Much like WebAssemblyAsmPrinter in the backend, we have to own these.
166166
std::vector<std::unique_ptr<wasm::WasmSignature>> Signatures;
167+
std::vector<std::unique_ptr<std::string>> Names;
167168

168169
// Order of labels, directives and instructions in a .s file have no
169170
// syntactical enforcement. This class is a callback from the actual parser,
@@ -232,6 +233,12 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
232233
Signatures.push_back(std::move(Sig));
233234
}
234235

236+
std::string *storeName(StringRef Name) {
237+
std::unique_ptr<std::string> N = std::make_unique<std::string>(Name);
238+
Names.push_back(std::move(N));
239+
return Names.back().get();
240+
}
241+
235242
std::pair<StringRef, StringRef> nestingString(NestingType NT) {
236243
switch (NT) {
237244
case Function:
@@ -725,7 +732,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
725732
return true;
726733
auto ExportName = expectIdent();
727734
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
728-
WasmSym->setExportName(ExportName);
735+
WasmSym->setExportName(storeName(ExportName));
729736
TOut.emitExportName(WasmSym, ExportName);
730737
}
731738

@@ -737,7 +744,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
737744
return true;
738745
auto ImportModule = expectIdent();
739746
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
740-
WasmSym->setImportModule(ImportModule);
747+
WasmSym->setImportModule(storeName(ImportModule));
741748
TOut.emitImportModule(WasmSym, ImportModule);
742749
}
743750

@@ -749,7 +756,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
749756
return true;
750757
auto ImportName = expectIdent();
751758
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
752-
WasmSym->setImportName(ImportName);
759+
WasmSym->setImportName(storeName(ImportName));
753760
TOut.emitImportName(WasmSym, ImportName);
754761
}
755762

‎llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,22 +122,22 @@ void WebAssemblyAsmPrinter::emitEndOfAsmFile(Module &M) {
122122
F.hasFnAttribute("wasm-import-module")) {
123123
StringRef Name =
124124
F.getFnAttribute("wasm-import-module").getValueAsString();
125-
Sym->setImportModule(Name);
125+
Sym->setImportModule(storeName(Name));
126126
getTargetStreamer()->emitImportModule(Sym, Name);
127127
}
128128
if (TM.getTargetTriple().isOSBinFormatWasm() &&
129129
F.hasFnAttribute("wasm-import-name")) {
130130
StringRef Name =
131131
F.getFnAttribute("wasm-import-name").getValueAsString();
132-
Sym->setImportName(Name);
132+
Sym->setImportName(storeName(Name));
133133
getTargetStreamer()->emitImportName(Sym, Name);
134134
}
135135
}
136136

137137
if (F.hasFnAttribute("wasm-export-name")) {
138138
auto *Sym = cast<MCSymbolWasm>(getSymbol(&F));
139139
StringRef Name = F.getFnAttribute("wasm-export-name").getValueAsString();
140-
Sym->setExportName(Name);
140+
Sym->setExportName(storeName(Name));
141141
getTargetStreamer()->emitExportName(Sym, Name);
142142
}
143143
}

‎llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,13 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyAsmPrinter final : public AsmPrinter {
2626
WebAssemblyFunctionInfo *MFI;
2727
// TODO: Do the uniquing of Signatures here instead of ObjectFileWriter?
2828
std::vector<std::unique_ptr<wasm::WasmSignature>> Signatures;
29+
std::vector<std::unique_ptr<std::string>> Names;
30+
31+
std::string *storeName(StringRef Name) {
32+
std::unique_ptr<std::string> N = std::make_unique<std::string>(Name);
33+
Names.push_back(std::move(N));
34+
return Names.back().get();
35+
}
2936

3037
public:
3138
explicit WebAssemblyAsmPrinter(TargetMachine &TM,

0 commit comments

Comments
 (0)