From 8b752419594deb607a4c3e0b91dceb27a90ee583 Mon Sep 17 00:00:00 2001
From: Dominic Gannaway <dg@domgan.com>
Date: Thu, 12 Jul 2018 23:30:58 -0700
Subject: [PATCH] Prevent loops from getting stuck in infinite loops (#2247)

Summary:
Release notes: stop abstract loops from getting stuck in infinite loops when body contains a return, throw or break completion

Fixes https://github.com/facebook/prepack/issues/2053. This PR adds logic to "for loops" where they might get stuck in an infinite loop when there is no incrementor but there is a return, throw or break completion in the loop body. We check this via a new function `andfailIfContainsBreakOrReturnOrThrowCompletion` and only do this after `100` iterations of evaluating the loop body. I was thinking of making the iteration count configurable but wanted feedback on the approach first. I tried `1000` iterations, but it actually ended up taking about 2 minutes to evaluate the test case :/
Pull Request resolved: https://github.com/facebook/prepack/pull/2247

Differential Revision: D8827425

Pulled By: trueadm

fbshipit-source-id: d05f539e2c8a6dce15b7f23c1b76e89087437738
---
 src/evaluators/ForStatement.js                | 35 +++++++++++++++++++
 .../optimized-functions/LoopBailout18.js      | 31 ++++++++++++++++
 .../optimized-functions/LoopBailout19.js      | 15 ++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 test/serializer/optimized-functions/LoopBailout18.js
 create mode 100644 test/serializer/optimized-functions/LoopBailout19.js

diff --git a/src/evaluators/ForStatement.js b/src/evaluators/ForStatement.js
index 3324b9d12..3297dd40a 100644
--- a/src/evaluators/ForStatement.js
+++ b/src/evaluators/ForStatement.js
@@ -99,6 +99,7 @@ function ForBodyEvaluation(
   // 2. Perform ? CreatePerIterationEnvironment(perIterationBindings).
   CreatePerIterationEnvironment(realm, perIterationBindings);
   let env = realm.getRunningContext().lexicalEnvironment;
+  let possibleInfiniteLoopIterations = 0;
 
   // 3. Repeat
   while (true) {
@@ -159,10 +160,39 @@ function ForBodyEvaluation(
 
       // ii. Perform ? GetValue(incRef).
       Environment.GetValue(realm, incRef);
+    } else if (realm.useAbstractInterpretation) {
+      // If we have no increment and we've hit 100 iterations of trying to evaluate
+      // this loop body, then see if we have a break, return or throw completion in a
+      // guarded condition and fail if it does. We already have logic to guard
+      // against loops that are actually infinite. However, because there may be so
+      // many forked execution paths, and they're non linear, then it might
+      // computationally lead to a something that seems like an infinite loop.
+      possibleInfiniteLoopIterations++;
+      if (possibleInfiniteLoopIterations > 100) {
+        failIfContainsBreakOrReturnOrThrowCompletion(realm.savedCompletion);
+      }
     }
   }
   invariant(false);
 
+  function failIfContainsBreakOrReturnOrThrowCompletion(c: void | Completion | Value) {
+    if (c === undefined) return;
+    if (c instanceof ThrowCompletion || c instanceof BreakCompletion || c instanceof ReturnCompletion) {
+      let diagnostic = new CompilerDiagnostic(
+        "break, throw or return cannot be guarded by abstract condition",
+        c.location,
+        "PP0035",
+        "FatalError"
+      );
+      realm.handleError(diagnostic);
+      throw new FatalError();
+    }
+    if (c instanceof PossiblyNormalCompletion || c instanceof ForkedAbruptCompletion) {
+      failIfContainsBreakOrReturnOrThrowCompletion(c.consequent);
+      failIfContainsBreakOrReturnOrThrowCompletion(c.alternate);
+    }
+  }
+
   function failIfContainsBreakOrContinueCompletionWithNonLocalTarget(c: void | Completion | Value) {
     if (c === undefined) return;
     if (c instanceof ContinueCompletion || c instanceof BreakCompletion) {
@@ -322,6 +352,11 @@ let BailOutWrapperClosureRefVisitor = {
       let { id, init } = node.declarations[index];
 
       if (t.isIdentifier(id)) {
+        // If init is undefined, then we need to ensure we provide
+        // an actual Babel undefined node for it.
+        if (init === null) {
+          init = t.identifier("undefined");
+        }
         return t.assignmentExpression("=", id, init);
       } else {
         // We do not currently support ObjectPattern, SpreadPattern and ArrayPattern
diff --git a/test/serializer/optimized-functions/LoopBailout18.js b/test/serializer/optimized-functions/LoopBailout18.js
new file mode 100644
index 000000000..c33e60dda
--- /dev/null
+++ b/test/serializer/optimized-functions/LoopBailout18.js
@@ -0,0 +1,31 @@
+function fn(x) {
+  for (
+    var _iterator = x.items,
+      _isArray = Array.isArray(_iterator),
+      _i = 0,
+      _iterator = _isArray ? _iterator : _iterator[Symbol.iterator]();
+    ;
+
+  ) {
+    // ...
+
+    var _ref;
+
+    if (_isArray) {
+      if (_i >= _iterator.length) break;
+      _ref = _iterator[_i++];
+    } else {
+      _i = _iterator.next();
+      if (_i.done) break;
+      _ref = _i.value;
+    }
+
+    var item = _ref;
+  }
+}
+
+global.__optimize && __optimize(fn);
+
+inspect = function() {
+  return JSON.stringify(fn([1, 2, 3, 4, 5]));
+};
diff --git a/test/serializer/optimized-functions/LoopBailout19.js b/test/serializer/optimized-functions/LoopBailout19.js
new file mode 100644
index 000000000..c35635d9b
--- /dev/null
+++ b/test/serializer/optimized-functions/LoopBailout19.js
@@ -0,0 +1,15 @@
+function fn(x, oldItems) {
+  var items = [];
+  for (var i; i < x; ) {
+    i++;
+    var oldItem = oldItems[i];
+    items.push(oldItem + 2);
+  }
+  return items;
+}
+
+global.__optimize && __optimize(fn);
+
+inspect = function() {
+  return JSON.stringify(fn(5, [1, 2, 3, 4, 5]));
+};