From 0273e8185b9c8ec49c19e603ad7ba9c9c550ed2e Mon Sep 17 00:00:00 2001 From: "antonm@chromium.org" Date: Tue, 8 Feb 2011 19:42:14 +0000 Subject: [PATCH] Propagate exceptions thrown when setting elements. Plus use more robust path when formatting messages---work directly with fixed arrays. BUG=v8:1107 TEST=test/mjsunit/getter-in-prototype.js,test/mjsunit/regress/regress-1107.js Review URL: http://codereview.chromium.org/6451004 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6689 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/handles.cc | 1 + src/messages.cc | 9 +++++--- src/objects.cc | 30 +++++++++++++++----------- src/objects.h | 3 ++- test/mjsunit/getter-in-prototype.js | 8 +++++++ test/mjsunit/regress/regress-1107.js | 32 ++++++++++++++++++++++++++++ 6 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 test/mjsunit/regress/regress-1107.js diff --git a/src/handles.cc b/src/handles.cc index 274c34ddeb..795eda073c 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -808,6 +808,7 @@ static bool CompileLazyHelper(CompilationInfo* info, ClearExceptionFlag flag) { // Compile the source information to a code object. ASSERT(info->IsOptimizing() || !info->shared_info()->is_compiled()); + ASSERT(!Top::has_pending_exception()); bool result = Compiler::CompileLazy(info); ASSERT(result != Top::has_pending_exception()); if (!result && flag == CLEAR_EXCEPTION) Top::clear_pending_exception(); diff --git a/src/messages.cc b/src/messages.cc index 432364919b..990000a32e 100644 --- a/src/messages.cc +++ b/src/messages.cc @@ -69,10 +69,13 @@ Handle MessageHandler::MakeMessageObject( Handle stack_trace, Handle stack_frames) { Handle type_handle = Factory::LookupAsciiSymbol(type); - Handle arguments_handle = Factory::NewJSArray(args.length()); + Handle arguments_elements = + Factory::NewFixedArray(args.length()); for (int i = 0; i < args.length(); i++) { - SetElement(arguments_handle, i, args[i]); + arguments_elements->set(i, *args[i]); } + Handle arguments_handle = + Factory::NewJSArrayWithElements(arguments_elements); int start = 0; int end = 0; @@ -87,7 +90,7 @@ Handle MessageHandler::MakeMessageObject( ? Factory::undefined_value() : Handle::cast(stack_trace); - Handle stack_frames_handle = stack_frames.is_null() + Handle stack_frames_handle = stack_frames.is_null() ? Factory::undefined_value() : Handle::cast(stack_frames); diff --git a/src/objects.cc b/src/objects.cc index 6cfd9468da..a6e0dcd4c1 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1707,8 +1707,9 @@ void JSObject::LookupCallbackSetterInPrototypes(String* name, } -bool JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index, - Object* value) { +MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index, + Object* value, + bool* found) { for (Object* pt = GetPrototype(); pt != Heap::null_value(); pt = pt->GetPrototype()) { @@ -1718,15 +1719,16 @@ bool JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index, NumberDictionary* dictionary = JSObject::cast(pt)->element_dictionary(); int entry = dictionary->FindEntry(index); if (entry != NumberDictionary::kNotFound) { - Object* element = dictionary->ValueAt(entry); PropertyDetails details = dictionary->DetailsAt(entry); if (details.type() == CALLBACKS) { - SetElementWithCallback(element, index, value, JSObject::cast(pt)); - return true; + *found = true; + return SetElementWithCallback( + dictionary->ValueAt(entry), index, value, JSObject::cast(pt)); } } } - return false; + *found = false; + return Heap::the_hole_value(); } @@ -6969,9 +6971,11 @@ MaybeObject* JSObject::SetFastElement(uint32_t index, uint32_t elms_length = static_cast(elms->length()); if (check_prototype && - (index >= elms_length || elms->get(index)->IsTheHole()) && - SetElementWithCallbackSetterInPrototypes(index, value)) { - return value; + (index >= elms_length || elms->get(index)->IsTheHole())) { + bool found; + MaybeObject* result = + SetElementWithCallbackSetterInPrototypes(index, value, &found); + if (found) return result; } @@ -7103,9 +7107,11 @@ MaybeObject* JSObject::SetElementWithoutInterceptor(uint32_t index, } } else { // Index not already used. Look for an accessor in the prototype chain. - if (check_prototype && - SetElementWithCallbackSetterInPrototypes(index, value)) { - return value; + if (check_prototype) { + bool found; + MaybeObject* result = + SetElementWithCallbackSetterInPrototypes(index, value, &found); + if (found) return result; } // When we set the is_extensible flag to false we always force // the element into dictionary mode (and force them to stay there). diff --git a/src/objects.h b/src/objects.h index bf598307a2..71ef5069e6 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1579,7 +1579,8 @@ class JSObject: public HeapObject { void LookupRealNamedProperty(String* name, LookupResult* result); void LookupRealNamedPropertyInPrototypes(String* name, LookupResult* result); void LookupCallbackSetterInPrototypes(String* name, LookupResult* result); - bool SetElementWithCallbackSetterInPrototypes(uint32_t index, Object* value); + MUST_USE_RESULT MaybeObject* SetElementWithCallbackSetterInPrototypes( + uint32_t index, Object* value, bool* found); void LookupCallback(String* name, LookupResult* result); // Returns the number of properties on this object filtering out properties diff --git a/test/mjsunit/getter-in-prototype.js b/test/mjsunit/getter-in-prototype.js index dd26c53319..5563123e79 100644 --- a/test/mjsunit/getter-in-prototype.js +++ b/test/mjsunit/getter-in-prototype.js @@ -31,9 +31,11 @@ var o = {}; var p = {}; p.__defineGetter__('x', function(){}); +p.__defineGetter__(0, function(){}); o.__proto__ = p; assertThrows("o.x = 42"); +assertThrows("o[0] = 42"); function f() { with(o) { @@ -48,3 +50,9 @@ function g() { x = 42; } assertThrows("g()"); + +__proto__ = p; +function g2() { + this[0] = 42; +} +assertThrows("g2()"); diff --git a/test/mjsunit/regress/regress-1107.js b/test/mjsunit/regress/regress-1107.js new file mode 100644 index 0000000000..4ba277a2e1 --- /dev/null +++ b/test/mjsunit/regress/regress-1107.js @@ -0,0 +1,32 @@ +// Copyright 2011 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. + +// Test that even if we cannot set element 0 on all the objects, we still +// can format exception messages to some extent. + +Object.prototype.__defineGetter__(0, function(){}); +assertThrows("x");