Skip to content

Commit 6dfa533

Browse files
committedJun 29, 2016
deps: backport IsValid changes from 4e8736d in V8
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 <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
1 parent 35ee1d1 commit 6dfa533

File tree

5 files changed

+10
-10
lines changed

5 files changed

+10
-10
lines changed
 

‎deps/v8/src/heap/incremental-marking.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ void IncrementalMarking::DeactivateIncrementalWriteBarrier() {
366366
DeactivateIncrementalWriteBarrierForSpace(heap_->new_space());
367367

368368
LargePage* lop = heap_->lo_space()->first_page();
369-
while (lop->is_valid()) {
369+
while (LargePage::IsValid(lop)) {
370370
SetOldSpacePageFlags(lop, false, false);
371371
lop = lop->next_page();
372372
}
@@ -398,7 +398,7 @@ void IncrementalMarking::ActivateIncrementalWriteBarrier() {
398398
ActivateIncrementalWriteBarrier(heap_->new_space());
399399

400400
LargePage* lop = heap_->lo_space()->first_page();
401-
while (lop->is_valid()) {
401+
while (LargePage::IsValid(lop)) {
402402
SetOldSpacePageFlags(lop, true, is_compacting_);
403403
lop = lop->next_page();
404404
}

‎deps/v8/src/heap/spaces-inl.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -325,14 +325,14 @@ void MemoryChunk::IncrementLiveBytesFromMutator(HeapObject* object, int by) {
325325

326326
bool PagedSpace::Contains(Address addr) {
327327
Page* p = Page::FromAddress(addr);
328-
if (!p->is_valid()) return false;
328+
if (!Page::IsValid(p)) return false;
329329
return p->owner() == this;
330330
}
331331

332332
bool PagedSpace::Contains(Object* o) {
333333
if (!o->IsHeapObject()) return false;
334334
Page* p = Page::FromAddress(HeapObject::cast(o)->address());
335-
if (!p->is_valid()) return false;
335+
if (!Page::IsValid(p)) return false;
336336
return p->owner() == this;
337337
}
338338

‎deps/v8/src/heap/spaces.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -2977,7 +2977,7 @@ LargePage* LargeObjectSpace::FindPage(Address a) {
29772977
if (e != NULL) {
29782978
DCHECK(e->value != NULL);
29792979
LargePage* page = reinterpret_cast<LargePage*>(e->value);
2980-
DCHECK(page->is_valid());
2980+
DCHECK(LargePage::IsValid(page));
29812981
if (page->Contains(a)) {
29822982
return page;
29832983
}

‎deps/v8/src/heap/spaces.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -558,9 +558,9 @@ class MemoryChunk {
558558
!chunk->high_water_mark_.TrySetValue(old_mark, new_mark));
559559
}
560560

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

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

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

‎deps/v8/test/cctest/heap/test-spaces.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ TEST(MemoryAllocator) {
322322
NOT_EXECUTABLE);
323323

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

@@ -334,7 +334,7 @@ TEST(MemoryAllocator) {
334334
Page* other = memory_allocator->AllocatePage<Page>(
335335
faked_space.AreaSize(), static_cast<PagedSpace*>(&faked_space),
336336
NOT_EXECUTABLE);
337-
CHECK(other->is_valid());
337+
CHECK(Page::IsValid(other));
338338
total_pages++;
339339
other->InsertAfter(first_page);
340340
int page_count = 0;
@@ -345,7 +345,7 @@ TEST(MemoryAllocator) {
345345
CHECK(total_pages == page_count);
346346

347347
Page* second_page = first_page->next_page();
348-
CHECK(second_page->is_valid());
348+
CHECK(Page::IsValid(second_page));
349349

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

0 commit comments

Comments
 (0)
Please sign in to comment.