Skip to content

Commit 739327f

Browse files
targosjoelostrowski
authored andcommitted
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 0b03189 commit 739327f

File tree

5 files changed

+10
-10
lines changed

5 files changed

+10
-10
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ void IncrementalMarking::DeactivateIncrementalWriteBarrier() {
404404
DeactivateIncrementalWriteBarrierForSpace(heap_->new_space());
405405

406406
LargePage* lop = heap_->lo_space()->first_page();
407-
while (lop->is_valid()) {
407+
while (LargePage::IsValid(lop)) {
408408
SetOldSpacePageFlags(lop, false, false);
409409
lop = lop->next_page();
410410
}
@@ -436,7 +436,7 @@ void IncrementalMarking::ActivateIncrementalWriteBarrier() {
436436
ActivateIncrementalWriteBarrier(heap_->new_space());
437437

438438
LargePage* lop = heap_->lo_space()->first_page();
439-
while (lop->is_valid()) {
439+
while (LargePage::IsValid(lop)) {
440440
SetOldSpacePageFlags(lop, true, is_compacting_);
441441
lop = lop->next_page();
442442
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,14 +284,14 @@ void MemoryChunk::IncrementLiveBytesFromMutator(HeapObject* object, int by) {
284284

285285
bool PagedSpace::Contains(Address addr) {
286286
Page* p = Page::FromAddress(addr);
287-
if (!p->is_valid()) return false;
287+
if (!Page::IsValid(p)) return false;
288288
return p->owner() == this;
289289
}
290290

291291
bool PagedSpace::Contains(Object* o) {
292292
if (!o->IsHeapObject()) return false;
293293
Page* p = Page::FromAddress(HeapObject::cast(o)->address());
294-
if (!p->is_valid()) return false;
294+
if (!Page::IsValid(p)) return false;
295295
return p->owner() == this;
296296
}
297297

deps/v8/src/heap/spaces.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3044,7 +3044,7 @@ LargePage* LargeObjectSpace::FindPage(Address a) {
30443044
if (e != NULL) {
30453045
DCHECK(e->value != NULL);
30463046
LargePage* page = reinterpret_cast<LargePage*>(e->value);
3047-
DCHECK(page->is_valid());
3047+
DCHECK(LargePage::IsValid(page));
30483048
if (page->Contains(a)) {
30493049
return page;
30503050
}

deps/v8/src/heap/spaces.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,9 +454,9 @@ class MemoryChunk {
454454
!chunk->high_water_mark_.TrySetValue(old_mark, new_mark));
455455
}
456456

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

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

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

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ TEST(MemoryAllocator) {
321321
faked_space.AreaSize(), &faked_space, NOT_EXECUTABLE);
322322

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

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

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

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

0 commit comments

Comments
 (0)