Memory leak in Literal

[dcodeIO]

Pinning this here so we don't forget: An exnref Literal's std::unique_ptr<ExceptionPackage> exn (part of the union) is leaky in that it is not destroyed upon copy assignment (if I'm not mistaken).

Steps to reproduce:

• Compile with -fsanitize=address
• Run wasm-shell test/spec/exception-handling.wast

==7466==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 256 byte(s) in 4 object(s) allocated from:
    #0 0x7f8563cb4947 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10f947)
    #1 0x7f8562fa3291 in wasm::Literal::operator=(wasm::Literal const&) (/path/to/binaryen/lbuild/bin/../lib/libbinaryen.so+0x1382291)
    #2 0x56542f4eb7bd in wasm::SmallVector<wasm::Literal, 1ul>::SmallVector(wasm::SmallVector<wasm::Literal, 1ul> const&) (/path/to/binaryen/lbuild/bin/wasm-shell+0x3e7bd)
    #3 0x56542f545ad4 in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x98ad4)
    #4 0x56542f54a284 in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d284)
    #5 0x56542f5454be in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x984be)
    #6 0x56542f54a284 in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d284)
    #7 0x56542f5468a6 in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x998a6)
    #8 0x56542f58410f in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visitBlock(wasm::Block*) (/path/to/binaryen/lbuild/bin/wasm-shell+0xd710f)
    #9 0x56542f546828 in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x99828)
    #10 0x56542f58410f in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visitBlock(wasm::Block*) (/path/to/binaryen/lbuild/bin/wasm-shell+0xd710f)
    #11 0x56542f546828 in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x99828)
    #12 0x56542f54a284 in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d284)
    #13 0x56542f54a0cb in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d0cb)
    #14 0x56542f54a284 in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d284)
    #15 0x56542f54ac79 in wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::callFunctionInternal(wasm::Name, std::vector<wasm::Literal, std::allocator<wasm::Literal> > const&) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9dc79)
    #16 0x56542f4e3cc8 in run_asserts(wasm::Name, unsigned long*, bool*, wasm::Module*, wasm::Element*, wasm::SExpressionWasmBuilder*, wasm::Name) (/path/to/binaryen/lbuild/bin/wasm-shell+0x36cc8)
    #17 0x56542f4ce1b4 in main (/path/to/binaryen/lbuild/bin/wasm-shell+0x211b4)
    #18 0x7f856185a0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f8563cb4947 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10f947)
    #1 0x7f8562fa3434 in wasm::Literal::operator=(wasm::Literal const&) (/path/to/binaryen/lbuild/bin/../lib/libbinaryen.so+0x1382434)
    #2 0x56542f4eb7bd in wasm::SmallVector<wasm::Literal, 1ul>::SmallVector(wasm::SmallVector<wasm::Literal, 1ul> const&) (/path/to/binaryen/lbuild/bin/wasm-shell+0x3e7bd)
    #3 0x56542f545ad4 in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x98ad4)
    #4 0x56542f54a284 in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d284)
    #5 0x56542f5454be in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x984be)
    #6 0x56542f54a284 in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d284)
    #7 0x56542f5468a6 in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x998a6)
    #8 0x56542f58410f in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visitBlock(wasm::Block*) (/path/to/binaryen/lbuild/bin/wasm-shell+0xd710f)
    #9 0x56542f546828 in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x99828)
    #10 0x56542f58410f in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visitBlock(wasm::Block*) (/path/to/binaryen/lbuild/bin/wasm-shell+0xd710f)
    #11 0x56542f546828 in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x99828)
    #12 0x56542f54a284 in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d284)
    #13 0x56542f54a0cb in wasm::OverriddenVisitor<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner, wasm::Flow>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d0cb)
    #14 0x56542f54a284 in wasm::ExpressionRunner<wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::RuntimeExpressionRunner>::visit(wasm::Expression*) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9d284)
    #15 0x56542f54ac79 in wasm::ModuleInstanceBase<std::map<wasm::Name, wasm::Literals, std::less<wasm::Name>, std::allocator<std::pair<wasm::Name const, wasm::Literals> > >, wasm::ModuleInstance>::callFunctionInternal(wasm::Name, std::vector<wasm::Literal, std::allocator<wasm::Literal> > const&) (/path/to/binaryen/lbuild/bin/wasm-shell+0x9dc79)
    #16 0x56542f4e3cc8 in run_asserts(wasm::Name, unsigned long*, bool*, wasm::Module*, wasm::Element*, wasm::SExpressionWasmBuilder*, wasm::Name) (/path/to/binaryen/lbuild/bin/wasm-shell+0x36cc8)
    #17 0x56542f4ce1b4 in main (/path/to/binaryen/lbuild/bin/wasm-shell+0x211b4)
    #18 0x7f856185a0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: 280 byte(s) leaked in 5 allocation(s).

Unfortunately, my attempts to fix this have failed so far, leading to even more obscure use-after-free kind of errors. Requesting expert support :)

[tlively]

Taking a look now :)

[dcodeIO]

Things I tried fwiw:

• Implement copy assignment as if (&other != this) { this->~Literal(); new (this) auto(other); } return *this moving initialization into the ctor, lead to use-after-free
• Make Literal::type immutable to ensure that the payload doesn't become interpreted as something else, wasn't it
• Use an ExceptionPackage* exn instead with new and delete, same problem
• Try std::shared_ptr, but that increases the size of literals, so I scrapped that
• Make all methods that take std::unique_ptr<ExceptionPackage> take std::unique_ptr<ExceptionPackage>&& to guarantee that it's moved, didn't help
• Implement a move constructor, didn't help (and probably wasn't quite correct)
• Thought about making all uses of Literal const Literal, gave up
• Implement a non-const copy constructor using const_cast, didn't help
• Look up copy-and-swap idiom, didn't understand
• Make Literals extend std::vector instead of SmallVector, same problem

I'm super curious what's the cause btw, that'll be enlightening. Like when you forgot a name of someone or something, and then suddenly.