From: rossberg@chromium.org Date: Tue, 6 May 2014 16:02:18 +0000 (+0000) Subject: Revert "Prevent liveedit on or under generators with open activations" X-Git-Tag: upstream/4.7.83~9241 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5c9ad091e960ce35e367f53e40a95fc9d99911af;p=platform%2Fupstream%2Fv8.git Revert "Prevent liveedit on or under generators with open activations" Seems to crash some tests on buildbots. TBR=ishell@chromium.org CC=wingo@igalia.com,yangguo@chromium.org BUG= Review URL: https://codereview.chromium.org/273433002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21178 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/liveedit-debugger.js b/src/liveedit-debugger.js index 07214f9..021a4f0 100644 --- a/src/liveedit-debugger.js +++ b/src/liveedit-debugger.js @@ -946,9 +946,7 @@ Debug.LiveEdit = new function() { BLOCKED_ON_ACTIVE_STACK: 2, BLOCKED_ON_OTHER_STACK: 3, BLOCKED_UNDER_NATIVE_CODE: 4, - REPLACED_ON_ACTIVE_STACK: 5, - BLOCKED_UNDER_GENERATOR: 6, - BLOCKED_ACTIVE_GENERATOR: 7 + REPLACED_ON_ACTIVE_STACK: 5 }; FunctionPatchabilityStatus.SymbolName = function(code) { diff --git a/src/liveedit.cc b/src/liveedit.cc index c3dd872..e6bb4b2 100644 --- a/src/liveedit.cc +++ b/src/liveedit.cc @@ -1682,6 +1682,11 @@ static const char* DropFrames(Vector 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 { @@ -1735,20 +1740,12 @@ static const char* DropActivationsInActiveThreadImpl( bool target_frame_found = false; int bottom_js_frame_index = top_frame_index; - bool non_droppable_frame_found = false; - LiveEdit::FunctionPatchabilityStatus non_droppable_reason; + bool c_code_found = false; for (; frame_index < frames.length(); frame_index++) { StackFrame* frame = frames[frame_index]; - if (frame->is_exit()) { - non_droppable_frame_found = true; - non_droppable_reason = LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE; - break; - } - if (!frame->is_java_script()) continue; - if (JavaScriptFrame::cast(frame)->function()->shared()->is_generator()) { - non_droppable_frame_found = true; - non_droppable_reason = LiveEdit::FUNCTION_BLOCKED_UNDER_GENERATOR; + if (!IsDropableFrame(frame)) { + c_code_found = true; break; } if (target.MatchActivation( @@ -1758,15 +1755,15 @@ static const char* DropActivationsInActiveThreadImpl( } } - 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. + if (c_code_found) { + // There is a C frames on stack. 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, non_droppable_reason)) { - // Fail. + if (target.MatchActivation( + frame, LiveEdit::FUNCTION_BLOCKED_UNDER_NATIVE_CODE)) { + // Cannot drop frame under C frames. return NULL; } } @@ -1836,47 +1833,6 @@ static const char* DropActivationsInActiveThread( } -bool LiveEdit::FindActiveGenerators(Handle shared_info_array, - Handle 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 = - Handle::cast(FixedArray::get(shared_info_array, i)); - Handle 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 shared_info_array, @@ -1907,29 +1863,18 @@ Handle LiveEdit::CheckAndDropActivations( Isolate* isolate = shared_info_array->GetIsolate(); int len = GetArrayLength(shared_info_array); - CHECK(shared_info_array->HasFastElements()); - Handle shared_info_array_elements( - FixedArray::cast(shared_info_array->elements())); - Handle result = isolate->factory()->NewJSArray(len); - Handle result_elements = - JSObject::EnsureWritableFastElements(result); // Fill the default values. for (int i = 0; i < len; i++) { - FunctionPatchabilityStatus status = FUNCTION_AVAILABLE_FOR_PATCH; - result_elements->set(i, Smi::FromInt(status)); + SetElementSloppy( + result, + i, + Handle(Smi::FromInt(FUNCTION_AVAILABLE_FOR_PATCH), isolate)); } - // 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; - } - // Check inactive threads. Fail if some functions are blocked there. + // First check inactive threads. Fail if some functions are blocked there. InactiveThreadActivationsChecker inactive_threads_checker(shared_info_array, result); isolate->thread_manager()->IterateArchivedThreads( @@ -1992,9 +1937,6 @@ 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; } diff --git a/src/liveedit.h b/src/liveedit.h index 63174e0..5be63ac 100644 --- a/src/liveedit.h +++ b/src/liveedit.h @@ -89,11 +89,6 @@ class LiveEdit : AllStatic { Handle orig_function_shared, Handle subst_function_shared); - // Find open generator activations, and set corresponding "result" elements to - // FUNCTION_BLOCKED_ACTIVE_GENERATOR. - static bool FindActiveGenerators(Handle shared_info_array, - Handle 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 @@ -112,9 +107,7 @@ 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_BLOCKED_UNDER_GENERATOR = 6, - FUNCTION_BLOCKED_ACTIVE_GENERATOR = 7 + FUNCTION_REPLACED_ON_ACTIVE_STACK = 5 }; // Compares 2 strings line-by-line, then token-wise and returns diff in form diff --git a/src/objects-inl.h b/src/objects-inl.h index 30bb8a1..029e80d 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -5718,14 +5718,6 @@ 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); diff --git a/src/objects.h b/src/objects.h index b464013..12f2ee0 100644 --- a/src/objects.h +++ b/src/objects.h @@ -7460,8 +7460,6 @@ 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 deleted file mode 100644 index 341ef48..0000000 --- a/test/mjsunit/harmony/generators-debug-liveedit.js +++ /dev/null @@ -1,119 +0,0 @@ -// 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); - })); -})(); diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index a08f5ff..036fb48 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -183,7 +183,6 @@ '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], @@ -280,7 +279,6 @@ '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], @@ -332,7 +330,6 @@ '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,7 +350,6 @@ 'debug-liveedit-stack-padding': [SKIP], 'debug-liveedit-restart-frame': [SKIP], 'debug-liveedit-double-call': [SKIP], - 'harmony/generators-debug-liveedit': [SKIP], # This test dumps core for arm.debug, so no reason to expect it to work # for NaCl. The other three fuzz-natives tests seem to run fine.