[Turbofan]: Fix error in serializer try ranges with generators

ripsawridge · Commit Bot · commit d5dd2e660c15 · 2019-11-13T09:28:17.000Z
When simulating bytecode, we store the current environment at the site of the appropriate catch handler when entering a try range. If the start of the try range is dead, we don't bother to store an environment. However, generators can create alive regions inside the try range. At such moments, we should recognize we're in a try range and store the environment for the handler. Bug: chromium:1017159 Change-Id: Icccc2ccf530895099bc62b97d9aaec8b97d5f4e8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1879247 Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Maya Lekova <mslekova@chromium.org> Commit-Queue: Michael Stanton <mvstanton@chromium.org> Cr-Commit-Position: refs/heads/master@{#64929}
diff --git a/src/compiler/serializer-for-background-compilation.cc b/src/compiler/serializer-for-background-compilation.cc
@@ -1028,31 +1028,64 @@ class HandlerRangeMatcher {
       : bytecode_iterator_(bytecode_iterator) {
     HandlerTable table(*bytecode_array);
     for (int i = 0, n = table.NumberOfRangeEntries(); i < n; ++i) {
-      ranges_.insert(
-          std::make_pair(table.GetRangeStart(i), table.GetRangeHandler(i)));
+      ranges_.insert({table.GetRangeStart(i), table.GetRangeEnd(i),
+                      table.GetRangeHandler(i)});
     }
     ranges_iterator_ = ranges_.cbegin();
   }
 
-  int NextHandlerOffsetForRangeStart() {
+  using OffsetReporter = std::function<void(int handler_offset)>;
+
+  void HandlerOffsetForCurrentPosition(const OffsetReporter& offset_reporter) {
     CHECK(!bytecode_iterator_.done());
-    int handler_offset = -1;
+    const int current_offset = bytecode_iterator_.current_offset();
+
+    // Remove outdated try ranges from the stack.
+    while (!stack_.empty()) {
+      const int end = stack_.top().end;
+      if (end < current_offset) {
+        stack_.pop();
+      } else {
+        break;
+      }
+    }
+
+    // Advance the iterator and maintain the stack.
     while (ranges_iterator_ != ranges_.cend() &&
-           ranges_iterator_->first < bytecode_iterator_.current_offset()) {
+           ranges_iterator_->start <= current_offset) {
+      if (ranges_iterator_->end >= current_offset) {
+        stack_.push(*ranges_iterator_);
+        if (ranges_iterator_->start == current_offset) {
+          offset_reporter(ranges_iterator_->handler);
+        }
+      }
       ranges_iterator_++;
     }
-    if (ranges_iterator_ != ranges_.cend() &&
-        ranges_iterator_->first == bytecode_iterator_.current_offset()) {
-      handler_offset = ranges_iterator_->second;
-      ranges_iterator_++;
+
+    if (!stack_.empty() && stack_.top().start < current_offset) {
+      offset_reporter(stack_.top().handler);
     }
-    return handler_offset;
   }
 
  private:
   BytecodeArrayIterator const& bytecode_iterator_;
-  std::multimap<int, int> ranges_;
-  std::multimap<int, int>::const_iterator ranges_iterator_;
+
+  struct Range {
+    int start;
+    int end;
+    int handler;
+    friend bool operator<(const Range& a, const Range& b) {
+      if (a.start < b.start) return true;
+      if (a.start == b.start) {
+        if (a.end < b.end) return true;
+        CHECK_GT(a.end, b.end);
+      }
+      return false;
+    }
+  };
+  std::set<Range> ranges_;
+  std::set<Range>::const_iterator ranges_iterator_;
+  std::stack<Range> stack_;
 };
 
 Handle<FeedbackVector> SerializerForBackgroundCompilation::feedback_vector()
@@ -1104,16 +1137,16 @@ void SerializerForBackgroundCompilation::TraverseBytecode() {
       continue;  // Skip this bytecode since TF won't generate code for it.
     }
 
-    int handler_offset = try_start_matcher.NextHandlerOffsetForRangeStart();
-    while (handler_offset >= 0) {
-      // We may have nested try ranges that nonetheless start at the same
-      // offset. We loop here in order to save the environment for each catch
-      // handler.
-      TRACE_BROKER(broker(),
-                   "Entered try block. Handler at offset " << handler_offset);
-      ContributeToJumpTargetEnvironment(handler_offset);
-      handler_offset = try_start_matcher.NextHandlerOffsetForRangeStart();
-    }
+    auto save_handler_environments = [&](int handler_offset) {
+      auto it = jump_target_environments_.find(handler_offset);
+      if (it == jump_target_environments_.end()) {
+        ContributeToJumpTargetEnvironment(handler_offset);
+        TRACE_BROKER(broker(),
+                     "Handler offset for current pos: " << handler_offset);
+      }
+    };
+    try_start_matcher.HandlerOffsetForCurrentPosition(
+        save_handler_environments);
 
     if (bytecode_analysis.IsLoopHeader(current_offset)) {
       // Graph builder might insert jumps to resume targets in the loop body.
diff --git a/test/mjsunit/regress/regress-crbug-1017159.js b/test/mjsunit/regress/regress-crbug-1017159.js
@@ -0,0 +1,27 @@
+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Flags: --allow-natives-syntax
+
+function* foo() {
+  __v_1 = foo.x;   // LdaNamedProperty
+  for (;;) {
+    try {
+      yield;
+      try {
+        __v_0 == "object";
+        __v_1[__getRandomProperty()]();
+      } catch(e) {
+        print();
+      }
+    } catch(e) {
+      "Caught: " + e;
+    }
+    break;
+  }
+};
+
+%PrepareFunctionForOptimization(foo);
+%OptimizeFunctionOnNextCall(foo);
+foo();
