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

Enforce asm restrictions on switch more precisely.

Enforce cases have no duplicates.
Enforce cases have a maximum range of 2^31.
Enforce default case comes last.

BUG= https://code.google.com/p/v8/issues/detail?id=4203
TEST=test-asm-validator
R=aseemgarg@chromium.org,titzer@chromium.org
LOG=N

Review URL: https://codereview.chromium.org/1578963003

Cr-Commit-Position: refs/heads/master@{#33221}
parent 210e65ed
......@@ -2,10 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "src/v8.h"
#include "src/typing-asm.h"
#include <limits>
#include "src/v8.h"
#include "src/ast/ast.h"
#include "src/ast/scopes.h"
#include "src/codegen.h"
......@@ -386,9 +388,14 @@ void AsmTyper::VisitSwitchStatement(SwitchStatement* stmt) {
RECURSE(VisitWithExpectation(stmt->tag(), cache_.kAsmSigned,
"switch expression non-integer"));
ZoneList<CaseClause*>* clauses = stmt->cases();
ZoneSet<int32_t> cases(zone());
for (int i = 0; i < clauses->length(); ++i) {
CaseClause* clause = clauses->at(i);
if (!clause->is_default()) {
if (clause->is_default()) {
if (i != clauses->length() - 1) {
FAIL(clause, "default case out of order");
}
} else {
Expression* label = clause->label();
RECURSE(VisitWithExpectation(label, cache_.kAsmSigned,
"case label non-integer"));
......@@ -396,11 +403,22 @@ void AsmTyper::VisitSwitchStatement(SwitchStatement* stmt) {
Handle<Object> value = label->AsLiteral()->value();
int32_t value32;
if (!value->ToInt32(&value32)) FAIL(label, "illegal case label value");
if (cases.find(value32) != cases.end()) {
FAIL(label, "duplicate case value");
}
cases.insert(value32);
}
// TODO(bradnelson): Detect duplicates.
ZoneList<Statement*>* stmts = clause->statements();
RECURSE(VisitStatements(stmts));
}
if (cases.size() > 0) {
int64_t min_case = *cases.begin();
int64_t max_case = *cases.rbegin();
if (max_case - min_case > std::numeric_limits<int32_t>::max()) {
FAIL(stmt, "case range too large");
}
}
}
......
......@@ -2024,3 +2024,27 @@ TEST(SwitchTest) {
}
CHECK_FUNC_TYPES_END
}
TEST(BadSwitchRange) {
CHECK_FUNC_ERROR(
"function bar() { switch (1) { case -1: case 0x7fffffff: } }\n"
"function foo() { bar(); }",
"asm: line 39: case range too large\n");
}
TEST(DuplicateSwitchCase) {
CHECK_FUNC_ERROR(
"function bar() { switch (1) { case 0: case 0: } }\n"
"function foo() { bar(); }",
"asm: line 39: duplicate case value\n");
}
TEST(BadSwitchOrder) {
CHECK_FUNC_ERROR(
"function bar() { switch (1) { default: case 0: } }\n"
"function foo() { bar(); }",
"asm: line 39: default case out of order\n");
}
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