Skip to content

Commit

Permalink
deps: backport IsValid changes from 4e8736d in V8
Browse files Browse the repository at this point in the history
V8 erroneously did null pointer checks on `this`.
It can lead to a SIGSEGV crash if node is compiled with GCC 6.
Backport relevant changes from [1] that fix this issue.

[1]: https://codereview.chromium.org/1900423002

Fixes: nodejs#6272
PR-URL: nodejs#6544
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
  • Loading branch information
targos committed May 4, 2016
1 parent a2e5719 commit 96198d5
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 10 deletions.
4 changes: 2 additions & 2 deletions deps/v8/src/heap/incremental-marking.cc
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ void IncrementalMarking::DeactivateIncrementalWriteBarrier() {
DeactivateIncrementalWriteBarrierForSpace(heap_->new_space());

LargePage* lop = heap_->lo_space()->first_page();
while (lop->is_valid()) {
while (LargePage::IsValid(lop)) {
SetOldSpacePageFlags(lop, false, false);
lop = lop->next_page();
}
Expand Down Expand Up @@ -436,7 +436,7 @@ void IncrementalMarking::ActivateIncrementalWriteBarrier() {
ActivateIncrementalWriteBarrier(heap_->new_space());

LargePage* lop = heap_->lo_space()->first_page();
while (lop->is_valid()) {
while (LargePage::IsValid(lop)) {
SetOldSpacePageFlags(lop, true, is_compacting_);
lop = lop->next_page();
}
Expand Down
4 changes: 2 additions & 2 deletions deps/v8/src/heap/spaces-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -284,14 +284,14 @@ void MemoryChunk::IncrementLiveBytesFromMutator(HeapObject* object, int by) {

bool PagedSpace::Contains(Address addr) {
Page* p = Page::FromAddress(addr);
if (!p->is_valid()) return false;
if (!Page::IsValid(p)) return false;
return p->owner() == this;
}

bool PagedSpace::Contains(Object* o) {
if (!o->IsHeapObject()) return false;
Page* p = Page::FromAddress(HeapObject::cast(o)->address());
if (!p->is_valid()) return false;
if (!Page::IsValid(p)) return false;
return p->owner() == this;
}

Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/heap/spaces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3044,7 +3044,7 @@ LargePage* LargeObjectSpace::FindPage(Address a) {
if (e != NULL) {
DCHECK(e->value != NULL);
LargePage* page = reinterpret_cast<LargePage*>(e->value);
DCHECK(page->is_valid());
DCHECK(LargePage::IsValid(page));
if (page->Contains(a)) {
return page;
}
Expand Down
4 changes: 2 additions & 2 deletions deps/v8/src/heap/spaces.h
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,9 @@ class MemoryChunk {
!chunk->high_water_mark_.TrySetValue(old_mark, new_mark));
}

Address address() { return reinterpret_cast<Address>(this); }
static bool IsValid(MemoryChunk* chunk) { return chunk != nullptr; }

bool is_valid() { return address() != NULL; }
Address address() { return reinterpret_cast<Address>(this); }

base::Mutex* mutex() { return mutex_; }

Expand Down
6 changes: 3 additions & 3 deletions deps/v8/test/cctest/heap/test-spaces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ TEST(MemoryAllocator) {
faked_space.AreaSize(), &faked_space, NOT_EXECUTABLE);

first_page->InsertAfter(faked_space.anchor()->prev_page());
CHECK(first_page->is_valid());
CHECK(Page::IsValid(first_page));
CHECK(first_page->next_page() == faked_space.anchor());
total_pages++;

Expand All @@ -332,7 +332,7 @@ TEST(MemoryAllocator) {
// Again, we should get n or n - 1 pages.
Page* other = memory_allocator->AllocatePage(faked_space.AreaSize(),
&faked_space, NOT_EXECUTABLE);
CHECK(other->is_valid());
CHECK(Page::IsValid(other));
total_pages++;
other->InsertAfter(first_page);
int page_count = 0;
Expand All @@ -343,7 +343,7 @@ TEST(MemoryAllocator) {
CHECK(total_pages == page_count);

Page* second_page = first_page->next_page();
CHECK(second_page->is_valid());
CHECK(Page::IsValid(second_page));

// OldSpace's destructor will tear down the space and free up all pages.
}
Expand Down

0 comments on commit 96198d5

Please sign in to comment.