Revision: 5702
Author: [email protected]
Date: Tue Oct 26 02:21:12 2010
Log: Port some GC fixes from the bleeding edge to the 2.4 branch. These
are:
5627 and 5628: Fix creation of an exception to avoid a rare GC corner case.
http://codereview.chromium.org/3782009
5640: Fix missing check for GC failure in number dictionary code.
http://codereview.chromium.org/3781014
5674: Fix the --noinline-new flag on ARM so that it forces us into C++ code
on every allocation. Fix three places where the generated code couldn't
cope with an unlucky GC.
http://codereview.chromium.org/3872003
5676: Fix GC error in ES5 read-only properties implementation.
http://codereview.chromium.org/3920005
Review URL: http://codereview.chromium.org/4174001
http://code.google.com/p/v8/source/detail?r=5702
Added:
/branches/2.4/test/mjsunit/define-property-gc.js
/branches/2.4/test/mjsunit/regress/regress-create-exception.js
Modified:
/branches/2.4/src/arm/codegen-arm.cc
/branches/2.4/src/arm/ic-arm.cc
/branches/2.4/src/arm/macro-assembler-arm.cc
/branches/2.4/src/handles.cc
/branches/2.4/src/handles.h
/branches/2.4/src/objects.cc
/branches/2.4/src/runtime.cc
/branches/2.4/src/version.cc
/branches/2.4/test/mjsunit/math-min-max.js
=======================================
--- /dev/null
+++ /branches/2.4/test/mjsunit/define-property-gc.js Tue Oct 26 02:21:12
2010
@@ -0,0 +1,45 @@
+// 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.
+
+// Tests the handling of GC issues in the defineProperty method.
+// Flags: --max-new-space-size=256
+
+function Regular() {
+ this[0] = 0;
+ this[1] = 1;
+}
+
+
+function foo() {
+ var descElementNonWritable = { value: 'foofoo', writable: false };
+ for (var i = 0; i < 1000; i++) {
+ var regular = new Regular();
+ Object.defineProperty(regular, '1', descElementNonWritable);
+ }
+}
+
+foo();
=======================================
--- /dev/null
+++ /branches/2.4/test/mjsunit/regress/regress-create-exception.js Tue Oct
26 02:21:12 2010
@@ -0,0 +1,58 @@
+// 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: --max-new-space-size=256
+
+// Check for GC bug constructing exceptions.
+var v = [1, 2, 3, 4]
+
+Object.preventExtensions(v);
+
+function foo() {
+ var re = /2147483647/; // Equal to 0x7fffffff.
+ for (var i = 0; i < 10000; i++) {
+ var ok = false;
+ try {
+ var j = 1;
+ // Allocate some heap numbers in order to randomize the behaviour of
the
+ // garbage collector. 93 is chosen to be a prime number to avoid the
+ // allocation settling into a too neat pattern.
+ for (var j = 0; j < i % 93; j++) {
+ j *= 1.123567; // An arbitrary floating point number.
+ }
+ v[0x7fffffff] = 0; // Trigger exception.
+ assertTrue(false);
+ return j; // Make sure that future optimizations don't eliminate j.
+ } catch(e) {
+ ok = true;
+ assertTrue(re.test(e));
+ }
+ assertTrue(ok);
+ }
+}
+
+foo();
=======================================
--- /branches/2.4/src/arm/codegen-arm.cc Wed Oct 13 01:35:23 2010
+++ /branches/2.4/src/arm/codegen-arm.cc Tue Oct 26 02:21:12 2010
@@ -1180,20 +1180,23 @@
void DeferredInlineSmiOperation::GenerateAnswerOutOfRange() {
// The input from a bitwise operation were Smis but the result cannot fit
- // into a Smi, so we store it into a heap number. tos_resgiter_ holds the
- // result to be converted.
+ // into a Smi, so we store it into a heap number.
VirtualFrame::scratch0()
+ // holds the untagged result to be converted. tos_register_ contains the
+ // input. See the calls to JumpToAnswerOutOfRange to see how we got
here.
ASSERT(Token::IsBitOp(op_));
ASSERT(!reversed_);
+ Register untagged_result = VirtualFrame::scratch0();
+
if (FLAG_debug_code) {
__ Abort("Should not fall through!");
}
__ bind(&answer_out_of_range_);
if (((value_ & 0x1f) == 0) && (op_ == Token::SHR)) {
- // >>> 0 is a special case where the result is already tagged but wrong
- // because the Smi is negative. We untag it.
- __ mov(tos_register_, Operand(tos_register_, ASR, kSmiTagSize));
+ // >>> 0 is a special case where the untagged_result register is not
set up
+ // yet. We untag the input to get it.
+ __ mov(untagged_result, Operand(tos_register_, ASR, kSmiTagSize));
}
// This routine uses the registers from r2 to r6. At the moment they are
@@ -1201,12 +1204,12 @@
// SpillAll and MergeTo like DeferredInlineSmiOperation::Generate()
above.
// Allocate the result heap number.
- Register heap_number_map = r7;
+ Register heap_number_map = VirtualFrame::scratch1();
Register heap_number = r4;
__ LoadRoot(heap_number_map, Heap::kHeapNumberMapRootIndex);
// If the allocation fails, fall back to the GenericBinaryOpStub.
__ AllocateHeapNumber(heap_number, r5, r6, heap_number_map,
entry_label());
- WriteNonSmiAnswer(tos_register_, heap_number, r3);
+ WriteNonSmiAnswer(untagged_result, heap_number, r3);
__ mov(tos_register_, Operand(heap_number));
Exit();
@@ -1474,25 +1477,29 @@
switch (op) {
case Token::SHL: {
if (shift_value != 0) {
- Register scratch = VirtualFrame::scratch0();
+ Register untagged_result = VirtualFrame::scratch0();
+ Register scratch = VirtualFrame::scratch1();
int adjusted_shift = shift_value - kSmiTagSize;
ASSERT(adjusted_shift >= 0);
if (adjusted_shift != 0) {
- __ mov(tos, Operand(tos, LSL, adjusted_shift));
+ __ mov(untagged_result, Operand(tos, LSL, adjusted_shift));
+ } else {
+ __ mov(untagged_result, Operand(tos));
}
// Check that the *signed* result fits in a smi.
- __ add(scratch, tos, Operand(0x40000000), SetCC);
+ __ add(scratch, untagged_result, Operand(0x40000000), SetCC);
deferred->JumpToAnswerOutOfRange(mi);
- __ mov(tos, Operand(tos, LSL, kSmiTagSize));
+ __ mov(tos, Operand(untagged_result, LSL, kSmiTagSize));
}
break;
}
case Token::SHR: {
if (shift_value != 0) {
- Register scratch = VirtualFrame::scratch0();
- __ mov(scratch, Operand(tos, ASR, kSmiTagSize)); // Remove
tag.
- __ mov(tos, Operand(scratch, LSR, shift_value));
+ Register untagged_result = VirtualFrame::scratch0();
+ // Remove tag.
+ __ mov(untagged_result, Operand(tos, ASR, kSmiTagSize));
+ __ mov(untagged_result, Operand(untagged_result, LSR,
shift_value));
if (shift_value == 1) {
// Check that the *unsigned* result fits in a smi.
// Neither of the two high-order bits can be set:
@@ -1501,17 +1508,10 @@
// tagging.
// These two cases can only happen with shifts by 0 or 1 when
// handed a valid smi.
- __ tst(tos, Operand(0xc0000000));
- if (!CpuFeatures::IsSupported(VFP3)) {
- // If the unsigned result does not fit in a Smi, we
require an
- // unsigned to double conversion. Without VFP V8 has to
fall
- // back to the runtime. The deferred code will expect tos
- // to hold the original Smi to be shifted.
- __ mov(tos, Operand(scratch, LSL, kSmiTagSize), LeaveCC,
ne);
- }
+ __ tst(untagged_result, Operand(0xc0000000));
deferred->JumpToAnswerOutOfRange(ne);
}
- __ mov(tos, Operand(tos, LSL, kSmiTagSize));
+ __ mov(tos, Operand(untagged_result, LSL, kSmiTagSize));
} else {
__ cmp(tos, Operand(0, RelocInfo::NONE));
deferred->JumpToAnswerOutOfRange(mi);
@@ -4733,6 +4733,7 @@
runtime.set_entry_frame(frame_);
Register heap_number_map = r6;
+ Register new_heap_number = r5;
__ LoadRoot(heap_number_map, Heap::kHeapNumberMapRootIndex);
// Get the double value from the heap number into vfp register d0.
@@ -4742,8 +4743,12 @@
// Calculate the square root of d0 and place result in a heap number
object.
__ vsqrt(d0, d0);
- __ AllocateHeapNumberWithValue(
- tos, d0, scratch1, scratch2, heap_number_map,
runtime.entry_label());
+ __ AllocateHeapNumberWithValue(new_heap_number,
+ d0,
+ scratch1, scratch2,
+ heap_number_map,
+ runtime.entry_label());
+ __ mov(tos, Operand(new_heap_number));
done.Jump();
runtime.Bind();
=======================================
--- /branches/2.4/src/arm/ic-arm.cc Mon Oct 4 01:54:21 2010
+++ /branches/2.4/src/arm/ic-arm.cc Tue Oct 26 02:21:12 2010
@@ -1410,9 +1410,12 @@
__ bind(&box_int);
// Allocate a HeapNumber for the result and perform int-to-double
- // conversion. Use r0 for result as key is not needed any more.
+ // conversion. Don't touch r0 or r1 as they are needed if allocation
+ // fails.
__ LoadRoot(r6, Heap::kHeapNumberMapRootIndex);
- __ AllocateHeapNumber(r0, r3, r4, r6, &slow);
+ __ AllocateHeapNumber(r5, r3, r4, r6, &slow);
+ // Now we can use r0 for the result as key is not needed any more.
+ __ mov(r0, r5);
if (CpuFeatures::IsSupported(VFP3)) {
CpuFeatures::Scope scope(VFP3);
=======================================
--- /branches/2.4/src/arm/macro-assembler-arm.cc Wed Sep 15 05:33:05 2010
+++ /branches/2.4/src/arm/macro-assembler-arm.cc Tue Oct 26 02:21:12 2010
@@ -908,6 +908,17 @@
Register scratch2,
Label* gc_required,
AllocationFlags flags) {
+ if (!FLAG_inline_new) {
+ if (FLAG_debug_code) {
+ // Trash the registers to simulate an allocation failure.
+ mov(result, Operand(0x7091));
+ mov(scratch1, Operand(0x7191));
+ mov(scratch2, Operand(0x7291));
+ }
+ jmp(gc_required);
+ return;
+ }
+
ASSERT(!result.is(scratch1));
ASSERT(!scratch1.is(scratch2));
@@ -959,6 +970,17 @@
Register scratch2,
Label* gc_required,
AllocationFlags flags) {
+ if (!FLAG_inline_new) {
+ if (FLAG_debug_code) {
+ // Trash the registers to simulate an allocation failure.
+ mov(result, Operand(0x7091));
+ mov(scratch1, Operand(0x7191));
+ mov(scratch2, Operand(0x7291));
+ }
+ jmp(gc_required);
+ return;
+ }
+
ASSERT(!result.is(scratch1));
ASSERT(!scratch1.is(scratch2));
=======================================
--- /branches/2.4/src/handles.cc Wed Oct 13 01:35:23 2010
+++ /branches/2.4/src/handles.cc Tue Oct 26 02:21:12 2010
@@ -208,6 +208,14 @@
CALL_HEAP_FUNCTION_VOID(
object->TransformToFastProperties(unused_property_fields));
}
+
+
+void NumberDictionarySet(Handle<NumberDictionary> dictionary,
+ uint32_t index,
+ Handle<Object> value,
+ PropertyDetails details) {
+ CALL_HEAP_FUNCTION_VOID(dictionary->Set(index, *value, details));
+}
void FlattenString(Handle<String> string) {
=======================================
--- /branches/2.4/src/handles.h Thu Sep 30 03:07:24 2010
+++ /branches/2.4/src/handles.h Tue Oct 26 02:21:12 2010
@@ -193,6 +193,10 @@
void NormalizeElements(Handle<JSObject> object);
void TransformToFastProperties(Handle<JSObject> object,
int unused_property_fields);
+void NumberDictionarySet(Handle<NumberDictionary> dictionary,
+ uint32_t index,
+ Handle<Object> value,
+ PropertyDetails details);
// Flattens a string.
void FlattenString(Handle<String> str);
=======================================
--- /branches/2.4/src/objects.cc Mon Oct 18 07:30:25 2010
+++ /branches/2.4/src/objects.cc Tue Oct 26 02:21:12 2010
@@ -6451,7 +6451,7 @@
// When we set the is_extensible flag to false we always force
// the element into dictionary mode (and force them to stay there).
if (!map()->is_extensible()) {
- Handle<Object> number(Heap::NumberFromUint32(index));
+ Handle<Object> number(Factory::NewNumberFromUint(index));
Handle<String> index_string(Factory::NumberToString(number));
Handle<Object> args[1] = { index_string };
return Top::Throw(*Factory::NewTypeError("object_not_extensible",
@@ -8565,7 +8565,9 @@
details = PropertyDetails(details.attributes(),
details.type(),
DetailsAt(entry).index());
- SetEntry(entry, NumberDictionaryShape::AsObject(key), value, details);
+ Object* object_key = NumberDictionaryShape::AsObject(key);
+ if (object_key->IsFailure()) return object_key;
+ SetEntry(entry, object_key, value, details);
return this;
}
=======================================
--- /branches/2.4/src/runtime.cc Mon Oct 18 07:30:25 2010
+++ /branches/2.4/src/runtime.cc Tue Oct 26 02:21:12 2010
@@ -3538,12 +3538,12 @@
if (((unchecked & (DONT_DELETE | DONT_ENUM | READ_ONLY)) != 0) &&
is_element) {
// Normalize the elements to enable attributes on the property.
- js_object->NormalizeElements();
- NumberDictionary* dictionary = js_object->element_dictionary();
+ NormalizeElements(js_object);
+ Handle<NumberDictionary> dictionary(js_object->element_dictionary());
// Make sure that we never go back to fast case.
dictionary->set_requires_slow_elements();
PropertyDetails details = PropertyDetails(attr, NORMAL);
- dictionary->Set(index, *obj_value, details);
+ NumberDictionarySet(dictionary, index, obj_value, details);
}
LookupResult result;
@@ -3557,7 +3557,7 @@
// new attributes.
if (result.IsProperty() && attr != result.GetAttributes()) {
// New attributes - normalize to avoid writing to instance descriptor
- js_object->NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
+ NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
// Use IgnoreAttributes version since a readonly property may be
// overridden and SetProperty does not allow this.
return js_object->IgnoreAttributesAndSetLocalProperty(*name,
@@ -4154,7 +4154,7 @@
Handle<Object> object = args.at<Object>(0);
if (object->IsJSObject()) {
Handle<JSObject> js_object = Handle<JSObject>::cast(object);
- js_object->NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
+ NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
}
return *object;
}
=======================================
--- /branches/2.4/src/version.cc Wed Oct 20 23:30:44 2010
+++ /branches/2.4/src/version.cc Tue Oct 26 02:21:12 2010
@@ -35,7 +35,7 @@
#define MAJOR_VERSION 2
#define MINOR_VERSION 4
#define BUILD_NUMBER 9
-#define PATCH_LEVEL 8
+#define PATCH_LEVEL 9
#define CANDIDATE_VERSION false
// Define SONAME to have the SCons build the put a specific SONAME into the
=======================================
--- /branches/2.4/test/mjsunit/math-min-max.js Wed Jul 7 05:22:46 2010
+++ /branches/2.4/test/mjsunit/math-min-max.js Tue Oct 26 02:21:12 2010
@@ -55,7 +55,10 @@
assertEquals(0, ZERO);
assertEquals(Infinity, 1/ZERO);
assertEquals(-Infinity, 1/-ZERO);
-assertFalse(%_IsSmi(ZERO));
+// Here we would like to have assertFalse(%_IsSmi(ZERO)); This is,
however,
+// unreliable, since a new space exhaustion at a critical moment could send
+// us into the runtime system, which would quite legitimately put a Smi
zero
+// here.
assertFalse(%_IsSmi(-ZERO));
var o = {};
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
