Reland "Prevent liveedit on or under generators with open activations"
authoryangguo@chromium.org <yangguo@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Thu, 22 May 2014 07:32:59 +0000 (07:32 +0000)
committeryangguo@chromium.org <yangguo@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Thu, 22 May 2014 07:32:59 +0000 (07:32 +0000)
The change relative to the previous CL is a logic change in
DropActivationsInActiveThreadImpl.  The previous CL skipped the matcher
unless the frame was a JS frame; this was correct for
MultipleFunctionTarget but not for SingleFrameTarget.

I have not been able to reproduce the original failures on either
architecture (ia32 or x64; stack frame dropping is unsupported on other
architectures).

R=yangguo@chromium.org
LOG=N
TEST=mjsunit/harmony/generators-debug-liveedit.js
BUG=

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

Patch from Andy Wingo <wingo@igalia.com>.

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21419 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/liveedit-debugger.js
src/liveedit.cc
src/liveedit.h
src/objects-inl.h
src/objects.h
test/mjsunit/harmony/generators-debug-liveedit.js [new file with mode: 0644]
test/mjsunit/mjsunit.status

index 021a4f0..07214f9 100644 (file)
@@ -946,7 +946,9 @@ Debug.LiveEdit = new function() {
       BLOCKED_ON_ACTIVE_STACK: 2,
       BLOCKED_ON_OTHER_STACK: 3,
       BLOCKED_UNDER_NATIVE_CODE: 4,
-      REPLACED_ON_ACTIVE_STACK: 5
+      REPLACED_ON_ACTIVE_STACK: 5,
+      BLOCKED_UNDER_GENERATOR: 6,
+      BLOCKED_ACTIVE_GENERATOR: 7
   };
 
   FunctionPatchabilityStatus.SymbolName = function(code) {
index e6bb4b2..0158577 100644 (file)
@@ -1682,11 +1682,6 @@ static const char* DropFrames(Vector<StackFrame*> frames,
 }
 
 
-static bool IsDropableFrame(StackFrame* frame) {
-  return !frame->is_exit();
-}
-
-
 // Describes a set of call frames that execute any of listed functions.
 // Finding no such frames does not mean error.
 class MultipleFunctionTarget {
@@ -1740,12 +1735,20 @@ static const char* DropActivationsInActiveThreadImpl(
 
   bool target_frame_found = false;
   int bottom_js_frame_index = top_frame_index;
-  bool c_code_found = false;
+  bool non_droppable_frame_found = false;
+  LiveEdit::FunctionPatchabilityStatus non_droppable_reason;
 
   for (; frame_index < frames.length(); frame_index++) {
     StackFrame* frame = frames[frame_index];
-    if (!IsDropableFrame(frame)) {
-      c_code_found = true;
+    if (frame->is_exit()) {
+      non_droppable_frame_found = true;
+      non_droppable_reason = LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE;
+      break;
+    }
+    if (frame->is_java_script() &&
+        JavaScriptFrame::cast(frame)->function()->shared()->is_generator()) {
+      non_droppable_frame_found = true;
+      non_droppable_reason = LiveEdit::FUNCTION_BLOCKED_UNDER_GENERATOR;
       break;
     }
     if (target.MatchActivation(
@@ -1755,15 +1758,15 @@ static const char* DropActivationsInActiveThreadImpl(
     }
   }
 
-  if (c_code_found) {
-    // There is a C frames on stack. Check that there are no target frames
-    // below them.
+  if (non_droppable_frame_found) {
+    // There is a C or generator frame on stack.  We can't drop C frames, and we
+    // can't restart generators.  Check that there are no target frames below
+    // them.
     for (; frame_index < frames.length(); frame_index++) {
       StackFrame* frame = frames[frame_index];
       if (frame->is_java_script()) {
-        if (target.MatchActivation(
-                frame, LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE)) {
-          // Cannot drop frame under C frames.
+        if (target.MatchActivation(frame, non_droppable_reason)) {
+          // Fail.
           return NULL;
         }
       }
@@ -1833,6 +1836,47 @@ static const char* DropActivationsInActiveThread(
 }
 
 
+bool LiveEdit::FindActiveGenerators(Handle<FixedArray> shared_info_array,
+                                    Handle<FixedArray> result,
+                                    int len) {
+  Isolate* isolate = shared_info_array->GetIsolate();
+  Heap* heap = isolate->heap();
+  heap->EnsureHeapIsIterable();
+  bool found_suspended_activations = false;
+
+  ASSERT_LE(len, result->length());
+
+  DisallowHeapAllocation no_allocation;
+
+  FunctionPatchabilityStatus active = FUNCTION_BLOCKED_ACTIVE_GENERATOR;
+
+  HeapIterator iterator(heap);
+  HeapObject* obj = NULL;
+  while ((obj = iterator.next()) != NULL) {
+    if (!obj->IsJSGeneratorObject()) continue;
+
+    JSGeneratorObject* gen = JSGeneratorObject::cast(obj);
+    if (gen->is_closed()) continue;
+
+    HandleScope scope(isolate);
+
+    for (int i = 0; i < len; i++) {
+      Handle<JSValue> jsvalue =
+          Handle<JSValue>::cast(FixedArray::get(shared_info_array, i));
+      Handle<SharedFunctionInfo> shared =
+          UnwrapSharedFunctionInfoFromJSValue(jsvalue);
+
+      if (gen->function()->shared() == *shared) {
+        result->set(i, Smi::FromInt(active));
+        found_suspended_activations = true;
+      }
+    }
+  }
+
+  return found_suspended_activations;
+}
+
+
 class InactiveThreadActivationsChecker : public ThreadVisitor {
  public:
   InactiveThreadActivationsChecker(Handle<JSArray> shared_info_array,
@@ -1863,18 +1907,29 @@ Handle<JSArray> LiveEdit::CheckAndDropActivations(
   Isolate* isolate = shared_info_array->GetIsolate();
   int len = GetArrayLength(shared_info_array);
 
+  CHECK(shared_info_array->HasFastElements());
+  Handle<FixedArray> shared_info_array_elements(
+      FixedArray::cast(shared_info_array->elements()));
+
   Handle<JSArray> result = isolate->factory()->NewJSArray(len);
+  Handle<FixedArray> result_elements =
+      JSObject::EnsureWritableFastElements(result);
 
   // Fill the default values.
   for (int i = 0; i < len; i++) {
-    SetElementSloppy(
-        result,
-        i,
-        Handle<Smi>(Smi::FromInt(FUNCTION_AVAILABLE_FOR_PATCH), isolate));
+    FunctionPatchabilityStatus status = FUNCTION_AVAILABLE_FOR_PATCH;
+    result_elements->set(i, Smi::FromInt(status));
   }
 
+  // Scan the heap for active generators -- those that are either currently
+  // running (as we wouldn't want to restart them, because we don't know where
+  // to restart them from) or suspended.  Fail if any one corresponds to the set
+  // of functions being edited.
+  if (FindActiveGenerators(shared_info_array_elements, result_elements, len)) {
+    return result;
+  }
 
-  // First check inactive threads. Fail if some functions are blocked there.
+  // Check inactive threads. Fail if some functions are blocked there.
   InactiveThreadActivationsChecker inactive_threads_checker(shared_info_array,
                                                             result);
   isolate->thread_manager()->IterateArchivedThreads(
@@ -1937,6 +1992,9 @@ const char* LiveEdit::RestartFrame(JavaScriptFrame* frame) {
   if (target.saved_status() == LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE) {
     return "Function is blocked under native code";
   }
+  if (target.saved_status() == LiveEdit::FUNCTION_BLOCKED_UNDER_GENERATOR) {
+    return "Function is blocked under a generator activation";
+  }
   return NULL;
 }
 
index e06688a..b22d01a 100644 (file)
@@ -89,6 +89,11 @@ class LiveEdit : AllStatic {
                                          Handle<JSValue> orig_function_shared,
                                          Handle<JSValue> subst_function_shared);
 
+  // Find open generator activations, and set corresponding "result" elements to
+  // FUNCTION_BLOCKED_ACTIVE_GENERATOR.
+  static bool FindActiveGenerators(Handle<FixedArray> shared_info_array,
+                                   Handle<FixedArray> result, int len);
+
   // Checks listed functions on stack and return array with corresponding
   // FunctionPatchabilityStatus statuses; extra array element may
   // contain general error message. Modifies the current stack and
@@ -107,7 +112,9 @@ class LiveEdit : AllStatic {
     FUNCTION_BLOCKED_ON_ACTIVE_STACK = 2,
     FUNCTION_BLOCKED_ON_OTHER_STACK = 3,
     FUNCTION_BLOCKED_UNDER_NATIVE_CODE = 4,
-    FUNCTION_REPLACED_ON_ACTIVE_STACK = 5
+    FUNCTION_REPLACED_ON_ACTIVE_STACK = 5,
+    FUNCTION_BLOCKED_UNDER_GENERATOR = 6,
+    FUNCTION_BLOCKED_ACTIVE_GENERATOR = 7
   };
 
   // Compares 2 strings line-by-line, then token-wise and returns diff in form
index dca3eca..319e6a8 100644 (file)
@@ -5715,6 +5715,14 @@ bool JSGeneratorObject::is_suspended() {
   return continuation() > 0;
 }
 
+bool JSGeneratorObject::is_closed() {
+  return continuation() == kGeneratorClosed;
+}
+
+bool JSGeneratorObject::is_executing() {
+  return continuation() == kGeneratorExecuting;
+}
+
 JSGeneratorObject* JSGeneratorObject::cast(Object* obj) {
   ASSERT(obj->IsJSGeneratorObject());
   ASSERT(HeapObject::cast(obj)->Size() == JSGeneratorObject::kSize);
index 944efb9..a161b0b 100644 (file)
@@ -7495,6 +7495,8 @@ class JSGeneratorObject: public JSObject {
   // cannot be resumed.
   inline int continuation();
   inline void set_continuation(int continuation);
+  inline bool is_closed();
+  inline bool is_executing();
   inline bool is_suspended();
 
   // [operand_stack]: Saved operand stack.
diff --git a/test/mjsunit/harmony/generators-debug-liveedit.js b/test/mjsunit/harmony/generators-debug-liveedit.js
new file mode 100644 (file)
index 0000000..341ef48
--- /dev/null
@@ -0,0 +1,119 @@
+// Copyright 2014 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: --expose-debug-as debug --harmony-generators
+
+var Debug = debug.Debug;
+var LiveEdit = Debug.LiveEdit;
+
+unique_id = 0;
+
+var Generator = (function*(){}).constructor;
+
+function assertIteratorResult(value, done, result) {
+  assertEquals({value: value, done: done}, result);
+}
+
+function MakeGenerator() {
+  // Prevents eval script caching.
+  unique_id++;
+  return Generator('callback',
+      "/* " + unique_id + "*/\n" +
+      "yield callback();\n" +
+      "return 'Cat';\n");
+}
+
+function MakeFunction() {
+  // Prevents eval script caching.
+  unique_id++;
+  return Function('callback',
+      "/* " + unique_id + "*/\n" +
+      "callback();\n" +
+      "return 'Cat';\n");
+}
+
+// First, try MakeGenerator with no perturbations.
+(function(){
+  var generator = MakeGenerator();
+  function callback() {};
+  var iter = generator(callback);
+  assertIteratorResult(undefined, false, iter.next());
+  assertIteratorResult("Cat", true, iter.next());
+})();
+
+function patch(fun, from, to) {
+  function debug() {
+    var log = new Array();
+    var script = Debug.findScript(fun);
+    var pos = script.source.indexOf(from);
+    try {
+      LiveEdit.TestApi.ApplySingleChunkPatch(script, pos, from.length, to,
+                                             log);
+    } finally {
+      print("Change log: " + JSON.stringify(log) + "\n");
+    }
+  }
+  Debug.ExecuteInDebugContext(debug, false);
+}
+
+// Try to edit a MakeGenerator while it's running, then again while it's
+// stopped.
+(function(){
+  var generator = MakeGenerator();
+
+  var gen_patch_attempted = false;
+  function attempt_gen_patch() {
+    assertFalse(gen_patch_attempted);
+    gen_patch_attempted = true;
+    assertThrows(function() { patch(generator, "'Cat'", "'Capybara'") },
+                 LiveEdit.Failure);
+  };
+  var iter = generator(attempt_gen_patch);
+  assertIteratorResult(undefined, false, iter.next());
+  // Patch should not succeed because there is a live generator activation on
+  // the stack.
+  assertIteratorResult("Cat", true, iter.next());
+  assertTrue(gen_patch_attempted);
+
+  // At this point one iterator is live, but closed, so the patch will succeed.
+  patch(generator, "'Cat'", "'Capybara'");
+  iter = generator(function(){});
+  assertIteratorResult(undefined, false, iter.next());
+  // Patch successful.
+  assertIteratorResult("Capybara", true, iter.next());
+
+  // Patching will fail however when a live iterator is suspended.
+  iter = generator(function(){});
+  assertIteratorResult(undefined, false, iter.next());
+  assertThrows(function() { patch(generator, "'Capybara'", "'Tapir'") },
+               LiveEdit.Failure);
+  assertIteratorResult("Capybara", true, iter.next());
+
+  // Try to patch functions with activations inside and outside generator
+  // function activations.  We should succeed in the former case, but not in the
+  // latter.
+  var fun_outside = MakeFunction();
+  var fun_inside = MakeFunction();
+  var fun_patch_attempted = false;
+  var fun_patch_restarted = false;
+  function attempt_fun_patches() {
+    if (fun_patch_attempted) {
+      assertFalse(fun_patch_restarted);
+      fun_patch_restarted = true;
+      return;
+    }
+    fun_patch_attempted = true;
+    // Patching outside a generator activation must fail.
+    assertThrows(function() { patch(fun_outside, "'Cat'", "'Cobra'") },
+                 LiveEdit.Failure);
+    // Patching inside a generator activation may succeed.
+    patch(fun_inside, "'Cat'", "'Koala'");
+  }
+  iter = generator(function() { return fun_inside(attempt_fun_patches) });
+  assertEquals('Cat',
+               fun_outside(function () {
+                 assertIteratorResult('Koala', false, iter.next());
+                 assertTrue(fun_patch_restarted);
+               }));
+})();
index e460e7c..2df8747 100644 (file)
   'debug-liveedit-stack-padding': [SKIP],
   'debug-liveedit-restart-frame': [SKIP],
   'debug-liveedit-double-call': [SKIP],
+  'harmony/generators-debug-liveedit': [SKIP],
 
   # BUG(v8:3147). It works on other architectures by accident.
   'regress/regress-conditional-position': [FAIL],
   'debug-liveedit-stack-padding': [SKIP],
   'debug-liveedit-restart-frame': [SKIP],
   'debug-liveedit-double-call': [SKIP],
+  'harmony/generators-debug-liveedit': [SKIP],
 
   # Currently always deopt on minus zero
   'math-floor-of-div-minus-zero': [SKIP],
   'debug-liveedit-stack-padding': [SKIP],
   'debug-liveedit-restart-frame': [SKIP],
   'debug-liveedit-double-call': [SKIP],
+  'harmony/generators-debug-liveedit': [SKIP],
 
   # Currently always deopt on minus zero
   'math-floor-of-div-minus-zero': [SKIP],
   'debug-liveedit-stack-padding': [SKIP],
   'debug-liveedit-restart-frame': [SKIP],
   'debug-liveedit-double-call': [SKIP],
+  'harmony/generators-debug-liveedit': [SKIP],
 
   # NaCl builds have problems with this test since Pepper_28.
   # V8 Issue 2786