aboutsummaryrefslogtreecommitdiff
path: root/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
diff options
context:
space:
mode:
authorTim Neumann <timnn@google.com>2024-04-02 19:59:29 +0200
committerGitHub <noreply@github.com>2024-04-02 10:59:29 -0700
commitf792f14b01605453c7c0c17f3b4564335c0d9d14 (patch)
treeae3db8d7ca006a1432b16959ce8ab0e6db43db4c /llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
parent9c9f94063ce1bd819ba4482f537d1e580dd31eef (diff)
downloadllvm-f792f14b01605453c7c0c17f3b4564335c0d9d14.zip
llvm-f792f14b01605453c7c0c17f3b4564335c0d9d14.tar.gz
llvm-f792f14b01605453c7c0c17f3b4564335c0d9d14.tar.bz2
[WebAssembly] Allocate MCSymbolWasm data on MCContext (#85866)
Fixes #85578, a use-after-free caused by some `MCSymbolWasm` data being freed too early. Previously, `WebAssemblyAsmParser` owned the data that is moved to `MCContext` by this PR, which caused problems when handling module ASM, because the ASM parser was destroyed after parsing the module ASM, but the symbols persisted. The added test passes locally with an LLVM build with AddressSanitizer enabled. Implementation notes: * I've called the added method <code>allocate<b><i>Generic</i></b>String</code> and added the second paragraph of its documentation to maybe guide people a bit on when to use this method (based on my (limited) understanding of the `MCContext` class). We could also just call it `allocateString` and remove that second paragraph. * The added `createWasmSignature` method does not support taking the return and parameter types as arguments: Specifying them afterwards is barely any longer and prevents them from being accidentally specified in the wrong order. * This removes a _"TODO: Do the uniquing of Signatures here instead of ObjectFileWriter?"_ since the field it's attached to is also removed. Let me know if you think that TODO should be preserved somewhere.
Diffstat (limited to 'llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp')
-rw-r--r--llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp45
1 files changed, 14 insertions, 31 deletions
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 020c0d6..8e20631 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -196,10 +196,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
MCAsmParser &Parser;
MCAsmLexer &Lexer;
- // 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,
// and yet we have to be feeding data to the streamer in a very particular
@@ -287,16 +283,6 @@ public:
return Parser.Error(Loc.isValid() ? Loc : Lexer.getTok().getLoc(), Msg);
}
- void addSignature(std::unique_ptr<wasm::WasmSignature> &&Sig) {
- Signatures.push_back(std::move(Sig));
- }
-
- StringRef storeName(StringRef Name) {
- std::unique_ptr<std::string> N = std::make_unique<std::string>(Name);
- Names.push_back(std::move(N));
- return *Names.back();
- }
-
std::pair<StringRef, StringRef> nestingString(NestingType NT) {
switch (NT) {
case Function:
@@ -640,21 +626,20 @@ public:
// represent as a signature, such that we can re-build this signature,
// attach it to an anonymous symbol, which is what WasmObjectWriter
// expects to be able to recreate the actual unique-ified type indices.
+ auto &Ctx = getContext();
auto Loc = Parser.getTok();
- auto Signature = std::make_unique<wasm::WasmSignature>();
- if (parseSignature(Signature.get()))
+ auto Signature = Ctx.createWasmSignature();
+ if (parseSignature(Signature))
return true;
// Got signature as block type, don't need more
- TC.setLastSig(*Signature.get());
+ TC.setLastSig(*Signature);
if (ExpectBlockType)
- NestingStack.back().Sig = *Signature.get();
+ NestingStack.back().Sig = *Signature;
ExpectBlockType = false;
- auto &Ctx = getContext();
// The "true" here will cause this to be a nameless symbol.
MCSymbol *Sym = Ctx.createTempSymbol("typeindex", true);
auto *WasmSym = cast<MCSymbolWasm>(Sym);
- WasmSym->setSignature(Signature.get());
- addSignature(std::move(Signature));
+ WasmSym->setSignature(Signature);
WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
const MCExpr *Expr = MCSymbolRefExpr::create(
WasmSym, MCSymbolRefExpr::VK_WASM_TYPEINDEX, Ctx);
@@ -887,12 +872,11 @@ public:
CurrentState = FunctionStart;
LastFunctionLabel = WasmSym;
}
- auto Signature = std::make_unique<wasm::WasmSignature>();
- if (parseSignature(Signature.get()))
+ auto Signature = Ctx.createWasmSignature();
+ if (parseSignature(Signature))
return ParseStatus::Failure;
TC.funcDecl(*Signature);
- WasmSym->setSignature(Signature.get());
- addSignature(std::move(Signature));
+ WasmSym->setSignature(Signature);
WasmSym->setType(wasm::WASM_SYMBOL_TYPE_FUNCTION);
TOut.emitFunctionType(WasmSym);
// TODO: backend also calls TOut.emitIndIdx, but that is not implemented.
@@ -909,7 +893,7 @@ public:
if (ExportName.empty())
return ParseStatus::Failure;
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
- WasmSym->setExportName(storeName(ExportName));
+ WasmSym->setExportName(Ctx.allocateString(ExportName));
TOut.emitExportName(WasmSym, ExportName);
return expect(AsmToken::EndOfStatement, "EOL");
}
@@ -924,7 +908,7 @@ public:
if (ImportModule.empty())
return ParseStatus::Failure;
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
- WasmSym->setImportModule(storeName(ImportModule));
+ WasmSym->setImportModule(Ctx.allocateString(ImportModule));
TOut.emitImportModule(WasmSym, ImportModule);
return expect(AsmToken::EndOfStatement, "EOL");
}
@@ -939,7 +923,7 @@ public:
if (ImportName.empty())
return ParseStatus::Failure;
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
- WasmSym->setImportName(storeName(ImportName));
+ WasmSym->setImportName(Ctx.allocateString(ImportName));
TOut.emitImportName(WasmSym, ImportName);
return expect(AsmToken::EndOfStatement, "EOL");
}
@@ -949,11 +933,10 @@ public:
if (SymName.empty())
return ParseStatus::Failure;
auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
- auto Signature = std::make_unique<wasm::WasmSignature>();
+ auto Signature = Ctx.createWasmSignature();
if (parseRegTypeList(Signature->Params))
return ParseStatus::Failure;
- WasmSym->setSignature(Signature.get());
- addSignature(std::move(Signature));
+ WasmSym->setSignature(Signature);
WasmSym->setType(wasm::WASM_SYMBOL_TYPE_TAG);
TOut.emitTagType(WasmSym);
// TODO: backend also calls TOut.emitIndIdx, but that is not implemented.