Reviewers: Hannes Payer,
Description:
Introduce safe interface to "copy and grow" FixedArray.
This introduces a CopyFixedArrayAndGrow method on Factory that takes
the "grow amount" instead of the "new size" as an argument. The new
interface is safer because it allows for mutations by the GC that
potentially trim the source array.
This also fixes as bug in SharedFunctionInfo::AddToOptimizedCodeMap
where the aformentioned scenario led to unused entries within the
optimized code map.
Note that FixedArray::CopySize is hereby deprecated because it is
considered unsafe and should no longer be used.
[email protected]
TEST=mjsunit/regress/regress-crbug-513507
BUG=chromium:513507
LOG=n
Please review this at https://codereview.chromium.org/1255173006/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+68, -10 lines):
M src/factory.h
M src/factory.cc
M src/heap/heap.h
M src/heap/heap.cc
M src/objects.h
M src/objects.cc
A test/mjsunit/regress/regress-crbug-513507.js
Index: src/factory.cc
diff --git a/src/factory.cc b/src/factory.cc
index
ea4f69a96b863957f6de9255e4f649e4096875f1..bc5d1e4e0ecbd424df06bcf248678116d5fcbfda
100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -978,6 +978,14 @@ Handle<FixedArray>
Factory::CopyFixedArrayWithMap(Handle<FixedArray> array,
}
+Handle<FixedArray> Factory::CopyFixedArrayAndGrow(Handle<FixedArray> array,
+ int grow_by) {
+ CALL_HEAP_FUNCTION(isolate(),
+ isolate()->heap()->CopyFixedArrayAndGrow(*array,
grow_by),
+ FixedArray);
+}
+
+
Handle<FixedArray> Factory::CopyFixedArray(Handle<FixedArray> array) {
CALL_HEAP_FUNCTION(isolate(),
isolate()->heap()->CopyFixedArray(*array),
Index: src/factory.h
diff --git a/src/factory.h b/src/factory.h
index
9a595c411566288711fccba9a1a8e68908609b18..c52d6d31d94c2543881198c93e5de18ccfc1dca7
100644
--- a/src/factory.h
+++ b/src/factory.h
@@ -322,6 +322,9 @@ class Factory final {
Handle<FixedArray> CopyFixedArrayWithMap(Handle<FixedArray> array,
Handle<Map> map);
+ Handle<FixedArray> CopyFixedArrayAndGrow(Handle<FixedArray> array,
+ int grow_by);
+
Handle<FixedArray> CopyFixedArray(Handle<FixedArray> array);
// This method expects a COW array in new space, and creates a copy
Index: src/heap/heap.cc
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index
0432ee71fa0c329e6c1bf6e690883ab26f211a75..03af4c4e59a935d6cba4ec7b96dc395ab9a9da93
100644
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -4426,7 +4426,7 @@ AllocationResult
Heap::CopyAndTenureFixedCOWArray(FixedArray* src) {
FixedArray* result = FixedArray::cast(obj);
result->set_length(len);
- // Copy the content
+ // Copy the content.
DisallowHeapAllocation no_gc;
WriteBarrierMode mode = result->GetWriteBarrierMode(no_gc);
for (int i = 0; i < len; i++) result->set(i, src->get(i), mode);
@@ -4445,6 +4445,27 @@ AllocationResult Heap::AllocateEmptyFixedTypedArray(
}
+AllocationResult Heap::CopyFixedArrayAndGrow(FixedArray* src, int grow_by)
{
+ int old_len = src->length();
+ int new_len = old_len + grow_by;
+ HeapObject* obj;
+ {
+ AllocationResult allocation = AllocateRawFixedArray(new_len,
NOT_TENURED);
+ if (!allocation.To(&obj)) return allocation;
+ }
+ obj->set_map_no_write_barrier(fixed_array_map());
+ FixedArray* result = FixedArray::cast(obj);
+ result->set_length(new_len);
+
+ // Copy the content.
+ DisallowHeapAllocation no_gc;
+ WriteBarrierMode mode = result->GetWriteBarrierMode(no_gc);
+ for (int i = 0; i < old_len; i++) result->set(i, src->get(i), mode);
+ MemsetPointer(result->data_start() + old_len, undefined_value(),
grow_by);
+ return result;
+}
+
+
AllocationResult Heap::CopyFixedArrayWithMap(FixedArray* src, Map* map) {
int len = src->length();
HeapObject* obj;
@@ -4462,7 +4483,7 @@ AllocationResult
Heap::CopyFixedArrayWithMap(FixedArray* src, Map* map) {
FixedArray* result = FixedArray::cast(obj);
result->set_length(len);
- // Copy the content
+ // Copy the content.
DisallowHeapAllocation no_gc;
WriteBarrierMode mode = result->GetWriteBarrierMode(no_gc);
for (int i = 0; i < len; i++) result->set(i, src->get(i), mode);
Index: src/heap/heap.h
diff --git a/src/heap/heap.h b/src/heap/heap.h
index
54dfc7ae1a26da7fb32d5f3922da73012a72102a..6d75879374cf72351a64d9da48716ccdcf27b39e
100644
--- a/src/heap/heap.h
+++ b/src/heap/heap.h
@@ -2019,17 +2019,18 @@ class Heap {
// Allocates an uninitialized fixed array. It must be filled by the
caller.
MUST_USE_RESULT AllocationResult AllocateUninitializedFixedArray(int
length);
- // Make a copy of src and return it. Returns
- // Failure::RetryAfterGC(requested_bytes, space) if the allocation
failed.
+ // Make a copy of src and return it.
MUST_USE_RESULT inline AllocationResult CopyFixedArray(FixedArray* src);
- // Make a copy of src, set the map, and return the copy. Returns
- // Failure::RetryAfterGC(requested_bytes, space) if the allocation
failed.
+ // Make a copy of src, also grow the copy, and return the copy.
+ MUST_USE_RESULT AllocationResult
+ CopyFixedArrayAndGrow(FixedArray* src, int grow_by);
+
+ // Make a copy of src, set the map, and return the copy.
MUST_USE_RESULT AllocationResult
CopyFixedArrayWithMap(FixedArray* src, Map* map);
- // Make a copy of src and return it. Returns
- // Failure::RetryAfterGC(requested_bytes, space) if the allocation
failed.
+ // Make a copy of src and return it.
MUST_USE_RESULT inline AllocationResult CopyFixedDoubleArray(
FixedDoubleArray* src);
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
51675d4c21597465a1e4fa790127b4e33d411d6f..f4a637c008612f9faaf858d14d00855c3865576e
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -9539,9 +9539,9 @@ void SharedFunctionInfo::AddToOptimizedCodeMap(
// Copy old map and append one new entry.
Handle<FixedArray> old_code_map = Handle<FixedArray>::cast(value);
DCHECK(!shared->SearchOptimizedCodeMap(*native_context,
osr_ast_id).code);
+ new_code_map =
+ isolate->factory()->CopyFixedArrayAndGrow(old_code_map,
kEntryLength);
old_length = old_code_map->length();
- new_code_map = FixedArray::CopySize(
- old_code_map, old_length + kEntryLength);
// Zap the old map for the sake of the heap verifier.
if (Heap::ShouldZapGarbage()) {
Object** data = old_code_map->data_start();
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
e80ad0ae3af1a12dcd7efe2337ccc00c1b007a7a..fb97f2804a732edf438b903c65332f48c869ef42
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -2399,6 +2399,7 @@ class FixedArray: public FixedArrayBase {
void Shrink(int length);
// Copy operation.
+ // TODO(mstarzinger): Deprecated, use Factory::CopyFixedArrayAndGrow!
static Handle<FixedArray> CopySize(Handle<FixedArray> array,
int new_length,
PretenureFlag pretenure =
NOT_TENURED);
Index: test/mjsunit/regress/regress-crbug-513507.js
diff --git a/test/mjsunit/regress/regress-crbug-513507.js
b/test/mjsunit/regress/regress-crbug-513507.js
new file mode 100644
index
0000000000000000000000000000000000000000..dbf35c91febd403577c03381c430a53a9014ae5b
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-513507.js
@@ -0,0 +1,24 @@
+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --noflush-optimized-code-cache --allow-natives-syntax
+
+// The following triggers a GC in
SharedFunctionInfo::AddToOptimizedCodeMap.
+// Flags: --gc-interval=1234 --gc-global
+
+function makeFun() {
+ function fun(osr_fuse) {
+ for (var i = 0; i < 3; ++i) {
+ if (i == osr_fuse) %OptimizeOsr();
+ }
+ for (var i = 3; i < 6; ++i) {
+ if (i == osr_fuse) %OptimizeOsr();
+ }
+ }
+ return fun;
+}
+
+makeFun()(7); // Warm up.
+makeFun()(4); // Optimize once.
+makeFun()(1); // Optimize again.
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
