From b1d43d0b31e8aea7b31261764fef5bee4ad13903 Mon Sep 17 00:00:00 2001 From: bradnelson Date: Thu, 28 Jan 2016 17:05:29 -0800 Subject: [PATCH] Accurately type foreign functions, and variables. Associate a type with foreign functions at their callsite. Associate a type with foreign variables. More pervasively forbid computation in the module body. Confirm foreign call arguments are exports. 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/1642993002 Cr-Commit-Position: refs/heads/master@{#33596} --- src/typing-asm.cc | 58 ++++++++++-- src/typing-asm.h | 3 + test/cctest/test-asm-validator.cc | 151 +++++++++++++++++++++++++++++- 3 files changed, 201 insertions(+), 11 deletions(-) diff --git a/src/typing-asm.cc b/src/typing-asm.cc index 14db16ba94..6e073f9204 100644 --- a/src/typing-asm.cc +++ b/src/typing-asm.cc @@ -36,7 +36,6 @@ namespace internal { if (!valid_) return; \ } while (false) - AsmTyper::AsmTyper(Isolate* isolate, Zone* zone, Script* script, FunctionLiteral* root) : zone_(zone), @@ -62,6 +61,7 @@ AsmTyper::AsmTyper(Isolate* isolate, Zone* zone, Script* script, ZoneAllocationPolicy(zone)), in_function_(false), building_function_tables_(false), + visiting_exports_(false), cache_(TypeCache::Get()) { InitializeAstVisitor(isolate); InitializeStdlib(); @@ -135,6 +135,7 @@ void AsmTyper::VisitAsmModule(FunctionLiteral* fun) { } // Validate exports. + visiting_exports_ = true; ReturnStatement* stmt = fun->body()->last()->AsReturnStatement(); if (stmt == nullptr) { FAIL(fun->body()->last(), "last statement in module is not a return"); @@ -489,11 +490,11 @@ void AsmTyper::VisitDebuggerStatement(DebuggerStatement* stmt) { void AsmTyper::VisitFunctionLiteral(FunctionLiteral* expr) { - Scope* scope = expr->scope(); - DCHECK(scope->is_function_scope()); if (in_function_) { FAIL(expr, "invalid nested function"); } + Scope* scope = expr->scope(); + DCHECK(scope->is_function_scope()); if (!expr->bounds().upper->IsFunction()) { FAIL(expr, "invalid function literal"); @@ -523,6 +524,9 @@ void AsmTyper::VisitDoExpression(DoExpression* expr) { void AsmTyper::VisitConditional(Conditional* expr) { + if (!in_function_) { + FAIL(expr, "ternary operator inside module body"); + } RECURSE(VisitWithExpectation(expr->condition(), Type::Number(), "condition expected to be integer")); if (!computed_type_->Is(cache_.kAsmInt)) { @@ -554,8 +558,18 @@ void AsmTyper::VisitConditional(Conditional* expr) { void AsmTyper::VisitVariableProxy(VariableProxy* expr) { + VisitVariableProxy(expr, false); +} + +void AsmTyper::VisitVariableProxy(VariableProxy* expr, bool assignment) { Variable* var = expr->var(); VariableInfo* info = GetVariableInfo(var, false); + if (!assignment && !in_function_ && !building_function_tables_ && + !visiting_exports_) { + if (var->location() != VariableLocation::PARAMETER || var->index() >= 3) { + FAIL(expr, "illegal variable reference in module body"); + } + } if (info == NULL || info->type == NULL) { if (var->mode() == TEMPORARY) { SetType(var, Type::Any(zone())); @@ -675,8 +689,8 @@ void AsmTyper::VisitAssignment(Assignment* expr) { FAIL(expr, "intish or floatish assignment"); } if (expr->target()->IsVariableProxy()) { - RECURSE(VisitWithExpectation(expr->target(), target_type, - "assignment target expected to match value")); + expected_type_ = target_type; + VisitVariableProxy(expr->target()->AsVariableProxy(), true); } else if (expr->target()->IsProperty()) { Property* property = expr->target()->AsProperty(); RECURSE(VisitWithExpectation(property->obj(), Type::Any(), @@ -912,6 +926,7 @@ void AsmTyper::VisitProperty(Property* expr) { void AsmTyper::VisitCall(Call* expr) { + Type* expected_type = expected_type_; RECURSE(VisitWithExpectation(expr->expression(), Type::Any(), "callee expected to be any")); StandardMember standard_member = kNone; @@ -974,9 +989,17 @@ void AsmTyper::VisitCall(Call* expr) { Expression* arg = args->at(i); RECURSE(VisitWithExpectation(arg, Type::Any(), "foreign call argument expected to be any")); + // Checking for asm extern types explicitly, as the type system + // doesn't correctly check their inheritance relationship. + if (!computed_type_->Is(cache_.kAsmSigned) && + !computed_type_->Is(cache_.kAsmFixnum) && + !computed_type_->Is(cache_.kAsmDouble)) { + FAIL(arg, + "foreign call argument expected to be int, double, or fixnum"); + } } intish_ = kMaxUncombinedAdditiveSteps; - IntersectResult(expr, Type::Number()); + IntersectResult(expr, expected_type); } else { FAIL(expr, "invalid callee"); } @@ -1014,6 +1037,9 @@ void AsmTyper::VisitCallRuntime(CallRuntime* expr) { void AsmTyper::VisitUnaryOperation(UnaryOperation* expr) { + if (!in_function_) { + FAIL(expr, "unary operator inside module body"); + } switch (expr->op()) { case Token::NOT: // Used to encode != and !== RECURSE(VisitWithExpectation(expr->expression(), cache_.kAsmInt, @@ -1041,7 +1067,7 @@ void AsmTyper::VisitIntegerBitwiseOperator(BinaryOperation* expr, Type* left_expected, Type* right_expected, Type* result_type, bool conversion) { - RECURSE(VisitWithExpectation(expr->left(), Type::Number(), + RECURSE(VisitWithExpectation(expr->left(), Type::Number(zone()), "left bitwise operand expected to be a number")); int left_intish = intish_; Type* left_type = computed_type_; @@ -1082,6 +1108,15 @@ void AsmTyper::VisitIntegerBitwiseOperator(BinaryOperation* expr, void AsmTyper::VisitBinaryOperation(BinaryOperation* expr) { + if (!in_function_) { + if (expr->op() != Token::BIT_OR && expr->op() != Token::MUL) { + FAIL(expr, "illegal binary operator inside module body"); + } + if (!(expr->left()->IsProperty() || expr->left()->IsVariableProxy()) || + !expr->right()->IsLiteral()) { + FAIL(expr, "illegal computation inside module body"); + } + } switch (expr->op()) { case Token::COMMA: { RECURSE(VisitWithExpectation(expr->left(), Type::Any(), @@ -1098,6 +1133,9 @@ void AsmTyper::VisitBinaryOperation(BinaryOperation* expr) { // BIT_OR allows Any since it is used as a type coercion. VisitIntegerBitwiseOperator(expr, Type::Any(), cache_.kAsmInt, cache_.kAsmSigned, true); + if (expr->left()->IsCall() && expr->op() == Token::BIT_OR) { + IntersectResult(expr->left(), cache_.kAsmSigned); + } return; } case Token::BIT_XOR: { @@ -1184,6 +1222,9 @@ void AsmTyper::VisitBinaryOperation(BinaryOperation* expr) { } else if (expr->op() == Token::MUL && expr->right()->IsLiteral() && right_type->Is(cache_.kAsmDouble)) { // For unary +, expressed as x * 1.0 + if (expr->left()->IsCall() && expr->op() == Token::MUL) { + IntersectResult(expr->left(), cache_.kAsmDouble); + } IntersectResult(expr, cache_.kAsmDouble); return; } else if (type->Is(cache_.kAsmFloat) && expr->op() != Token::MOD) { @@ -1207,6 +1248,9 @@ void AsmTyper::VisitBinaryOperation(BinaryOperation* expr) { void AsmTyper::VisitCompareOperation(CompareOperation* expr) { + if (!in_function_) { + FAIL(expr, "comparison inside module body"); + } Token::Value op = expr->op(); if (op != Token::EQ && op != Token::NE && op != Token::LT && op != Token::LTE && op != Token::GT && op != Token::GTE) { diff --git a/src/typing-asm.h b/src/typing-asm.h index b7f53831e6..63e204e50a 100644 --- a/src/typing-asm.h +++ b/src/typing-asm.h @@ -113,6 +113,7 @@ class AsmTyper : public AstVisitor { bool in_function_; // In module function? bool building_function_tables_; + bool visiting_exports_; TypeCache const& cache_; @@ -161,6 +162,8 @@ class AsmTyper : public AstVisitor { void VisitLiteral(Literal* expr, bool is_return); + void VisitVariableProxy(VariableProxy* expr, bool assignment); + void VisitIntegerBitwiseOperator(BinaryOperation* expr, Type* left_expected, Type* right_expected, Type* result_type, bool conversion); diff --git a/test/cctest/test-asm-validator.cc b/test/cctest/test-asm-validator.cc index 28fd7c6772..03610930e5 100644 --- a/test/cctest/test-asm-validator.cc +++ b/test/cctest/test-asm-validator.cc @@ -1726,7 +1726,7 @@ TEST(ForeignFunction) { "function foo() { bar(); }") { CHECK_EXPR(FunctionLiteral, FUNC_I_TYPE) { CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmSigned)) { - CHECK_EXPR(Call, Bounds(Type::Number(zone))) { + CHECK_EXPR(Call, Bounds(cache.kAsmSigned)) { CHECK_VAR(baz, Bounds(Type::Any(zone))); CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum)); CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum)); @@ -1788,7 +1788,7 @@ TEST(BadStandardFunctionCallOutside) { "var s0 = sin(0);\n" "function bar() { }\n" "function foo() { bar(); }", - "asm: line 39: calls forbidden outside function bodies\n"); + "asm: line 39: illegal variable reference in module body\n"); } @@ -1797,7 +1797,7 @@ TEST(BadFunctionCallOutside) { "function bar() { return 0.0; }\n" "var s0 = bar(0);\n" "function foo() { bar(); }", - "asm: line 40: calls forbidden outside function bodies\n"); + "asm: line 40: illegal variable reference in module body\n"); } @@ -1837,7 +1837,7 @@ TEST(NestedAssignmentInHeap) { CHECK_EXPR(Property, Bounds::Unbounded()) { CHECK_VAR(i8, Bounds(cache.kInt8Array)); CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmSigned)) { - CHECK_EXPR(Assignment, Bounds(cache.kAsmSigned)) { + CHECK_EXPR(Assignment, Bounds(cache.kAsmInt)) { CHECK_VAR(x, Bounds(cache.kAsmInt)); CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum)); } @@ -2050,3 +2050,146 @@ TEST(BadSwitchOrder) { "function foo() { bar(); }", "asm: line 39: default case out of order\n"); } + +TEST(BadForeignCall) { + const char test_function[] = + "function TestModule(stdlib, foreign, buffer) {\n" + " \"use asm\";\n" + " var ffunc = foreign.foo;\n" + " function test1() { var x = 0; ffunc(x); }\n" + " return { testFunc1: test1 };\n" + "}\n"; + v8::V8::Initialize(); + HandleAndZoneScope handles; + Zone* zone = handles.main_zone(); + ZoneVector types(zone); + CHECK_EQ( + "asm: line 4: foreign call argument expected to be int, double, or " + "fixnum\n", + Validate(zone, test_function, &types)); +} + +TEST(BadImports) { + const char test_function[] = + "function TestModule(stdlib, foreign, buffer) {\n" + " \"use asm\";\n" + " var fint = (foreign.bar | 0) | 0;\n" + " function test1() {}\n" + " return { testFunc1: test1 };\n" + "}\n"; + v8::V8::Initialize(); + HandleAndZoneScope handles; + Zone* zone = handles.main_zone(); + ZoneVector types(zone); + CHECK_EQ("asm: line 3: illegal computation inside module body\n", + Validate(zone, test_function, &types)); +} + +TEST(BadVariableReference) { + const char test_function[] = + "function TestModule(stdlib, foreign, buffer) {\n" + " \"use asm\";\n" + " var x = 0;\n" + " var y = x;\n" + " function test1() {}\n" + " return { testFunc1: test1 };\n" + "}\n"; + v8::V8::Initialize(); + HandleAndZoneScope handles; + Zone* zone = handles.main_zone(); + ZoneVector types(zone); + CHECK_EQ("asm: line 4: illegal variable reference in module body\n", + Validate(zone, test_function, &types)); +} + +TEST(Imports) { + const char test_function[] = + "function TestModule(stdlib, foreign, buffer) {\n" + " \"use asm\";\n" + " var ffunc = foreign.foo;\n" + " var fint = foreign.bar | 0;\n" + " var fdouble = +foreign.baz;\n" + " function test1() { return ffunc(fint|0, fdouble) | 0; }\n" + " function test2() { return +ffunc(fdouble, fint|0); }\n" + " return { testFunc1: test1, testFunc2: test2 };\n" + "}\n"; + + v8::V8::Initialize(); + HandleAndZoneScope handles; + Zone* zone = handles.main_zone(); + ZoneVector types(zone); + CHECK_EQ("", Validate(zone, test_function, &types)); + TypeCache cache; + + CHECK_TYPES_BEGIN { + // Module. + CHECK_EXPR(FunctionLiteral, Bounds::Unbounded()) { + // function test1 + CHECK_EXPR(FunctionLiteral, FUNC_I_TYPE) { + CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmSigned)) { + CHECK_EXPR(Call, Bounds(cache.kAsmSigned)) { + CHECK_VAR(ffunc, Bounds(Type::Any(zone))); + CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmSigned)) { + CHECK_VAR(fint, Bounds(cache.kAsmInt)); + CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum)); + } + CHECK_VAR(fdouble, Bounds(cache.kAsmDouble)); + } + CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum)); + } + } + // function test2 + CHECK_EXPR(FunctionLiteral, FUNC_D_TYPE) { + CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmDouble)) { + CHECK_EXPR(Call, Bounds(cache.kAsmDouble)) { + CHECK_VAR(ffunc, Bounds(Type::Any(zone))); + CHECK_VAR(fdouble, Bounds(cache.kAsmDouble)); + CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmSigned)) { + CHECK_VAR(fint, Bounds(cache.kAsmInt)); + CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum)); + } + } + CHECK_EXPR(Literal, Bounds(cache.kAsmDouble)); + } + } + // "use asm"; + CHECK_EXPR(Literal, Bounds(Type::String(zone))); + // var func = foreign.foo; + CHECK_EXPR(Assignment, Bounds(Type::Any(zone))) { + CHECK_VAR(ffunc, Bounds(Type::Any(zone))); + CHECK_EXPR(Property, Bounds(Type::Any(zone))) { + CHECK_VAR(foreign, Bounds::Unbounded()); + CHECK_EXPR(Literal, Bounds::Unbounded()); + } + } + // var fint = foreign.bar | 0; + CHECK_EXPR(Assignment, Bounds(cache.kAsmInt)) { + CHECK_VAR(fint, Bounds(cache.kAsmInt)); + CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmSigned)) { + CHECK_EXPR(Property, Bounds(Type::Number())) { + CHECK_VAR(foreign, Bounds::Unbounded()); + CHECK_EXPR(Literal, Bounds::Unbounded()); + } + CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum)); + } + } + // var fdouble = +foreign.baz; + CHECK_EXPR(Assignment, Bounds(cache.kAsmDouble)) { + CHECK_VAR(fdouble, Bounds(cache.kAsmDouble)); + CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmDouble)) { + CHECK_EXPR(Property, Bounds(Type::Number())) { + CHECK_VAR(foreign, Bounds::Unbounded()); + CHECK_EXPR(Literal, Bounds::Unbounded()); + } + CHECK_EXPR(Literal, Bounds(cache.kAsmDouble)); + } + } + // return { testFunc1: test1, testFunc2: test2 }; + CHECK_EXPR(ObjectLiteral, Bounds::Unbounded()) { + CHECK_VAR(test1, FUNC_I_TYPE); + CHECK_VAR(test2, FUNC_D_TYPE); + } + } + } + CHECK_TYPES_END +}