From bb53dc689042d310b4d73367a32026159151268e Mon Sep 17 00:00:00 2001 From: "peter.rybin@gmail.com" Date: Tue, 13 Nov 2012 19:13:27 +0000 Subject: [PATCH] Issue 2368: LiveEdit crashes when new object/array literal is added Review URL: https://codereview.chromium.org/11191039 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12952 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/liveedit.cc | 139 +++++++++++++++++++++++++++++++- test/mjsunit/debug-liveedit-literals.js | 94 +++++++++++++++++++++ 2 files changed, 230 insertions(+), 3 deletions(-) create mode 100644 test/mjsunit/debug-liveedit-literals.js diff --git a/src/liveedit.cc b/src/liveedit.cc index dc7d4b1..574a376 100644 --- a/src/liveedit.cc +++ b/src/liveedit.cc @@ -703,12 +703,14 @@ class FunctionInfoWrapper : public JSArrayBasedStruct { : JSArrayBasedStruct(array) { } void SetInitialProperties(Handle 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 function_code, @@ -726,6 +728,9 @@ class FunctionInfoWrapper : public JSArrayBasedStruct { Handle 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 { 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; }; @@ -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,129 @@ static void ReplaceCodeObject(Handle original, } +// Patch function literals. +// Name 'literals' is a misnomer. Rather it's a cache for complex object +// boilerplates and for a native context. We must clean cached values. +// Additionally we may need to allocate a new array if number of literals +// changed. +class LiteralFixer { + public: + static void PatchLiterals(FunctionInfoWrapper* compile_info_wrapper, + Handle shared_info, + Isolate* isolate) { + int new_literal_count = compile_info_wrapper->GetLiteralCount(); + if (new_literal_count > 0) { + new_literal_count += JSFunction::kLiteralsPrefixSize; + } + int old_literal_count = shared_info->num_literals(); + + if (old_literal_count == new_literal_count) { + // If literal count didn't change, simply go over all functions + // and clear literal arrays. + ClearValuesVisitor visitor; + IterateJSFunctions(*shared_info, &visitor); + } else { + // When literal count changes, we have to create new array instances. + // Since we cannot create instances when iterating heap, we should first + // collect all functions and fix their literal arrays. + Handle function_instances = + CollectJSFunctions(shared_info, isolate); + for (int i = 0; i < function_instances->length(); i++) { + Handle fun(JSFunction::cast(function_instances->get(i))); + Handle old_literals(fun->literals()); + Handle new_literals = + isolate->factory()->NewFixedArray(new_literal_count); + if (new_literal_count > 0) { + Handle native_context; + if (old_literals->length() > + JSFunction::kLiteralNativeContextIndex) { + native_context = Handle( + JSFunction::NativeContextFromLiterals(fun->literals())); + } else { + native_context = Handle(fun->context()->native_context()); + } + new_literals->set(JSFunction::kLiteralNativeContextIndex, + *native_context); + } + fun->set_literals(*new_literals); + } + + shared_info->set_num_literals(new_literal_count); + } + } + + private: + // Iterates all function instances in the HEAP that refers to the + // provided shared_info. + template + static void IterateJSFunctions(SharedFunctionInfo* shared_info, + Visitor* visitor) { + AssertNoAllocation no_allocations_please; + + 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) { + visitor->visit(function); + } + } + } + } + + // Finds all instances of JSFunction that refers to the provided shared_info + // and returns array with them. + static Handle CollectJSFunctions( + Handle shared_info, Isolate* isolate) { + CountVisitor count_visitor; + count_visitor.count = 0; + IterateJSFunctions(*shared_info, &count_visitor); + int size = count_visitor.count; + + Handle result = isolate->factory()->NewFixedArray(size); + if (size > 0) { + CollectVisitor collect_visitor(result); + IterateJSFunctions(*shared_info, &collect_visitor); + } + return result; + } + + class ClearValuesVisitor { + public: + void visit(JSFunction* fun) { + FixedArray* literals = fun->literals(); + int len = literals->length(); + for (int j = JSFunction::kLiteralsPrefixSize; j < len; j++) { + literals->set_undefined(j); + } + } + }; + + class CountVisitor { + public: + void visit(JSFunction* fun) { + count++; + } + int count; + }; + + class CollectVisitor { + public: + explicit CollectVisitor(Handle output) + : m_output(output), m_pos(0) {} + + void visit(JSFunction* fun) { + m_output->set(m_pos, fun); + m_pos++; + } + private: + Handle m_output; + int m_pos; + }; +}; + + // Check whether the code is natural function code (not a lazy-compile stub // code). static bool IsJSFunctionCode(Code* code) { @@ -1080,9 +1210,10 @@ MaybeObject* LiveEdit::ReplaceFunctionCode( Handle new_compile_info_array, Handle 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 +1244,8 @@ MaybeObject* LiveEdit::ReplaceFunctionCode( shared_info->set_start_position(start_position); shared_info->set_end_position(end_position); + LiteralFixer::PatchLiterals(&compile_info_wrapper, shared_info, isolate); + shared_info->set_construct_stub( Isolate::Current()->builtins()->builtin( Builtins::kJSConstructStubGeneric)); diff --git a/test/mjsunit/debug-liveedit-literals.js b/test/mjsunit/debug-liveedit-literals.js new file mode 100644 index 0000000..5f9217e --- /dev/null +++ b/test/mjsunit/debug-liveedit-literals.js @@ -0,0 +1,94 @@ +// 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 + +function Test(old_expression, new_expression) { + // Generate several instances of function to test that we correctly fix + // all functions in memory. + var function_instance_number = 11; + eval("var t1 =1;\n" + + "ChooseAnimalArray = [];\n" + + "for (var i = 0; i < function_instance_number; i++) {\n" + + " ChooseAnimalArray.push(\n" + + " function ChooseAnimal() {\n" + + " return " + old_expression + ";\n" + + " });\n" + + "}\n" + + "var t2 =1;\n"); + + for (var i = 0; i < ChooseAnimalArray.length; i++) { + assertEquals("Cat", ChooseAnimalArray[i]()); + } + + var script = Debug.findScript(ChooseAnimalArray[0]); + + var patch_pos = script.source.indexOf(old_expression); + var new_animal_patch = new_expression; + + var change_log = new Array(); + Debug.LiveEdit.TestApi.ApplySingleChunkPatch(script, patch_pos, + old_expression.length, new_expression, change_log); + + for (var i = 0; i < ChooseAnimalArray.length; i++) { + assertEquals("Capybara", ChooseAnimalArray[i]()); + } +} + +// Check that old literal boilerplate was reset. +Test("['Cat'][0]", "['Capybara'][0]"); +Test("['Cat'][0]", "{a:'Capybara'}.a"); + +// No literals -> 1 literal. +Test("'Cat'", "['Capybara'][0]"); + +// No literals -> 2 literals. +Test("'Cat'", "['Capy'][0] + {a:'bara'}.a"); + +// 1 literal -> no literals. +Test("['Cat'][0]", "'Capybara'"); + +// 2 literals -> no literals. +Test("['Ca'][0] + {a:'t'}.a", "'Capybara'"); + +// No literals -> regexp. +Test("'Cat'", "(/.A.Y.A.A/i).exec('Capybara')[0]"); + +// Array literal -> regexp. +Test("['Cat'][0]", "(/.A.Y.A.A/i).exec('Capybara')[0]"); + +// Regexp -> object literal. +Test("(/.A./i).exec('Cat')[0]", "{c:'Capybara'}.c"); + +// No literals -> regexp. +Test("'Cat'", "(/.A.Y.A.A/i).exec('Capybara')[0]"); + +// Regexp -> no literals. +Test("(/.A./i).exec('Cat')[0]", "'Capybara'"); -- 2.7.4