Revert of Accurately type foreign functions, and variables. (patchset #2...

Revert of Accurately type foreign functions, and variables. (patchset #2 id:20001 of https://codereview.chromium.org/1642993002/ )

Reason for revert:
[Sheriff] Breaks arm x-compile:
https://build.chromium.org/p/client.v8/builders/V8%20Arm%20-%20debug%20builder/builds/7484/steps/compile/logs/stdio

Original issue's description:
> 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
>
> Committed: https://crrev.com/b1d43d0b31e8aea7b31261764fef5bee4ad13903
> Cr-Commit-Position: refs/heads/master@{#33596}

TBR=aseemgarg@chromium.org,titzer@chromium.org,bradnelson@google.com,bradnelson@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= https://code.google.com/p/v8/issues/detail?id=4203

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

Cr-Commit-Position: refs/heads/master@{#33597}

src/typing-asm.cc

@@ -36,6 +36,7 @@ namespace internal {
     if (!valid_) return;            \
   } while (false)
 
+
 AsmTyper::AsmTyper(Isolate* isolate, Zone* zone, Script* script,
                    FunctionLiteral* root)
     : zone_(zone),
@@ -61,7 +62,6 @@ 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,7 +135,6 @@ 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");
@@ -490,11 +489,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");
@@ -524,9 +523,6 @@ 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)) {
@@ -558,18 +554,8 @@ 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()));
@@ -689,8 +675,8 @@ void AsmTyper::VisitAssignment(Assignment* expr) {
     FAIL(expr, "intish or floatish assignment");
   }
   if (expr->target()->IsVariableProxy()) {
-    expected_type_ = target_type;
-    VisitVariableProxy(expr->target()->AsVariableProxy(), true);
+    RECURSE(VisitWithExpectation(expr->target(), target_type,
+                                 "assignment target expected to match value"));
   } else if (expr->target()->IsProperty()) {
     Property* property = expr->target()->AsProperty();
     RECURSE(VisitWithExpectation(property->obj(), Type::Any(),
@@ -926,7 +912,6 @@ 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;
@@ -989,17 +974,9 @@ 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, expected_type);
+    IntersectResult(expr, Type::Number());
   } else {
     FAIL(expr, "invalid callee");
   }
@@ -1037,9 +1014,6 @@ 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,
@@ -1067,7 +1041,7 @@ void AsmTyper::VisitIntegerBitwiseOperator(BinaryOperation* expr,
                                            Type* left_expected,
                                            Type* right_expected,
                                            Type* result_type, bool conversion) {
-  RECURSE(VisitWithExpectation(expr->left(), Type::Number(zone()),
+  RECURSE(VisitWithExpectation(expr->left(), Type::Number(),
                                "left bitwise operand expected to be a number"));
   int left_intish = intish_;
   Type* left_type = computed_type_;
@@ -1108,15 +1082,6 @@ 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(),
@@ -1133,9 +1098,6 @@ 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: {
@@ -1222,9 +1184,6 @@ 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) {
@@ -1248,9 +1207,6 @@ 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) {

src/typing-asm.h

@@ -113,7 +113,6 @@ class AsmTyper : public AstVisitor {
 
   bool in_function_;  // In module function?
   bool building_function_tables_;
-  bool visiting_exports_;
 
   TypeCache const& cache_;
 
@@ -162,8 +161,6 @@ 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);

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(cache.kAsmSigned)) {
+        CHECK_EXPR(Call, Bounds(Type::Number(zone))) {
           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: illegal variable reference in module body\n");
+      "asm: line 39: calls forbidden outside function bodies\n");
 }
 
 
@@ -1797,7 +1797,7 @@ TEST(BadFunctionCallOutside) {
       "function bar() { return 0.0; }\n"
       "var s0 = bar(0);\n"
       "function foo() { bar(); }",
-      "asm: line 40: illegal variable reference in module body\n");
+      "asm: line 40: calls forbidden outside function bodies\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.kAsmInt)) {
+            CHECK_EXPR(Assignment, Bounds(cache.kAsmSigned)) {
               CHECK_VAR(x, Bounds(cache.kAsmInt));
               CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum));
             }
@@ -2050,146 +2050,3 @@ 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<ExpressionTypeEntry> 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<ExpressionTypeEntry> 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<ExpressionTypeEntry> 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<ExpressionTypeEntry> 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
-}