From 99d9abaf82aff155a62a1ebb03c6a12920554d91 Mon Sep 17 00:00:00 2001 From: "dcarney@chromium.org" Date: Thu, 12 Jun 2014 12:01:01 +0000 Subject: [PATCH] remove this == null R=danno@chromium.org BUG=chromium:381910 Review URL: https://codereview.chromium.org/336483002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21807 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/api.cc | 24 ++++++++++++------------ src/factory.cc | 3 ++- src/heap.cc | 15 +++++++++------ src/hydrogen-load-elimination.cc | 5 ++--- src/spaces.cc | 30 ++++++++++++++++++++---------- src/spaces.h | 6 +++--- src/x64/assembler-x64-inl.h | 2 -- test/cctest/test-spaces.cc | 2 +- 8 files changed, 49 insertions(+), 38 deletions(-) diff --git a/src/api.cc b/src/api.cc index 0c97851..2a86a04 100644 --- a/src/api.cc +++ b/src/api.cc @@ -1192,14 +1192,14 @@ static i::Handle MakeAccessorInfo( Local FunctionTemplate::InstanceTemplate() { - i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate(); - if (!Utils::ApiCheck(this != NULL, + i::Handle handle = Utils::OpenHandle(this, true); + if (!Utils::ApiCheck(!handle.is_null(), "v8::FunctionTemplate::InstanceTemplate()", "Reading from empty handle")) { return Local(); } + i::Isolate* isolate = handle->GetIsolate(); ENTER_V8(isolate); - i::Handle handle = Utils::OpenHandle(this); if (handle->instance_template()->IsUndefined()) { Local templ = ObjectTemplate::New(isolate, ToApiHandle(handle)); @@ -1616,11 +1616,11 @@ Handle UnboundScript::GetScriptName() { Local Script::Run() { + i::Handle obj = + i::Handle::cast(Utils::OpenHandle(this, true)); // If execution is terminating, Compile(..)->Run() requires this // check. - if (this == NULL) return Local(); - i::Handle obj = - i::Handle::cast(Utils::OpenHandle(this)); + if (obj.is_null()) return Local(); i::Isolate* isolate = obj->GetIsolate(); ON_BAILOUT(isolate, "v8::Script::Run()", return Local()); LOG_API(isolate, "Script::Run"); @@ -2912,14 +2912,14 @@ int32_t Value::Int32Value() const { bool Value::Equals(Handle that) const { i::Isolate* isolate = i::Isolate::Current(); - if (!Utils::ApiCheck(this != NULL && !that.IsEmpty(), + i::Handle obj = Utils::OpenHandle(this, true); + if (!Utils::ApiCheck(!obj.is_null() && !that.IsEmpty(), "v8::Value::Equals()", "Reading from empty handle")) { return false; } LOG_API(isolate, "Equals"); ENTER_V8(isolate); - i::Handle obj = Utils::OpenHandle(this); i::Handle other = Utils::OpenHandle(*that); // If both obj and other are JSObjects, we'd better compare by identity // immediately when going into JS builtin. The reason is Invoke @@ -2939,13 +2939,13 @@ bool Value::Equals(Handle that) const { bool Value::StrictEquals(Handle that) const { i::Isolate* isolate = i::Isolate::Current(); - if (!Utils::ApiCheck(this != NULL && !that.IsEmpty(), + i::Handle obj = Utils::OpenHandle(this, true); + if (!Utils::ApiCheck(!obj.is_null() && !that.IsEmpty(), "v8::Value::StrictEquals()", "Reading from empty handle")) { return false; } LOG_API(isolate, "StrictEquals"); - i::Handle obj = Utils::OpenHandle(this); i::Handle other = Utils::OpenHandle(*that); // Must check HeapNumber first, since NaN !== NaN. if (obj->IsHeapNumber()) { @@ -2971,12 +2971,12 @@ bool Value::StrictEquals(Handle that) const { bool Value::SameValue(Handle that) const { - if (!Utils::ApiCheck(this != NULL && !that.IsEmpty(), + i::Handle obj = Utils::OpenHandle(this, true); + if (!Utils::ApiCheck(!obj.is_null() && !that.IsEmpty(), "v8::Value::SameValue()", "Reading from empty handle")) { return false; } - i::Handle obj = Utils::OpenHandle(this); i::Handle other = Utils::OpenHandle(*that); return obj->SameValue(*other); } diff --git a/src/factory.cc b/src/factory.cc index 33d856f..1b91cf0 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -1411,7 +1411,8 @@ Handle Factory::NewCode(const CodeDesc& desc, int obj_size = Code::SizeFor(body_size); Handle code = NewCodeRaw(obj_size, immovable); - ASSERT(!isolate()->code_range()->exists() || + ASSERT(isolate()->code_range() == NULL || + !isolate()->code_range()->valid() || isolate()->code_range()->contains(code->address())); // The code object has not been fully initialized yet. We rely on the diff --git a/src/heap.cc b/src/heap.cc index 9e28d01..21a5094 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -3365,8 +3365,9 @@ AllocationResult Heap::AllocateCode(int object_size, result->set_map_no_write_barrier(code_map()); Code* code = Code::cast(result); - ASSERT(!isolate_->code_range()->exists() || - isolate_->code_range()->contains(code->address())); + ASSERT(isolate_->code_range() == NULL || + !isolate_->code_range()->valid() || + isolate_->code_range()->contains(code->address())); code->set_gc_metadata(Smi::FromInt(0)); code->set_ic_age(global_ic_age_); return code; @@ -3407,8 +3408,9 @@ AllocationResult Heap::CopyCode(Code* code) { new_code->set_constant_pool(new_constant_pool); // Relocate the copy. - ASSERT(!isolate_->code_range()->exists() || - isolate_->code_range()->contains(code->address())); + ASSERT(isolate_->code_range() == NULL || + !isolate_->code_range()->valid() || + isolate_->code_range()->contains(code->address())); new_code->Relocate(new_addr - old_addr); return new_code; } @@ -3471,8 +3473,9 @@ AllocationResult Heap::CopyCode(Code* code, Vector reloc_info) { static_cast(reloc_info.length())); // Relocate the copy. - ASSERT(!isolate_->code_range()->exists() || - isolate_->code_range()->contains(code->address())); + ASSERT(isolate_->code_range() == NULL || + !isolate_->code_range()->valid() || + isolate_->code_range()->contains(code->address())); new_code->Relocate(new_addr - old_addr); #ifdef VERIFY_HEAP diff --git a/src/hydrogen-load-elimination.cc b/src/hydrogen-load-elimination.cc index bc67078..5cefcf7 100644 --- a/src/hydrogen-load-elimination.cc +++ b/src/hydrogen-load-elimination.cc @@ -25,11 +25,10 @@ class HFieldApproximation : public ZoneObject { // Recursively copy the entire linked list of field approximations. HFieldApproximation* Copy(Zone* zone) { - if (this == NULL) return NULL; HFieldApproximation* copy = new(zone) HFieldApproximation(); copy->object_ = this->object_; copy->last_value_ = this->last_value_; - copy->next_ = this->next_->Copy(zone); + copy->next_ = this->next_ == NULL ? NULL : this->next_->Copy(zone); return copy; } }; @@ -148,7 +147,7 @@ class HLoadEliminationTable : public ZoneObject { new(zone) HLoadEliminationTable(zone, aliasing_); copy->EnsureFields(fields_.length()); for (int i = 0; i < fields_.length(); i++) { - copy->fields_[i] = fields_[i]->Copy(zone); + copy->fields_[i] = fields_[i] == NULL ? NULL : fields_[i]->Copy(zone); } if (FLAG_trace_load_elimination) { TRACE((" copy-to B%d\n", succ->block_id())); diff --git a/src/spaces.cc b/src/spaces.cc index 7966a04..ab7f41e 100644 --- a/src/spaces.cc +++ b/src/spaces.cc @@ -322,9 +322,12 @@ void MemoryAllocator::FreeMemory(VirtualMemory* reservation, size_executable_ -= size; } // Code which is part of the code-range does not have its own VirtualMemory. - ASSERT(!isolate_->code_range()->contains( - static_cast
(reservation->address()))); - ASSERT(executable == NOT_EXECUTABLE || !isolate_->code_range()->exists()); + ASSERT(isolate_->code_range() == NULL || + !isolate_->code_range()->contains( + static_cast
(reservation->address()))); + ASSERT(executable == NOT_EXECUTABLE || + isolate_->code_range() == NULL || + !isolate_->code_range()->valid()); reservation->Release(); } @@ -342,11 +345,14 @@ void MemoryAllocator::FreeMemory(Address base, ASSERT(size_executable_ >= size); size_executable_ -= size; } - if (isolate_->code_range()->contains(static_cast
(base))) { + if (isolate_->code_range() != NULL && + isolate_->code_range()->contains(static_cast
(base))) { ASSERT(executable == EXECUTABLE); isolate_->code_range()->FreeRawMemory(base, size); } else { - ASSERT(executable == NOT_EXECUTABLE || !isolate_->code_range()->exists()); + ASSERT(executable == NOT_EXECUTABLE || + isolate_->code_range() == NULL || + !isolate_->code_range()->valid()); bool result = VirtualMemory::ReleaseRegion(base, size); USE(result); ASSERT(result); @@ -522,7 +528,8 @@ bool MemoryChunk::CommitArea(size_t requested) { } } else { CodeRange* code_range = heap_->isolate()->code_range(); - ASSERT(code_range->exists() && IsFlagSet(IS_EXECUTABLE)); + ASSERT(code_range != NULL && code_range->valid() && + IsFlagSet(IS_EXECUTABLE)); if (!code_range->CommitRawMemory(start, length)) return false; } @@ -538,7 +545,8 @@ bool MemoryChunk::CommitArea(size_t requested) { if (!reservation_.Uncommit(start, length)) return false; } else { CodeRange* code_range = heap_->isolate()->code_range(); - ASSERT(code_range->exists() && IsFlagSet(IS_EXECUTABLE)); + ASSERT(code_range != NULL && code_range->valid() && + IsFlagSet(IS_EXECUTABLE)); if (!code_range->UncommitRawMemory(start, length)) return false; } } @@ -628,7 +636,7 @@ MemoryChunk* MemoryAllocator::AllocateChunk(intptr_t reserve_area_size, OS::CommitPageSize()); // Allocate executable memory either from code range or from the // OS. - if (isolate_->code_range()->exists()) { + if (isolate_->code_range() != NULL && isolate_->code_range()->valid()) { base = isolate_->code_range()->AllocateRawMemory(chunk_size, commit_size, &chunk_size); @@ -1050,8 +1058,9 @@ intptr_t PagedSpace::SizeOfFirstPage() { case PROPERTY_CELL_SPACE: size = 8 * kPointerSize * KB; break; - case CODE_SPACE: - if (heap()->isolate()->code_range()->exists()) { + case CODE_SPACE: { + CodeRange* code_range = heap()->isolate()->code_range(); + if (code_range != NULL && code_range->valid()) { // When code range exists, code pages are allocated in a special way // (from the reserved code range). That part of the code is not yet // upgraded to handle small pages. @@ -1062,6 +1071,7 @@ intptr_t PagedSpace::SizeOfFirstPage() { kPointerSize); } break; + } default: UNREACHABLE(); } diff --git a/src/spaces.h b/src/spaces.h index a73daaf..b11bc1e 100644 --- a/src/spaces.h +++ b/src/spaces.h @@ -929,13 +929,13 @@ class CodeRange { // manage it. void TearDown(); - bool exists() { return this != NULL && code_range_ != NULL; } + bool valid() { return code_range_ != NULL; } Address start() { - if (this == NULL || code_range_ == NULL) return NULL; + ASSERT(valid()); return static_cast
(code_range_->address()); } bool contains(Address address) { - if (this == NULL || code_range_ == NULL) return false; + if (!valid()) return false; Address start = static_cast
(code_range_->address()); return start <= address && address < start + code_range_->size(); } diff --git a/src/x64/assembler-x64-inl.h b/src/x64/assembler-x64-inl.h index 5b508ac..f1731af 100644 --- a/src/x64/assembler-x64-inl.h +++ b/src/x64/assembler-x64-inl.h @@ -77,7 +77,6 @@ void Assembler::emit_code_target(Handle target, void Assembler::emit_runtime_entry(Address entry, RelocInfo::Mode rmode) { ASSERT(RelocInfo::IsRuntimeEntry(rmode)); - ASSERT(isolate()->code_range()->exists()); RecordRelocInfo(rmode); emitl(static_cast(entry - isolate()->code_range()->start())); } @@ -213,7 +212,6 @@ Handle Assembler::code_target_object_handle_at(Address pc) { Address Assembler::runtime_entry_at(Address pc) { - ASSERT(isolate()->code_range()->exists()); return Memory::int32_at(pc) + isolate()->code_range()->start(); } diff --git a/test/cctest/test-spaces.cc b/test/cctest/test-spaces.cc index 16ba98f..ec5d704 100644 --- a/test/cctest/test-spaces.cc +++ b/test/cctest/test-spaces.cc @@ -169,7 +169,7 @@ static void VerifyMemoryChunk(Isolate* isolate, commit_area_size, executable, NULL); - size_t alignment = code_range->exists() ? + size_t alignment = code_range != NULL && code_range->valid() ? MemoryChunk::kAlignment : OS::CommitPageSize(); size_t reserved_size = ((executable == EXECUTABLE)) ? RoundUp(header_size + guard_size + reserve_area_size + guard_size, -- 2.7.4