From 931584e5c96ed7ad629f6f0ee1b571619cb736e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Wed, 4 May 2016 10:07:14 +0200 Subject: [PATCH 1/2] 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: https://github.com/nodejs/node/issues/6272 --- deps/v8/src/heap/incremental-marking.cc | 4 ++-- deps/v8/src/heap/spaces-inl.h | 2 +- deps/v8/src/heap/spaces.cc | 2 +- deps/v8/src/heap/spaces.h | 4 ++-- deps/v8/test/cctest/test-spaces.cc | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/deps/v8/src/heap/incremental-marking.cc b/deps/v8/src/heap/incremental-marking.cc index 58eb0aa4097af8..b2b796f42a89a6 100644 --- a/deps/v8/src/heap/incremental-marking.cc +++ b/deps/v8/src/heap/incremental-marking.cc @@ -364,7 +364,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(); } @@ -396,7 +396,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(); } diff --git a/deps/v8/src/heap/spaces-inl.h b/deps/v8/src/heap/spaces-inl.h index c2c4d126976336..d63ee635acad5e 100644 --- a/deps/v8/src/heap/spaces-inl.h +++ b/deps/v8/src/heap/spaces-inl.h @@ -155,7 +155,7 @@ Page* Page::Initialize(Heap* heap, MemoryChunk* chunk, Executability executable, 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; } diff --git a/deps/v8/src/heap/spaces.cc b/deps/v8/src/heap/spaces.cc index 0806b2565da68f..c0e109b615f829 100644 --- a/deps/v8/src/heap/spaces.cc +++ b/deps/v8/src/heap/spaces.cc @@ -2953,7 +2953,7 @@ LargePage* LargeObjectSpace::FindPage(Address a) { if (e != NULL) { DCHECK(e->value != NULL); LargePage* page = reinterpret_cast(e->value); - DCHECK(page->is_valid()); + DCHECK(LargePage::IsValid(page)); if (page->Contains(a)) { return page; } diff --git a/deps/v8/src/heap/spaces.h b/deps/v8/src/heap/spaces.h index 3461de3ef009f1..e35c05757096e5 100644 --- a/deps/v8/src/heap/spaces.h +++ b/deps/v8/src/heap/spaces.h @@ -278,9 +278,9 @@ class MemoryChunk { // Only works for addresses in pointer spaces, not data or code spaces. static inline MemoryChunk* FromAnyPointerAddress(Heap* heap, Address addr); - Address address() { return reinterpret_cast
(this); } + static bool IsValid(MemoryChunk* chunk) { return chunk != nullptr; } - bool is_valid() { return address() != NULL; } + Address address() { return reinterpret_cast
(this); } MemoryChunk* next_chunk() const { return reinterpret_cast(base::Acquire_Load(&next_chunk_)); diff --git a/deps/v8/test/cctest/test-spaces.cc b/deps/v8/test/cctest/test-spaces.cc index 3f5e437223a57b..8ad9e869b3aa6e 100644 --- a/deps/v8/test/cctest/test-spaces.cc +++ b/deps/v8/test/cctest/test-spaces.cc @@ -314,7 +314,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++; @@ -325,7 +325,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; @@ -336,7 +336,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)); memory_allocator->Free(first_page); memory_allocator->Free(second_page); memory_allocator->TearDown(); From 8405d34c6eb2926c17182d39d59c5b772dc128d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Tue, 10 May 2016 09:33:06 +0200 Subject: [PATCH 2/2] deps: fix null pointer checks in V8's FrameStateDescriptor --- deps/v8/src/compiler/code-generator.cc | 9 ++++++--- deps/v8/src/compiler/instruction-selector-impl.h | 2 +- deps/v8/src/compiler/instruction-selector.cc | 3 ++- deps/v8/src/compiler/instruction.cc | 12 ++++++------ deps/v8/src/compiler/instruction.h | 7 ++++--- 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/deps/v8/src/compiler/code-generator.cc b/deps/v8/src/compiler/code-generator.cc index 2903c3d370dd82..16e6615961016c 100644 --- a/deps/v8/src/compiler/code-generator.cc +++ b/deps/v8/src/compiler/code-generator.cc @@ -524,7 +524,8 @@ void CodeGenerator::BuildTranslationForFrameStateDescriptor( translation, frame_state_offset, OutputFrameStateCombine::Ignore()); } - frame_state_offset += descriptor->outer_state()->GetTotalSize(); + frame_state_offset += + FrameStateDescriptor::GetTotalSize(descriptor->outer_state()); Handle shared_info; if (!descriptor->shared_info().ToHandle(&shared_info)) { @@ -562,8 +563,10 @@ int CodeGenerator::BuildTranslation(Instruction* instr, int pc_offset, frame_state_offset++; Translation translation( - &translations_, static_cast(descriptor->GetFrameCount()), - static_cast(descriptor->GetJSFrameCount()), zone()); + &translations_, + static_cast(FrameStateDescriptor::GetFrameCount(descriptor)), + static_cast(FrameStateDescriptor::GetJSFrameCount(descriptor)), + zone()); BuildTranslationForFrameStateDescriptor(descriptor, instr, &translation, frame_state_offset, state_combine); diff --git a/deps/v8/src/compiler/instruction-selector-impl.h b/deps/v8/src/compiler/instruction-selector-impl.h index b34a914efcca28..7c8b24d667ee8c 100644 --- a/deps/v8/src/compiler/instruction-selector-impl.h +++ b/deps/v8/src/compiler/instruction-selector-impl.h @@ -374,7 +374,7 @@ struct CallBuffer { size_t frame_state_value_count() const { return (frame_state_descriptor == NULL) ? 0 - : (frame_state_descriptor->GetTotalSize() + + : (FrameStateDescriptor::GetTotalSize(frame_state_descriptor) + 1); // Include deopt id. } }; diff --git a/deps/v8/src/compiler/instruction-selector.cc b/deps/v8/src/compiler/instruction-selector.cc index 813da4f132e2e9..38b12b581e53b8 100644 --- a/deps/v8/src/compiler/instruction-selector.cc +++ b/deps/v8/src/compiler/instruction-selector.cc @@ -1007,7 +1007,8 @@ void InstructionSelector::VisitDeoptimize(Node* value) { OperandGenerator g(this); FrameStateDescriptor* desc = GetFrameStateDescriptor(value); - size_t arg_count = desc->GetTotalSize() + 1; // Include deopt id. + size_t arg_count = + FrameStateDescriptor::GetTotalSize(desc) + 1; // Include deopt id. InstructionOperandVector args(instruction_zone()); args.reserve(arg_count); diff --git a/deps/v8/src/compiler/instruction.cc b/deps/v8/src/compiler/instruction.cc index 83b45b39ddec69..ee9b30dd1e2afc 100644 --- a/deps/v8/src/compiler/instruction.cc +++ b/deps/v8/src/compiler/instruction.cc @@ -697,9 +697,9 @@ size_t FrameStateDescriptor::GetSize(OutputFrameStateCombine combine) const { } -size_t FrameStateDescriptor::GetTotalSize() const { +size_t FrameStateDescriptor::GetTotalSize(const FrameStateDescriptor* desc) { size_t total_size = 0; - for (const FrameStateDescriptor* iter = this; iter != NULL; + for (const FrameStateDescriptor* iter = desc; iter != NULL; iter = iter->outer_state_) { total_size += iter->GetSize(); } @@ -707,9 +707,9 @@ size_t FrameStateDescriptor::GetTotalSize() const { } -size_t FrameStateDescriptor::GetFrameCount() const { +size_t FrameStateDescriptor::GetFrameCount(const FrameStateDescriptor* desc) { size_t count = 0; - for (const FrameStateDescriptor* iter = this; iter != NULL; + for (const FrameStateDescriptor* iter = desc; iter != NULL; iter = iter->outer_state_) { ++count; } @@ -717,9 +717,9 @@ size_t FrameStateDescriptor::GetFrameCount() const { } -size_t FrameStateDescriptor::GetJSFrameCount() const { +size_t FrameStateDescriptor::GetJSFrameCount(const FrameStateDescriptor* desc) { size_t count = 0; - for (const FrameStateDescriptor* iter = this; iter != NULL; + for (const FrameStateDescriptor* iter = desc; iter != NULL; iter = iter->outer_state_) { if (iter->type_ == FrameStateType::kJavaScriptFunction) { ++count; diff --git a/deps/v8/src/compiler/instruction.h b/deps/v8/src/compiler/instruction.h index a87ef7dc9c469f..b7c9430fe0ae4d 100644 --- a/deps/v8/src/compiler/instruction.h +++ b/deps/v8/src/compiler/instruction.h @@ -869,6 +869,10 @@ class FrameStateDescriptor : public ZoneObject { MaybeHandle shared_info, FrameStateDescriptor* outer_state = nullptr); + static size_t GetTotalSize(const FrameStateDescriptor* desc); + static size_t GetFrameCount(const FrameStateDescriptor* desc); + static size_t GetJSFrameCount(const FrameStateDescriptor* desc); + FrameStateType type() const { return type_; } BailoutId bailout_id() const { return bailout_id_; } OutputFrameStateCombine state_combine() const { return frame_state_combine_; } @@ -883,9 +887,6 @@ class FrameStateDescriptor : public ZoneObject { size_t GetSize(OutputFrameStateCombine combine = OutputFrameStateCombine::Ignore()) const; - size_t GetTotalSize() const; - size_t GetFrameCount() const; - size_t GetJSFrameCount() const; MachineType GetType(size_t index) const; void SetType(size_t index, MachineType type);