Commit d0befa95 authored by bradnelson's avatar bradnelson Committed by Commit bot

[wasm][asm.js] Permit ternary operator in asm.js returns in some cases.

In practice, Emscripten seems to emit cond?+a:+b type return
expressions. This is not allowed by the spec or errata, but we need
to support it for compatibility.

Similar patterns with ints / signed, do not seem to be supported.

BUG=v8:5891
R=mtrofin@chromium.org,aseemgarg@chromium.org

Review-Url: https://codereview.chromium.org/2648353010
Cr-Commit-Position: refs/heads/master@{#42677}
parent 8bf52534
......@@ -1225,10 +1225,12 @@ AsmType* AsmTyper::ValidateFunction(FunctionDeclaration* fun_decl) {
if (as_block != nullptr) {
statements = as_block->statements();
} else {
// We don't check whether AsReturnStatement() below returns non-null --
// we leave that to the ReturnTypeAnnotations method.
RECURSE(return_type_ =
ReturnTypeAnnotations(last_statement->AsReturnStatement()));
if (auto* ret_statement = last_statement->AsReturnStatement()) {
RECURSE(return_type_ =
ReturnTypeAnnotations(ret_statement->expression()));
} else {
return_type_ = AsmType::Void();
}
}
}
} while (return_type_ == AsmType::None());
......@@ -2747,15 +2749,8 @@ AsmType* AsmTyper::ParameterTypeAnnotations(Variable* parameter,
}
// 5.2 ReturnTypeAnnotations
AsmType* AsmTyper::ReturnTypeAnnotations(ReturnStatement* statement) {
if (statement == nullptr) {
return AsmType::Void();
}
auto* ret_expr = statement->expression();
if (ret_expr == nullptr) {
return AsmType::Void();
}
AsmType* AsmTyper::ReturnTypeAnnotations(Expression* ret_expr) {
DCHECK_NOT_NULL(ret_expr);
if (auto* binop = ret_expr->AsBinaryOperation()) {
if (IsDoubleAnnotation(binop)) {
......@@ -2763,14 +2758,14 @@ AsmType* AsmTyper::ReturnTypeAnnotations(ReturnStatement* statement) {
} else if (IsIntAnnotation(binop)) {
return AsmType::Signed();
}
FAIL(statement, "Invalid return type annotation.");
FAIL(ret_expr, "Invalid return type annotation.");
}
if (auto* call = ret_expr->AsCall()) {
if (IsCallToFround(call)) {
return AsmType::Float();
}
FAIL(statement, "Invalid function call in return statement.");
FAIL(ret_expr, "Invalid function call in return statement.");
}
if (auto* literal = ret_expr->AsLiteral()) {
......@@ -2789,28 +2784,46 @@ AsmType* AsmTyper::ReturnTypeAnnotations(ReturnStatement* statement) {
// return undefined
return AsmType::Void();
}
FAIL(statement, "Invalid literal in return statement.");
FAIL(ret_expr, "Invalid literal in return statement.");
}
if (auto* proxy = ret_expr->AsVariableProxy()) {
auto* var_info = Lookup(proxy->var());
if (var_info == nullptr) {
FAIL(statement, "Undeclared identifier in return statement.");
FAIL(ret_expr, "Undeclared identifier in return statement.");
}
if (var_info->mutability() != VariableInfo::kConstGlobal) {
FAIL(statement, "Identifier in return statement is not const.");
FAIL(ret_expr, "Identifier in return statement is not const.");
}
if (!var_info->type()->IsReturnType()) {
FAIL(statement, "Constant in return must be signed, float, or double.");
FAIL(ret_expr, "Constant in return must be signed, float, or double.");
}
return var_info->type();
}
FAIL(statement, "Invalid return type expression.");
// NOTE: This is not strictly valid asm.js, but is emitted by some versions of
// Emscripten.
if (auto* cond = ret_expr->AsConditional()) {
AsmType* a = AsmType::None();
AsmType* b = AsmType::None();
RECURSE(a = ReturnTypeAnnotations(cond->then_expression()));
if (a->IsA(AsmType::None())) {
return a;
}
RECURSE(b = ReturnTypeAnnotations(cond->else_expression()));
if (b->IsA(AsmType::None())) {
return b;
}
if (a->IsExactly(b)) {
return a;
}
}
FAIL(ret_expr, "Invalid return type expression.");
}
// 5.4 VariableTypeAnnotations
......
......@@ -364,7 +364,7 @@ class AsmTyper final {
AsmType* ParameterTypeAnnotations(Variable* parameter,
Expression* annotation);
// 5.2 ReturnTypeAnnotations
AsmType* ReturnTypeAnnotations(ReturnStatement* statement);
AsmType* ReturnTypeAnnotations(Expression* ret_expr);
// 5.4 VariableTypeAnnotations
// 5.5 GlobalVariableTypeAnnotations
AsmType* VariableTypeAnnotations(
......
......@@ -856,9 +856,10 @@ TEST(ErrorsInFunction) {
"}\n",
"Undeclared identifier in return statement"},
{"function f() {\n"
" var i = 0;\n"
" return i?0:1;\n"
"}\n",
"Invalid return type expression"},
"Type mismatch in return statement"},
{"function f() {\n"
" return stdlib.Math.E;"
"}\n",
......
......@@ -413,3 +413,66 @@ function assertValidAsm(func) {
Module();
assertFalse(%IsAsmWasmCode(Module));
})();
(function TestConditionalReturn() {
function Module() {
'use asm';
function foo(a, b) {
a = +a;
b = +b;
// Allowed, despite not matching the spec, as emscripten emits this in
// practice.
return a == b ? +a : +b;
}
return foo;
}
var m = Module();
assertEquals(4, m(4, 4));
assertEquals(5, m(4, 5));
assertEquals(4, m(5, 4));
assertValidAsm(Module);
})();
(function TestMismatchedConditionalReturn() {
function Module() {
'use asm';
function foo(a, b) {
a = +a;
return a == 0.0 ? 0 : +a;
}
return foo;
}
Module();
assertFalse(% IsAsmWasmCode(Module));
})();
(function TestBadIntConditionalReturn() {
function Module() {
'use asm';
function foo(a, b) {
a = a | 0;
b = b | 0;
// Disallowed because signature must be signed, but these will be int.
return 1 ? a : b;
}
return foo;
}
Module();
assertFalse(% IsAsmWasmCode(Module));
})();
(function TestBadSignedConditionalReturn() {
function Module() {
'use asm';
function foo(a, b) {
a = a | 0;
b = b | 0;
// Disallowed because conditional yields int, even when both sides
// are signed.
return 1 ? a | 0 : b | 0;
}
return foo;
}
Module();
assertFalse(% IsAsmWasmCode(Module));
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment