From 85234cb540c2b2863dfea6c20496dcb451df8889 Mon Sep 17 00:00:00 2001 From: Thomas Lively <7121787+tlively@users.noreply.github.com> Date: Tue, 1 Sep 2020 15:55:58 -0700 Subject: Fix ExceptionPackage memory errors (#3088) First, adds an explicit destructor call to fix a memory leak in `Literal::operator=` in which existing `ExceptionPackage`s would be silently dropped. Next, changes `Literal::getExceptionPackage` to return the `ExceptionPackage` by value to avoid a use-after-free bug in the interpreter that was surfaced by the new destructor call. A future improvement would be to switch to using `std::variant`. Fixes #3087. --- src/literal.h | 5 +---- src/wasm-interpreter.h | 2 +- src/wasm/literal.cpp | 18 ++++++++++++++---- 3 files changed, 16 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/literal.h b/src/literal.h index 5c72bf96c..cffa9ab7d 100644 --- a/src/literal.h +++ b/src/literal.h @@ -135,10 +135,7 @@ public: assert(type == Type::funcref); return func; } - const ExceptionPackage& getExceptionPackage() const { - assert(type == Type::exnref); - return *exn.get(); - } + ExceptionPackage getExceptionPackage() const; // careful! int32_t* geti32Ptr() { diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index baeda0612..e3674416a 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -1297,7 +1297,7 @@ public: if (flow.getType() == Type::nullref) { trap("br_on_exn: argument is null"); } - const ExceptionPackage& ex = flow.getSingleValue().getExceptionPackage(); + auto ex = flow.getSingleValue().getExceptionPackage(); if (curr->event != ex.event) { // Not taken return flow; } diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index ffb007cf3..ce1c73d56 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -33,10 +33,7 @@ Literal::Literal(const uint8_t init[16]) : type(Type::v128) { memcpy(&v128, init, 16); } -Literal::Literal(const Literal& other) { *this = other; } - -Literal& Literal::operator=(const Literal& other) { - type = other.type; +Literal::Literal(const Literal& other) : type(other.type) { TODO_SINGLE_COMPOUND(type); switch (type.getBasic()) { case Type::i32: @@ -54,6 +51,7 @@ Literal& Literal::operator=(const Literal& other) { func = other.func; break; case Type::exnref: + // Avoid calling the destructor on an uninitialized value new (&exn) auto(std::make_unique(*other.exn)); break; case Type::none: @@ -63,6 +61,13 @@ Literal& Literal::operator=(const Literal& other) { case Type::unreachable: WASM_UNREACHABLE("unexpected type"); } +} + +Literal& Literal::operator=(const Literal& other) { + if (this != &other) { + this->~Literal(); + new (this) auto(other); + } return *this; } @@ -124,6 +129,11 @@ std::array Literal::getv128() const { return ret; } +ExceptionPackage Literal::getExceptionPackage() const { + assert(type == Type::exnref); + return *exn.get(); +} + Literal Literal::castToF32() { assert(type == Type::i32); Literal ret(i32); -- cgit v1.2.3