From ef674ecb40d1dcdcb39a33a7d28772edaeff63b8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 20 Mar 2023 15:14:52 -0700 Subject: [Wasm GC] Fix detection of externalize/internalize as constant (#5592) Both isValidInConstantExpression and isSingleConstantExpression must look recursively at the internals of a RefAs that externalizes and internalizes, or else we might do something like externalize a local.get, which is not constant. getLiteral must handle externalize/internalize as well, and return a properly- modified literal. Without these fixes the testcase hits different internal assertions, and we either fail to recognize something is constant or not, or think that it is but fail to produce a literal for it. --- src/ir/properties.cpp | 8 ++++++++ src/ir/properties.h | 15 ++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/ir/properties.cpp b/src/ir/properties.cpp index 4ade8aa10..b564b3bf1 100644 --- a/src/ir/properties.cpp +++ b/src/ir/properties.cpp @@ -36,6 +36,8 @@ bool isGenerative(Expression* curr, FeatureSet features) { return scanner.generative; } +// Checks an expression in a shallow manner (i.e., does not check children) as +// to whether it is valid in a wasm constant expression. static bool isValidInConstantExpression(Module& wasm, Expression* expr) { if (isSingleConstantExpression(expr) || expr->is() || expr->is() || expr->is() || expr->is() || @@ -43,6 +45,12 @@ static bool isValidInConstantExpression(Module& wasm, Expression* expr) { return true; } + if (auto* refAs = expr->dynCast()) { + if (refAs->op == ExternExternalize || refAs->op == ExternInternalize) { + return true; + } + } + if (auto* get = expr->dynCast()) { auto* g = wasm.getGlobalOrNull(get->name); // This is called from the validator, so we have to handle non-existent diff --git a/src/ir/properties.h b/src/ir/properties.h index 13eea6ba2..d47cf774b 100644 --- a/src/ir/properties.h +++ b/src/ir/properties.h @@ -82,10 +82,13 @@ inline bool isNamedControlFlow(Expression* curr) { // runtime will be equal as well. TODO: combine this with // isValidInConstantExpression or find better names(#4845) inline bool isSingleConstantExpression(const Expression* curr) { + if (auto* refAs = curr->dynCast()) { + if (refAs->op == ExternExternalize || refAs->op == ExternInternalize) { + return isSingleConstantExpression(refAs->value); + } + } return curr->is() || curr->is() || curr->is() || - curr->is() || - (curr->is() && (curr->cast()->op == ExternExternalize || - curr->cast()->op == ExternInternalize)); + curr->is(); } inline bool isConstantExpression(const Expression* curr) { @@ -120,6 +123,12 @@ inline Literal getLiteral(const Expression* curr) { } } else if (auto* s = curr->dynCast()) { return Literal(s->string.toString()); + } else if (auto* r = curr->dynCast()) { + if (r->op == ExternExternalize) { + return getLiteral(r->value).externalize(); + } else if (r->op == ExternInternalize) { + return getLiteral(r->value).internalize(); + } } WASM_UNREACHABLE("non-constant expression"); } -- cgit v1.2.3