Reviewers: Mads Ager,
Message:
Mads,
may you have a look?
Any other spaces where object could span the page? LargeObject seems fine
as it
only provides Contains(HeapObject*) and thus unless some one fakes
HeapObject*,
there should be no problems imho.
Couple of notes:
1) probably new space check should have been moved into IsLargeObjectPage
itself, but it hard for me to tell the performance impact;
2) I keep commented CHECK in the test. The story is x64 gcc in release mode
seems to be 'very smart' about aliasing and mistakenly breaks the CHECK (the
rest of the test is fine).
Description:
Fix LargeObjectSpace::Contains to check if addr is in new space.
Otherwise page header check is not quite robust: if there is a smi
at the same offset as Page::is_normal_page field, wrong result would
be returned.
That shouldn't be the problem for paged spaces as objects in those
pages do not span page boundaries and thus cannot mess with ::is_normal_page
field.
Please review this at http://codereview.chromium.org/1175001
Affected files:
M src/spaces.cc
M test/cctest/test-heap.cc
Index: src/spaces.cc
diff --git a/src/spaces.cc b/src/spaces.cc
index
08399ee8ddb708296137bd9cae4b9bfaf426ef64..5f191ed3a55c003dca4785d3ea04216f3c18058e
100644
--- a/src/spaces.cc
+++ b/src/spaces.cc
@@ -2749,6 +2749,9 @@ void LargeObjectSpace::FreeUnmarkedObjects() {
bool LargeObjectSpace::Contains(HeapObject* object) {
Address address = object->address();
+ if (Heap::new_space()->Contains(address)) {
+ return false;
+ }
Page* page = Page::FromAddress(address);
SLOW_ASSERT(!page->IsLargeObjectPage()
Index: test/cctest/test-heap.cc
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index
45c516038d5826bb05668ab7bb15b6c1ceceae12..2b88f0f94e187280a7a0535fd26bea0f6aff4712
100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -809,3 +809,46 @@ TEST(Iteration) {
CHECK_EQ(objs_count, next_objs_index);
CHECK_EQ(objs_count, ObjectsFoundInHeap(objs, objs_count));
}
+
+
+TEST(LargeObjectSpaceContains) {
+ InitializeVM();
+
+ int free_bytes = Heap::MaxObjectSizeInPagedSpace();
+ CHECK(Heap::CollectGarbage(free_bytes, NEW_SPACE));
+
+ Address current_top = Heap::new_space()->top();
+ Page* page = Page::FromAddress(current_top);
+ Address current_page = page->address();
+ Address next_page = current_page + Page::kPageSize;
+ int bytes_to_page = next_page - current_top;
+ if (bytes_to_page <= FixedArray::kHeaderSize) {
+ // Alas, need to cross another page to be able to
+ // put desired value.
+ next_page += Page::kPageSize;
+ bytes_to_page = next_page - current_top;
+ }
+ CHECK(bytes_to_page > FixedArray::kHeaderSize);
+
+ int* is_normal_page_ptr = &Page::FromAddress(next_page)->is_normal_page;
+ Address is_normal_page_addr =
reinterpret_cast<Address>(is_normal_page_ptr);
+
+ int bytes_to_allocate = (is_normal_page_addr - current_top) +
kPointerSize;
+
+ int n_elements = (bytes_to_allocate - FixedArray::kHeaderSize) /
+ kPointerSize;
+ CHECK_EQ(bytes_to_allocate, FixedArray::SizeFor(n_elements));
+ FixedArray* array = FixedArray::cast(
+ Heap::AllocateFixedArray(n_elements));
+
+ int index = n_elements - 1;
+ CHECK_EQ(is_normal_page_ptr,
+ HeapObject::RawField(array,
FixedArray::OffsetOfElementAt(index)));
+ array->set(index, Smi::FromInt(0));
+ // This chould have turned next page into LargeObjectPage:
+ // CHECK(Page::FromAddress(next_page)->IsLargeObjectPage());
+
+ HeapObject* addr = HeapObject::FromAddress(next_page + 2 * kPointerSize);
+ CHECK(Heap::new_space()->Contains(addr));
+ CHECK(!Heap::lo_space()->Contains(addr));
+}
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply
to this email with the words "REMOVE ME" as the subject.
