Revision: 21419
Author: [email protected]
Date: Thu May 22 07:32:59 2014 UTC
Log: Reland "Prevent liveedit on or under generators with open
activations"
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).
[email protected]
LOG=N
TEST=mjsunit/harmony/generators-debug-liveedit.js
BUG=
Review URL: https://codereview.chromium.org/270283002
Patch from Andy Wingo <[email protected]>.
http://code.google.com/p/v8/source/detail?r=21419
Added:
/branches/bleeding_edge/test/mjsunit/harmony/generators-debug-liveedit.js
Modified:
/branches/bleeding_edge/src/liveedit-debugger.js
/branches/bleeding_edge/src/liveedit.cc
/branches/bleeding_edge/src/liveedit.h
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/test/mjsunit/mjsunit.status
=======================================
--- /dev/null
+++
/branches/bleeding_edge/test/mjsunit/harmony/generators-debug-liveedit.js
Thu May 22 07:32:59 2014 UTC
@@ -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);
+ }));
+})();
=======================================
--- /branches/bleeding_edge/src/liveedit-debugger.js Tue May 6 16:02:18
2014 UTC
+++ /branches/bleeding_edge/src/liveedit-debugger.js Thu May 22 07:32:59
2014 UTC
@@ -946,7 +946,9 @@
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) {
=======================================
--- /branches/bleeding_edge/src/liveedit.cc Wed May 21 09:49:18 2014 UTC
+++ /branches/bleeding_edge/src/liveedit.cc Thu May 22 07:32:59 2014 UTC
@@ -1680,11 +1680,6 @@
return NULL;
}
-
-
-static bool IsDropableFrame(StackFrame* frame) {
- return !frame->is_exit();
-}
// Describes a set of call frames that execute any of listed functions.
@@ -1740,14 +1735,22 @@
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(
frame, LiveEdit::FUNCTION_BLOCKED_ON_ACTIVE_STACK)) {
target_frame_found = true;
@@ -1755,15 +1758,15 @@
}
}
- 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;
}
}
@@ -1831,6 +1834,47 @@
}
return NULL;
}
+
+
+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 {
@@ -1863,18 +1907,29 @@
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 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/liveedit.h Thu May 8 09:33:11 2014 UTC
+++ /branches/bleeding_edge/src/liveedit.h Thu May 22 07:32:59 2014 UTC
@@ -89,6 +89,11 @@
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 @@
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
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Tue May 20 14:22:05 2014 UTC
+++ /branches/bleeding_edge/src/objects-inl.h Thu May 22 07:32:59 2014 UTC
@@ -5714,6 +5714,14 @@
ASSERT_EQ(kGeneratorClosed, 0);
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());
=======================================
--- /branches/bleeding_edge/src/objects.h Wed May 21 12:16:47 2014 UTC
+++ /branches/bleeding_edge/src/objects.h Thu May 22 07:32:59 2014 UTC
@@ -7495,6 +7495,8 @@
// 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.
=======================================
--- /branches/bleeding_edge/test/mjsunit/mjsunit.status Thu May 8 14:34:37
2014 UTC
+++ /branches/bleeding_edge/test/mjsunit/mjsunit.status Thu May 22 07:32:59
2014 UTC
@@ -206,6 +206,7 @@
'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],
@@ -302,6 +303,7 @@
'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],
@@ -353,6 +355,7 @@
'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],
@@ -373,6 +376,7 @@
'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
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
