Revision: 5703
Author: [email protected]
Date: Tue Oct 26 02:25:42 2010
Log: Port some GC fixes from the bleeding edge to the 2.3 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 two 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/4171002
http://code.google.com/p/v8/source/detail?r=5703
Added:
/branches/2.3/test/mjsunit/define-property-gc.js
/branches/2.3/test/mjsunit/regress/regress-create-exception.js
Modified:
/branches/2.3/src/arm/codegen-arm.cc
/branches/2.3/src/arm/ic-arm.cc
/branches/2.3/src/arm/macro-assembler-arm.cc
/branches/2.3/src/handles.cc
/branches/2.3/src/handles.h
/branches/2.3/src/objects.cc
/branches/2.3/src/runtime.cc
/branches/2.3/src/version.cc
/branches/2.3/test/mjsunit/math-min-max.js
=======================================
--- /dev/null
+++ /branches/2.3/test/mjsunit/define-property-gc.js Tue Oct 26 02:25:42
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=262144
+
+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.3/test/mjsunit/regress/regress-create-exception.js Tue Oct
26 02:25:42 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=262144
+
+// 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.3/src/arm/codegen-arm.cc Wed Aug 25 08:26:24 2010
+++ /branches/2.3/src/arm/codegen-arm.cc Tue Oct 26 02:25:42 2010
@@ -4536,6 +4536,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.
@@ -4545,8 +4546,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.3/src/arm/ic-arm.cc Wed Sep 29 09:05:05 2010
+++ /branches/2.3/src/arm/ic-arm.cc Tue Oct 26 02:25:42 2010
@@ -1427,9 +1427,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.3/src/arm/macro-assembler-arm.cc Mon Aug 30 23:45:16 2010
+++ /branches/2.3/src/arm/macro-assembler-arm.cc Tue Oct 26 02:25:42 2010
@@ -906,6 +906,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));
@@ -957,6 +968,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.3/src/handles.cc Mon Sep 27 08:03:50 2010
+++ /branches/2.3/src/handles.cc Tue Oct 26 02:25:42 2010
@@ -193,6 +193,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.3/src/handles.h Mon Aug 16 09:28:43 2010
+++ /branches/2.3/src/handles.h Tue Oct 26 02:25:42 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.3/src/objects.cc Mon Sep 27 08:03:50 2010
+++ /branches/2.3/src/objects.cc Tue Oct 26 02:25:42 2010
@@ -6320,7 +6320,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",
@@ -8434,7 +8434,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.3/src/runtime.cc Mon Sep 27 08:03:50 2010
+++ /branches/2.3/src/runtime.cc Tue Oct 26 02:25:42 2010
@@ -4148,12 +4148,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;
@@ -4167,7 +4167,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,
@@ -4764,7 +4764,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.3/src/version.cc Wed Oct 20 02:05:50 2010
+++ /branches/2.3/src/version.cc Tue Oct 26 02:25:42 2010
@@ -35,7 +35,7 @@
#define MAJOR_VERSION 2
#define MINOR_VERSION 3
#define BUILD_NUMBER 11
-#define PATCH_LEVEL 20
+#define PATCH_LEVEL 21
#define CANDIDATE_VERSION false
// Define SONAME to have the SCons build the put a specific SONAME into the
=======================================
--- /branches/2.3/test/mjsunit/math-min-max.js Wed Jul 7 05:22:46 2010
+++ /branches/2.3/test/mjsunit/math-min-max.js Tue Oct 26 02:25:42 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
