Forgotten: changes to messages.cc are not mandatory, but I think it's a minor improvement over previous version.
yours, anton. On Tue, Feb 8, 2011 at 5:05 PM, <[email protected]> wrote: > Reviewers: Mads Ager, Lasse Reichstein, > > Message: > Lasse and Mads, > > may you have a look? > > And I'll send enough path which adds checks in CEntry stub what return value > and > Top::pending_exception are consistent on coming back to JS code. > > Description: > 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 > > Please review this at http://codereview.chromium.org/6451004/ > > SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge > > Affected files: > M src/handles.cc > M src/messages.cc > M src/objects.h > M src/objects.cc > M test/mjsunit/getter-in-prototype.js > A test/mjsunit/regress/regress-1107.js > > > Index: src/handles.cc > diff --git a/src/handles.cc b/src/handles.cc > index > 274c34ddebaf24283d1f0d1d21a5f9217dc469d2..795eda073c18e44244093ccc8a88785285d9015a > 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(); > Index: src/messages.cc > diff --git a/src/messages.cc b/src/messages.cc > index > 432364919bacd6a9c20b1e1a4b012e2be54e9617..990000a32ea84aa4d605562912afb3440c871354 > 100644 > --- a/src/messages.cc > +++ b/src/messages.cc > @@ -69,10 +69,13 @@ Handle<JSMessageObject> > MessageHandler::MakeMessageObject( > Handle<String> stack_trace, > Handle<JSArray> stack_frames) { > Handle<String> type_handle = Factory::LookupAsciiSymbol(type); > - Handle<JSArray> arguments_handle = Factory::NewJSArray(args.length()); > + Handle<FixedArray> 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<JSArray> arguments_handle = > + Factory::NewJSArrayWithElements(arguments_elements); > > int start = 0; > int end = 0; > @@ -87,7 +90,7 @@ Handle<JSMessageObject> MessageHandler::MakeMessageObject( > ? Factory::undefined_value() > : Handle<Object>::cast(stack_trace); > > - Handle<Object> stack_frames_handle = stack_frames.is_null() > + Handle<Object> stack_frames_handle = stack_frames.is_null() > ? Factory::undefined_value() > : Handle<Object>::cast(stack_frames); > > Index: src/objects.cc > diff --git a/src/objects.cc b/src/objects.cc > index > 775487a0a6be95205874632c74075da4fe6c7889..b92350c47ea49fe6130ca20c8fd60eb12e55c202 > 100644 > --- a/src/objects.cc > +++ b/src/objects.cc > @@ -1707,8 +1707,8 @@ void > JSObject::LookupCallbackSetterInPrototypes(String* name, > } > > > -bool JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index, > - Object* value) { > +MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t > index, > + Object* > value) { > for (Object* pt = GetPrototype(); > pt != Heap::null_value(); > pt = pt->GetPrototype()) { > @@ -1718,15 +1718,14 @@ 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; > + return SetElementWithCallback( > + dictionary->ValueAt(entry), index, value, JSObject::cast(pt)); > } > } > } > - return false; > + return Heap::the_hole_value(); > } > > > @@ -6962,9 +6961,10 @@ MaybeObject* JSObject::SetFastElement(uint32_t index, > uint32_t elms_length = static_cast<uint32_t>(elms->length()); > > if (check_prototype && > - (index >= elms_length || elms->get(index)->IsTheHole()) && > - SetElementWithCallbackSetterInPrototypes(index, value)) { > - return value; > + (index >= elms_length || elms->get(index)->IsTheHole())) { > + MaybeObject* result = > + SetElementWithCallbackSetterInPrototypes(index, value); > + if (result != Heap::the_hole_value()) return result; > } > > > @@ -7096,9 +7096,10 @@ 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) { > + MaybeObject* result = > + SetElementWithCallbackSetterInPrototypes(index, value); > + if (result != Heap::the_hole_value()) 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). > Index: src/objects.h > diff --git a/src/objects.h b/src/objects.h > index > bf598307a2f456dd618289aa0b431255f4e816ee..8087f7f673ffe7fdd4ff1e960329101dc486fec7 > 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); > void LookupCallback(String* name, LookupResult* result); > > // Returns the number of properties on this object filtering out > properties > Index: test/mjsunit/getter-in-prototype.js > diff --git a/test/mjsunit/getter-in-prototype.js > b/test/mjsunit/getter-in-prototype.js > index > dd26c533194c03b7703739d068ac095b48cb94c0..2c13573be402f5899779b4f0e2484622bb606fed > 100644 > --- a/test/mjsunit/getter-in-prototype.js > +++ b/test/mjsunit/getter-in-prototype.js > @@ -48,3 +48,20 @@ function g() { > x = 42; > } > assertThrows("g()"); > + > +p.__defineGetter__(0, function(){}); > + > +assertThrows("o[0] = 42"); > + > +function f2() { > + with(o) { > + this[0] = 42; > + } > +} > +assertThrows("f2()"); > + > +__proto__ = p; > +function g2() { > + this[0] = 42; > +} > +assertThrows("g2()"); > Index: test/mjsunit/regress/regress-1107.js > diff --git a/test/mjsunit/regress/regress-1107.js > b/test/mjsunit/regress/regress-1107.js > new file mode 100644 > index > 0000000000000000000000000000000000000000..80d2248d4f9f3c7a633e28e1bc8cb323d20df40b > --- /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.x"); > > > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
