From 62d056f889d4b94562a104e2fcad318857550d5b Mon Sep 17 00:00:00 2001 From: Thomas Lively <7121787+tlively@users.noreply.github.com> Date: Fri, 30 Sep 2022 16:20:23 -0500 Subject: Fix bugs with copying expressions (#5099) It does not make sense to construct an `Expression` directly because all expressions must be specific expressions. However, we previously allowed constructing Expressions, and in particular we allowed them to be copy constructed. Unrelatedly, `Fatal::operator<<` took its argument by value. Together, these two facts produced UB when printing Expressions in fatal error messages because a new Expression would be copy constructed with the original expression ID but without any of the actual data from the original specific expression. For example, when trying to print a Block, the printing code would try to look at the expression list, but the expression list would be junk stack data because the copied Expression does not contain an expression list. Fix the problem by making Expression's constructors visible only to its subclasses and making `Fatal::operator<<` take its argument by forwarding reference instead of by value. --- src/support/utilities.h | 2 +- src/wasm.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/support/utilities.h b/src/support/utilities.h index 57d170269..a8acfeed7 100644 --- a/src/support/utilities.h +++ b/src/support/utilities.h @@ -64,7 +64,7 @@ std::unique_ptr make_unique(Args&&... args) { class Fatal { public: Fatal() { std::cerr << "Fatal: "; } - template Fatal& operator<<(T arg) { + template Fatal& operator<<(T&& arg) { std::cerr << arg; return *this; } diff --git a/src/wasm.h b/src/wasm.h index eb5c15514..65f70cef9 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -746,6 +746,15 @@ public: Expression(Id id) : _id(id) {} +protected: + // An expression cannot be constructed without knowing what kind of expression + // it should be. + Expression(const Expression& other) = default; + Expression(Expression&& other) = default; + Expression& operator=(Expression& other) = default; + Expression& operator=(Expression&& other) = default; + +public: void finalize() {} template bool is() const { -- cgit v1.2.3