Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(319)

Unified Diff: src/liveedit.cc

Issue 11191039: Issue 2368: LiveEdit crashes when new object/array literal is added (Closed) Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Patch Set: style Created 8 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/mjsunit/debug-liveedit-literals.js » ('j') | test/mjsunit/debug-liveedit-literals.js » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/liveedit.cc
diff --git a/src/liveedit.cc b/src/liveedit.cc
index 2a3aafc1f1daa355d817f3d0031315844a4d748f..cc5c28dd667f1a952510d9a20ed969c013c16190 100644
--- a/src/liveedit.cc
+++ b/src/liveedit.cc
@@ -703,12 +703,14 @@ class FunctionInfoWrapper : public JSArrayBasedStruct<FunctionInfoWrapper> {
: JSArrayBasedStruct<FunctionInfoWrapper>(array) {
}
void SetInitialProperties(Handle<String> name, int start_position,
- int end_position, int param_num, int parent_index) {
+ int end_position, int param_num,
+ int literal_count, int parent_index) {
HandleScope scope;
this->SetField(kFunctionNameOffset_, name);
this->SetSmiValueField(kStartPositionOffset_, start_position);
this->SetSmiValueField(kEndPositionOffset_, end_position);
this->SetSmiValueField(kParamNumOffset_, param_num);
+ this->SetSmiValueField(kLiteralNumOffset_, literal_count);
this->SetSmiValueField(kParentIndexOffset_, parent_index);
}
void SetFunctionCode(Handle<Code> function_code,
@@ -726,6 +728,9 @@ class FunctionInfoWrapper : public JSArrayBasedStruct<FunctionInfoWrapper> {
Handle<JSValue> info_holder = WrapInJSValue(info);
this->SetField(kSharedFunctionInfoOffset_, info_holder);
}
+ int GetLiteralCount() {
+ return this->GetSmiValueField(kLiteralNumOffset_);
+ }
int GetParentIndex() {
return this->GetSmiValueField(kParentIndexOffset_);
}
@@ -759,7 +764,8 @@ class FunctionInfoWrapper : public JSArrayBasedStruct<FunctionInfoWrapper> {
static const int kOuterScopeInfoOffset_ = 6;
static const int kParentIndexOffset_ = 7;
static const int kSharedFunctionInfoOffset_ = 8;
- static const int kSize_ = 9;
+ static const int kLiteralNumOffset_ = 9;
+ static const int kSize_ = 10;
friend class JSArrayBasedStruct<FunctionInfoWrapper>;
};
@@ -819,6 +825,7 @@ class FunctionInfoListener {
FunctionInfoWrapper info = FunctionInfoWrapper::Create();
info.SetInitialProperties(fun->name(), fun->start_position(),
fun->end_position(), fun->parameter_count(),
+ fun->materialized_literal_count(),
current_parent_index_);
current_parent_index_ = len_;
SetElementNonStrict(result_, len_, info.GetJSArray());
@@ -1014,6 +1021,44 @@ static void ReplaceCodeObject(Handle<Code> original,
}
+// Finds all instances of JSFunction that refers to the provided shared_info.
+// Return their number and optionally writes them into output array.
+static int FindJSFunctions(Handle<SharedFunctionInfo> shared_info,
+ Handle<FixedArray> output) {
+ int pos = 0;
+
+ AssertNoAllocation no_allocations_please;
+
+ SharedFunctionInfo* shared_info_raw = *shared_info;
+ HeapIterator iterator;
+ for (HeapObject* obj = iterator.next(); obj != NULL; obj = iterator.next()) {
+ if (obj->IsJSFunction()) {
+ JSFunction* function = JSFunction::cast(obj);
+ if (function->shared() == shared_info_raw) {
+ if (!output.is_null()) {
+ output->set(pos, function);
+ }
+ pos++;
+ }
+ }
+ }
+ return pos;
+}
+
+
+// Finds all instances of JSFunction that refers to the provided shared_info
+// and returns array with them.
+static Handle<FixedArray> CollectJSFunctions(
+ Handle<SharedFunctionInfo> shared_info, Isolate* isolate) {
+ int size = FindJSFunctions(shared_info, Handle<FixedArray>::null());
+ Handle<FixedArray> result = isolate->factory()->NewFixedArray(size);
+ if (size > 0) {
+ FindJSFunctions(shared_info, result);
Yang 2012/10/23 16:10:31 Running twice over the heap seems really inefficie
Peter Rybin 2012/11/11 03:28:47 In fact I only copy how Runtime_DebugReferencedBy
+ }
+ return result;
+}
+
+
// Check whether the code is natural function code (not a lazy-compile stub
// code).
static bool IsJSFunctionCode(Code* code) {
@@ -1080,9 +1125,10 @@ MaybeObject* LiveEdit::ReplaceFunctionCode(
Handle<JSArray> new_compile_info_array,
Handle<JSArray> shared_info_array) {
HandleScope scope;
+ Isolate* isolate = Isolate::Current();
if (!SharedInfoWrapper::IsInstance(shared_info_array)) {
- return Isolate::Current()->ThrowIllegalOperation();
+ return isolate->ThrowIllegalOperation();
}
FunctionInfoWrapper compile_info_wrapper(new_compile_info_array);
@@ -1113,6 +1159,48 @@ MaybeObject* LiveEdit::ReplaceFunctionCode(
shared_info->set_start_position(start_position);
shared_info->set_end_position(end_position);
+ // Patch function literals.
+ // TODO(prybin): is the following legit?
+ // Name 'literals' is a misnomer. Rather it's a cache for complex object
+ // boilerplates and for a native context. We must clean cached values or
+ // adjust array size.
Yang 2012/10/23 16:10:31 Wouldn't adjusting array size imply cleaning cache
Peter Rybin 2012/11/11 03:28:47 Done.
+ {
+ int new_literal_count = compile_info_wrapper.GetLiteralCount();
+ if (new_literal_count > 0) {
+ new_literal_count += JSFunction::kLiteralsPrefixSize;
+ }
+ shared_info->set_num_literals(new_literal_count);
+
+ Handle<FixedArray> function_instances =
+ CollectJSFunctions(shared_info, isolate);
+ for (int i = 0; i < function_instances->length(); i++) {
Yang 2012/10/23 16:10:31 Instead of looping over function instances that yo
Peter Rybin 2012/11/11 03:28:47 I have to update literal array, and possibly creat
+ Handle<JSFunction> fun(JSFunction::cast(function_instances->get(i)));
+ Handle<FixedArray> old_literals(fun->literals());
+ if (old_literals->length() == new_literal_count) {
+ for (int j = JSFunction::kLiteralsPrefixSize;
+ j < new_literal_count; j++) {
+ old_literals->set(j, isolate->heap()->undefined_value());
Yang 2012/10/23 16:10:31 I guess you could use FixedArray->set_undefined he
Peter Rybin 2012/11/11 03:28:47 Done.
+ }
+ } else {
+ Handle<FixedArray> new_literals =
+ isolate->factory()->NewFixedArray(new_literal_count);
+ if (new_literal_count > 0) {
+ Handle<Context> native_context;
+ if (old_literals->length() >
+ JSFunction::kLiteralNativeContextIndex) {
+ native_context = Handle<Context>(
+ JSFunction::NativeContextFromLiterals(fun->literals()));
+ } else {
+ native_context = Handle<Context>(fun->context()->native_context());
+ }
+ new_literals->set(JSFunction::kLiteralNativeContextIndex,
+ *native_context);
+ }
+ fun->set_literals(*new_literals);
+ }
Yang 2012/10/23 16:10:31 I assume that we are not dealing with optimized co
Peter Rybin 2012/11/11 03:28:47 May be it more safe to always create an accurate l
+ }
+ }
+
shared_info->set_construct_stub(
Isolate::Current()->builtins()->builtin(
Builtins::kJSConstructStubGeneric));
« no previous file with comments | « no previous file | test/mjsunit/debug-liveedit-literals.js » ('j') | test/mjsunit/debug-liveedit-literals.js » ('J')

Powered by Google App Engine
This is Rietveld 408576698