Revision: 4069
Author: [email protected]
Date: Tue Mar 9 13:45:24 2010
Log: Check that function being patched has no activations on any thread
stack
Review URL: http://codereview.chromium.org/668246
http://code.google.com/p/v8/source/detail?r=4069
Added:
/branches/bleeding_edge/test/mjsunit/debug-liveedit-check-stack.js
Modified:
/branches/bleeding_edge/src/debug-delay.js
/branches/bleeding_edge/src/liveedit-delay.js
/branches/bleeding_edge/src/liveedit.cc
/branches/bleeding_edge/src/liveedit.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/runtime.h
/branches/bleeding_edge/test/mjsunit/fuzz-natives.js
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/debug-liveedit-check-stack.js Tue
Mar 9 13:45:24 2010
@@ -0,0 +1,84 @@
+// Copyright 2010 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --expose-debug-as debug
+// Get the Debug object exposed from the debug context global object.
+
+Debug = debug.Debug
+
+eval(
+ "function ChooseAnimal(callback) {\n " +
+ " callback();\n" +
+ " return 'Cat';\n" +
+ "}\n"
+);
+
+function Noop() {}
+var res = ChooseAnimal(Noop);
+
+assertEquals("Cat", res);
+
+var script = Debug.findScript(ChooseAnimal);
+
+var orig_animal = "'Cat'";
+var patch_pos = script.source.indexOf(orig_animal);
+var new_animal_patch = "'Capybara'";
+
+var got_exception = false;
+var successfully_changed = false;
+
+function Changer() {
+ // Never try the same patch again.
+ assertEquals(false, successfully_changed);
+ var change_log = new Array();
+ try {
+ Debug.LiveEditChangeScript(script, patch_pos, orig_animal.length,
new_animal_patch, change_log);
+ successfully_changed = true;
+ } catch (e) {
+ if (e instanceof Debug.LiveEditChangeScript.Failure) {
+ got_exception = true;
+ print(e);
+ } else {
+ throw e;
+ }
+ }
+ print("Change log: " + JSON.stringify(change_log) + "\n");
+}
+
+var new_res = ChooseAnimal(Changer);
+// Function must be not pached.
+assertEquals("Cat", new_res);
+
+assertEquals(true, got_exception);
+
+// This time it should succeed.
+Changer();
+
+new_res = ChooseAnimal(Noop);
+// Function must be not pached.
+assertEquals("Capybara", new_res);
+
=======================================
--- /branches/bleeding_edge/src/debug-delay.js Fri Mar 5 14:08:58 2010
+++ /branches/bleeding_edge/src/debug-delay.js Tue Mar 9 13:45:24 2010
@@ -1986,9 +1986,18 @@
}
var change_log = new Array();
- Debug.LiveEditChangeScript(the_script, change_pos, change_len,
new_string,
- change_log);
-
+ try {
+ Debug.LiveEditChangeScript(the_script, change_pos, change_len,
new_string,
+ change_log);
+ } catch (e) {
+ if (e instanceof Debug.LiveEditChangeScript.Failure) {
+ // Let's treat it as a "success" so that body with change_log will be
+ // sent back. "change_log" will have "failure" field set.
+ change_log.push( { failure: true } );
+ } else {
+ throw e;
+ }
+ }
response.body = {change_log: change_log};
};
=======================================
--- /branches/bleeding_edge/src/liveedit-delay.js Fri Mar 5 14:08:58 2010
+++ /branches/bleeding_edge/src/liveedit-delay.js Tue Mar 9 13:45:24 2010
@@ -47,39 +47,9 @@
Debug.LiveEditChangeScript = function(script, change_pos, change_len,
new_str,
change_log) {
- function Assert(condition, message) {
- if (!condition) {
- if (message) {
- throw "Assert " + message;
- } else {
- throw "Assert";
- }
- }
- }
-
- // An object describing function compilation details. Its index fields
- // apply to indexes inside array that stores these objects.
- function FunctionCompileInfo(raw_array) {
- this.function_name = raw_array[0];
- this.start_position = raw_array[1];
- this.end_position = raw_array[2];
- this.param_num = raw_array[3];
- this.code = raw_array[4];
- this.scope_info = raw_array[5];
- this.outer_index = raw_array[6];
- this.next_sibling_index = null;
- this.raw_array = raw_array;
- }
-
- // A structure describing SharedFunctionInfo.
- function SharedInfoWrapper(raw_array) {
- this.function_name = raw_array[0];
- this.start_position = raw_array[1];
- this.end_position = raw_array[2];
- this.info = raw_array[3];
- this.raw_array = raw_array;
- }
-
+ // So far the function works as namespace.
+ var liveedit = Debug.LiveEditChangeScript;
+ var Assert = liveedit.Assert;
// Fully compiles source string as a script. Returns Array of
// FunctionCompileInfo -- a descriptions of all functions of the script.
@@ -98,7 +68,7 @@
var compile_info = new Array();
var old_index_map = new Array();
for (var i = 0; i < raw_compile_info.length; i++) {
- compile_info.push(new FunctionCompileInfo(raw_compile_info[i]));
+ compile_info.push(new
liveedit.FunctionCompileInfo(raw_compile_info[i]));
old_index_map.push(i);
}
@@ -169,30 +139,6 @@
return index;
}
- // Compares a function interface old and new version, whether it
- // changed or not.
- function CompareFunctionExpectations(function_info1, function_info2) {
- // Check that function has the same number of parameters (there may
exist
- // an adapter, that won't survive function parameter number change).
- if (function_info1.param_num != function_info2.param_num) {
- return false;
- }
- var scope_info1 = function_info1.scope_info;
- var scope_info2 = function_info2.scope_info;
-
- if (!scope_info1) {
- return !scope_info2;
- }
-
- if (scope_info1.length != scope_info2.length) {
- return false;
- }
-
- // Check that outer scope structure is not changed. Otherwise the
function
- // will not properly work with existing scopes.
- return scope_info1.toString() == scope_info2.toString();
- }
-
// Variable forward declarations. Preprocessor "Minifier" needs them.
var old_compile_info;
var shared_infos;
@@ -265,6 +211,12 @@
// Find all SharedFunctionInfo's that are compiled from this script.
var shared_raw_list = %LiveEditFindSharedFunctionInfosForScript(script);
+
+ var shared_infos = new Array();
+
+ for (var i = 0; i < shared_raw_list.length; i++) {
+ shared_infos.push(new liveedit.SharedInfoWrapper(shared_raw_list[i]));
+ }
// Gather compile information about old version of script.
var old_compile_info = DebugGatherCompileInfo(old_source);
@@ -274,12 +226,13 @@
try {
new_compile_info = DebugGatherCompileInfo(new_source);
} catch (e) {
- throw "Failed to compile new version of script: " + e;
+ throw new liveedit.Failure("Failed to compile new version of script: "
+ e);
}
// An index of a single function, that is going to have its code
replaced.
var function_being_patched =
- FindChangedFunction(old_compile_info, change_pos, change_len_old);
+ FindChangedFunction(old_compile_info, change_pos, change_len_old);
+
// In old and new script versions function with a change should have the
// same indexes.
var function_being_patched2 =
@@ -290,7 +243,8 @@
// Check that function being patched has the same expectations in a new
// version. Otherwise we cannot safely patch its behavior and should
// choose the outer function instead.
- while
(!CompareFunctionExpectations(old_compile_info[function_being_patched],
+ while (!liveedit.CompareFunctionExpectations(
+ old_compile_info[function_being_patched],
new_compile_info[function_being_patched])) {
Assert(old_compile_info[function_being_patched].outer_index ==
@@ -300,21 +254,17 @@
Assert(function_being_patched != -1);
}
- // TODO: Need to check here that there are no activations of the function
- // being patched on stack.
+ // Check that function being patched is not currently on stack.
+ liveedit.CheckStackActivations(
+ [ FindFunctionInfo(function_being_patched) ], change_log );
+
// Committing all changes.
- var old_script_name = script.name + " (old)";
+ var old_script_name = liveedit.CreateNameForOldScript(script);
// Update the script text and create a new script representing an old
// version of the script.
var old_script = %LiveEditReplaceScript(script, new_source,
old_script_name);
-
- var shared_infos = new Array();
-
- for (var i = 0; i < shared_raw_list.length; i++) {
- shared_infos.push(new SharedInfoWrapper(shared_raw_list[i]));
- }
PatchCode(new_compile_info[function_being_patched],
FindFunctionInfo(function_being_patched));
@@ -364,3 +314,113 @@
}
}
+Debug.LiveEditChangeScript.Assert = function(condition, message) {
+ if (!condition) {
+ if (message) {
+ throw "Assert " + message;
+ } else {
+ throw "Assert";
+ }
+ }
+}
+
+// An object describing function compilation details. Its index fields
+// apply to indexes inside array that stores these objects.
+Debug.LiveEditChangeScript.FunctionCompileInfo = function(raw_array) {
+ this.function_name = raw_array[0];
+ this.start_position = raw_array[1];
+ this.end_position = raw_array[2];
+ this.param_num = raw_array[3];
+ this.code = raw_array[4];
+ this.scope_info = raw_array[5];
+ this.outer_index = raw_array[6];
+ this.next_sibling_index = null;
+ this.raw_array = raw_array;
+}
+
+// A structure describing SharedFunctionInfo.
+Debug.LiveEditChangeScript.SharedInfoWrapper = function(raw_array) {
+ this.function_name = raw_array[0];
+ this.start_position = raw_array[1];
+ this.end_position = raw_array[2];
+ this.info = raw_array[3];
+ this.raw_array = raw_array;
+}
+
+// Adds a suffix to script name to mark that it is old version.
+Debug.LiveEditChangeScript.CreateNameForOldScript = function(script) {
+ // TODO(635): try better than this; support several changes.
+ return script.name + " (old)";
+}
+
+// Compares a function interface old and new version, whether it
+// changed or not.
+Debug.LiveEditChangeScript.CompareFunctionExpectations =
+ function(function_info1, function_info2) {
+ // Check that function has the same number of parameters (there may exist
+ // an adapter, that won't survive function parameter number change).
+ if (function_info1.param_num != function_info2.param_num) {
+ return false;
+ }
+ var scope_info1 = function_info1.scope_info;
+ var scope_info2 = function_info2.scope_info;
+
+ if (!scope_info1) {
+ return !scope_info2;
+ }
+
+ if (scope_info1.length != scope_info2.length) {
+ return false;
+ }
+
+ // Check that outer scope structure is not changed. Otherwise the
function
+ // will not properly work with existing scopes.
+ return scope_info1.toString() == scope_info2.toString();
+}
+
+// For array of wrapped shared function infos checks that none of them
+// have activations on stack (of any thread). Throws a Failure exception
+// if this proves to be false.
+Debug.LiveEditChangeScript.CheckStackActivations =
function(shared_wrapper_list,
+ change_log) {
+ var liveedit = Debug.LiveEditChangeScript;
+
+ var shared_list = new Array();
+ for (var i = 0; i < shared_wrapper_list.length; i++) {
+ shared_list[i] = shared_wrapper_list[i].info;
+ }
+ var result = %LiveEditCheckStackActivations(shared_list);
+ var problems = new Array();
+ for (var i = 0; i < shared_list.length; i++) {
+ if (result[i] ==
liveedit.FunctionPatchabilityStatus.FUNCTION_BLOCKED_ON_STACK) {
+ var shared = shared_list[i];
+ var description = {
+ name: shared.function_name,
+ start_pos: shared.start_position,
+ end_pos: shared.end_position
+ };
+ problems.push(description);
+ }
+ }
+ if (problems.length > 0) {
+ change_log.push( { functions_on_stack: problems } );
+ throw new liveedit.Failure("Blocked by functions on stack");
+ }
+}
+
+// A copy of the FunctionPatchabilityStatus enum from liveedit.h
+Debug.LiveEditChangeScript.FunctionPatchabilityStatus = {
+ FUNCTION_AVAILABLE_FOR_PATCH: 0,
+ FUNCTION_BLOCKED_ON_STACK: 1
+}
+
+
+// A logical failure in liveedit process. This means that change_log
+// is valid and consistent description of what happened.
+Debug.LiveEditChangeScript.Failure = function(message) {
+ this.message = message;
+}
+
+Debug.LiveEditChangeScript.Failure.prototype.toString = function() {
+ return "LiveEdit Failure: " + this.message;
+}
=======================================
--- /branches/bleeding_edge/src/liveedit.cc Fri Mar 5 17:21:34 2010
+++ /branches/bleeding_edge/src/liveedit.cc Tue Mar 9 13:45:24 2010
@@ -320,9 +320,9 @@
CompilationZoneScope zone_scope(DELETE_ON_EXIT);
FunctionInfoListener listener;
+ Handle<Object> original_source = Handle<Object>(script->source());
script->set_source(*source);
active_function_info_listener = &listener;
- Handle<Object> original_source = Handle<Object>(script->source());
CompileScriptForTracker(script);
active_function_info_listener = NULL;
script->set_source(*original_source);
=======================================
--- /branches/bleeding_edge/src/liveedit.h Fri Mar 5 14:08:58 2010
+++ /branches/bleeding_edge/src/liveedit.h Tue Mar 9 13:45:24 2010
@@ -90,6 +90,12 @@
static void PatchFunctionPositions(Handle<JSArray> shared_info_array,
Handle<JSArray>
position_change_array);
+
+ // A copy of this is in liveedit-delay.js.
+ enum FunctionPatchabilityStatus {
+ FUNCTION_AVAILABLE_FOR_PATCH = 0,
+ FUNCTION_BLOCKED_ON_STACK = 1
+ };
};
#endif // ENABLE_DEBUGGER_SUPPORT
=======================================
--- /branches/bleeding_edge/src/runtime.cc Fri Mar 5 14:08:58 2010
+++ /branches/bleeding_edge/src/runtime.cc Tue Mar 9 13:45:24 2010
@@ -8441,6 +8441,44 @@
return Heap::undefined_value();
}
+
+
+static LiveEdit::FunctionPatchabilityStatus FindFunctionCodeOnStacks(
+ Handle<SharedFunctionInfo> shared) {
+ // TODO(635): check all threads, not only the current one.
+ for (StackFrameIterator it; !it.done(); it.Advance()) {
+ StackFrame* frame = it.frame();
+ if (frame->code() == shared->code()) {
+ return LiveEdit::FUNCTION_BLOCKED_ON_STACK;
+ }
+ }
+ return LiveEdit::FUNCTION_AVAILABLE_FOR_PATCH;
+}
+
+// For array of SharedFunctionInfo's (each wrapped in JSValue)
+// checks that none of them have activations on stacks (of any thread).
+// Returns array of the same length with corresponding results of
+// LiveEdit::FunctionPatchabilityStatus type.
+static Object* Runtime_LiveEditCheckStackActivations(Arguments args) {
+ ASSERT(args.length() == 1);
+ HandleScope scope;
+ CONVERT_ARG_CHECKED(JSArray, shared_array, 0);
+
+
+ int len = Smi::cast(shared_array->length())->value();
+ Handle<JSArray> result = Factory::NewJSArray(len);
+
+ for (int i = 0; i < len; i++) {
+ JSValue* wrapper = JSValue::cast(shared_array->GetElement(i));
+ Handle<SharedFunctionInfo> shared(
+ SharedFunctionInfo::cast(wrapper->value()));
+ LiveEdit::FunctionPatchabilityStatus check_res =
+ FindFunctionCodeOnStacks(shared);
+ SetElement(result, i, Handle<Smi>(Smi::FromInt(check_res)));
+ }
+
+ return *result;
+}
#endif // ENABLE_DEBUGGER_SUPPORT
=======================================
--- /branches/bleeding_edge/src/runtime.h Fri Mar 5 14:08:58 2010
+++ /branches/bleeding_edge/src/runtime.h Tue Mar 9 13:45:24 2010
@@ -331,7 +331,8 @@
F(LiveEditReplaceScript, 3, 1) \
F(LiveEditReplaceFunctionCode, 2, 1) \
F(LiveEditRelinkFunctionToScript, 2, 1) \
- F(LiveEditPatchFunctionPositions, 2, 1)
+ F(LiveEditPatchFunctionPositions, 2, 1) \
+ F(LiveEditCheckStackActivations, 1, 1)
#else
#define RUNTIME_FUNCTION_LIST_DEBUGGER_SUPPORT(F)
#endif
=======================================
--- /branches/bleeding_edge/test/mjsunit/fuzz-natives.js Fri Mar 5
17:21:34 2010
+++ /branches/bleeding_edge/test/mjsunit/fuzz-natives.js Tue Mar 9
13:45:24 2010
@@ -158,7 +158,8 @@
"LiveEditReplaceScript": true,
"LiveEditReplaceFunctionCode": true,
"LiveEditRelinkFunctionToScript": true,
- "LiveEditPatchFunctionPositions": true
+ "LiveEditPatchFunctionPositions": true,
+ "LiveEditCheckStackActivations": true
};
var currentlyUncallable = {
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
