diff options
| author | Thomas Lively <7121787+tlively@users.noreply.github.com> | 2020-09-01 15:55:58 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-09-01 15:55:58 -0700 |
| commit | 85234cb540c2b2863dfea6c20496dcb451df8889 (patch) | |
| tree | a2475df8d4f79a4d04f43cb991b4a4a07b59b9c4 | |
| parent | fecfa92d71633f8bb28bb97a60adc9b54998456f (diff) | |
| download | binaryen-85234cb540c2b2863dfea6c20496dcb451df8889.tar.gz binaryen-85234cb540c2b2863dfea6c20496dcb451df8889.tar.bz2 binaryen-85234cb540c2b2863dfea6c20496dcb451df8889.zip | |
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.
| -rw-r--r-- | src/literal.h | 5 | ||||
| -rw-r--r-- | src/wasm-interpreter.h | 2 | ||||
| -rw-r--r-- | src/wasm/literal.cpp | 18 |
3 files changed, 16 insertions, 9 deletions
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<ExceptionPackage>(*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<uint8_t, 16> 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); |