diff --git a/include/v8-platform.h b/include/v8-platform.h index 607cec7a89..91b3fd9cc3 100644 --- a/include/v8-platform.h +++ b/include/v8-platform.h @@ -581,6 +581,8 @@ enum class PagePermissions { * sub-spaces and (private or shared) memory pages can be allocated, freed, and * modified. This interface is meant to eventually replace the PageAllocator * interface, and can be used as an alternative in the meantime. + * + * This API is not yet stable and may change without notice! */ class VirtualAddressSpace { public: @@ -682,16 +684,16 @@ class VirtualAddressSpace { /** * Frees previously allocated pages. * + * This function will terminate the process on failure as this implies a bug + * in the client. As such, there is no return value. + * * \param address The start address of the pages to free. This address must - * have been obtains from a call to AllocatePages. + * have been obtained through a call to AllocatePages. * * \param size The size in bytes of the region to free. This must match the * size passed to AllocatePages when the pages were allocated. - * - * \returns true on success, false otherwise. */ - virtual V8_WARN_UNUSED_RESULT bool FreePages(Address address, - size_t size) = 0; + virtual void FreePages(Address address, size_t size) = 0; /** * Sets permissions of all allocated pages in the given range. @@ -731,17 +733,17 @@ class VirtualAddressSpace { /** * Frees an existing guard region. * + * This function will terminate the process on failure as this implies a bug + * in the client. As such, there is no return value. + * * \param address The start address of the guard region to free. This address * must have previously been used as address parameter in a successful * invocation of AllocateGuardRegion. * * \param size The size in bytes of the guard region to free. This must match * the size passed to AllocateGuardRegion when the region was created. - * - * \returns true on success, false otherwise. */ - virtual V8_WARN_UNUSED_RESULT bool FreeGuardRegion(Address address, - size_t size) = 0; + virtual void FreeGuardRegion(Address address, size_t size) = 0; /** * Allocates shared memory pages with the given permissions. @@ -769,16 +771,16 @@ class VirtualAddressSpace { /** * Frees previously allocated shared pages. * + * This function will terminate the process on failure as this implies a bug + * in the client. As such, there is no return value. + * * \param address The start address of the pages to free. This address must - * have been obtains from a call to AllocateSharedPages. + * have been obtained through a call to AllocateSharedPages. * * \param size The size in bytes of the region to free. This must match the * size passed to AllocateSharedPages when the pages were allocated. - * - * \returns true on success, false otherwise. */ - virtual V8_WARN_UNUSED_RESULT bool FreeSharedPages(Address address, - size_t size) = 0; + virtual void FreeSharedPages(Address address, size_t size) = 0; /** * Whether this instance can allocate subspaces or not. diff --git a/src/base/emulated-virtual-address-subspace.cc b/src/base/emulated-virtual-address-subspace.cc index e6c8952109..ae07a3cd96 100644 --- a/src/base/emulated-virtual-address-subspace.cc +++ b/src/base/emulated-virtual-address-subspace.cc @@ -30,7 +30,7 @@ EmulatedVirtualAddressSubspace::EmulatedVirtualAddressSubspace( } EmulatedVirtualAddressSubspace::~EmulatedVirtualAddressSubspace() { - CHECK(parent_space_->FreePages(base(), mapped_size_)); + parent_space_->FreePages(base(), mapped_size_); } void EmulatedVirtualAddressSubspace::SetRandomSeed(int64_t seed) { @@ -84,7 +84,7 @@ Address EmulatedVirtualAddressSubspace::AllocatePages( if (UnmappedRegionContains(result, size)) { return result; } else if (result) { - CHECK(parent_space_->FreePages(result, size)); + parent_space_->FreePages(result, size); } // Retry at a different address. @@ -94,15 +94,15 @@ Address EmulatedVirtualAddressSubspace::AllocatePages( return kNullAddress; } -bool EmulatedVirtualAddressSubspace::FreePages(Address address, size_t size) { +void EmulatedVirtualAddressSubspace::FreePages(Address address, size_t size) { if (MappedRegionContains(address, size)) { MutexGuard guard(&mutex_); - if (region_allocator_.FreeRegion(address) != size) return false; + CHECK_EQ(size, region_allocator_.FreeRegion(address)); CHECK(parent_space_->DecommitPages(address, size)); - return true; + } else { + DCHECK(UnmappedRegionContains(address, size)); + parent_space_->FreePages(address, size); } - if (!UnmappedRegionContains(address, size)) return false; - return parent_space_->FreePages(address, size); } Address EmulatedVirtualAddressSubspace::AllocateSharedPages( @@ -124,7 +124,7 @@ Address EmulatedVirtualAddressSubspace::AllocateSharedPages( if (UnmappedRegionContains(region, size)) { return region; } else if (region) { - CHECK(parent_space_->FreeSharedPages(region, size)); + parent_space_->FreeSharedPages(region, size); } hint = RandomPageAddress(); @@ -133,9 +133,10 @@ Address EmulatedVirtualAddressSubspace::AllocateSharedPages( return kNullAddress; } -bool EmulatedVirtualAddressSubspace::FreeSharedPages(Address address, +void EmulatedVirtualAddressSubspace::FreeSharedPages(Address address, size_t size) { - return parent_space_->FreeSharedPages(address, size); + DCHECK(UnmappedRegionContains(address, size)); + parent_space_->FreeSharedPages(address, size); } bool EmulatedVirtualAddressSubspace::SetPagePermissions( @@ -154,14 +155,15 @@ bool EmulatedVirtualAddressSubspace::AllocateGuardRegion(Address address, return parent_space_->AllocateGuardRegion(address, size); } -bool EmulatedVirtualAddressSubspace::FreeGuardRegion(Address address, +void EmulatedVirtualAddressSubspace::FreeGuardRegion(Address address, size_t size) { if (MappedRegionContains(address, size)) { MutexGuard guard(&mutex_); - return region_allocator_.FreeRegion(address) == size; + CHECK_EQ(size, region_allocator_.FreeRegion(address)); + } else { + DCHECK(UnmappedRegionContains(address, size)); + parent_space_->FreeGuardRegion(address, size); } - if (!UnmappedRegionContains(address, size)) return false; - return parent_space_->FreeGuardRegion(address, size); } bool EmulatedVirtualAddressSubspace::CanAllocateSubspaces() { diff --git a/src/base/emulated-virtual-address-subspace.h b/src/base/emulated-virtual-address-subspace.h index da490e1b26..c507835550 100644 --- a/src/base/emulated-virtual-address-subspace.h +++ b/src/base/emulated-virtual-address-subspace.h @@ -48,21 +48,21 @@ class V8_BASE_EXPORT EmulatedVirtualAddressSubspace final Address AllocatePages(Address hint, size_t size, size_t alignment, PagePermissions permissions) override; - bool FreePages(Address address, size_t size) override; + void FreePages(Address address, size_t size) override; Address AllocateSharedPages(Address hint, size_t size, PagePermissions permissions, PlatformSharedMemoryHandle handle, uint64_t offset) override; - bool FreeSharedPages(Address address, size_t size) override; + void FreeSharedPages(Address address, size_t size) override; bool SetPagePermissions(Address address, size_t size, PagePermissions permissions) override; bool AllocateGuardRegion(Address address, size_t size) override; - bool FreeGuardRegion(Address address, size_t size) override; + void FreeGuardRegion(Address address, size_t size) override; bool CanAllocateSubspaces() override; diff --git a/src/base/sanitizer/lsan-virtual-address-space.cc b/src/base/sanitizer/lsan-virtual-address-space.cc index bed822974f..cd8d0decae 100644 --- a/src/base/sanitizer/lsan-virtual-address-space.cc +++ b/src/base/sanitizer/lsan-virtual-address-space.cc @@ -35,14 +35,11 @@ Address LsanVirtualAddressSpace::AllocatePages(Address hint, size_t size, return result; } -bool LsanVirtualAddressSpace::FreePages(Address address, size_t size) { - bool result = vas_->FreePages(address, size); +void LsanVirtualAddressSpace::FreePages(Address address, size_t size) { + vas_->FreePages(address, size); #if defined(LEAK_SANITIZER) - if (result) { - __lsan_unregister_root_region(reinterpret_cast(address), size); - } + __lsan_unregister_root_region(reinterpret_cast(address), size); #endif // defined(LEAK_SANITIZER) - return result; } Address LsanVirtualAddressSpace::AllocateSharedPages( @@ -58,14 +55,11 @@ Address LsanVirtualAddressSpace::AllocateSharedPages( return result; } -bool LsanVirtualAddressSpace::FreeSharedPages(Address address, size_t size) { - bool result = vas_->FreeSharedPages(address, size); +void LsanVirtualAddressSpace::FreeSharedPages(Address address, size_t size) { + vas_->FreeSharedPages(address, size); #if defined(LEAK_SANITIZER) - if (result) { - __lsan_unregister_root_region(reinterpret_cast(address), size); - } + __lsan_unregister_root_region(reinterpret_cast(address), size); #endif // defined(LEAK_SANITIZER) - return result; } std::unique_ptr LsanVirtualAddressSpace::AllocateSubspace( diff --git a/src/base/sanitizer/lsan-virtual-address-space.h b/src/base/sanitizer/lsan-virtual-address-space.h index 733d240e99..00cd32a39f 100644 --- a/src/base/sanitizer/lsan-virtual-address-space.h +++ b/src/base/sanitizer/lsan-virtual-address-space.h @@ -33,14 +33,14 @@ class V8_BASE_EXPORT LsanVirtualAddressSpace final Address AllocatePages(Address hint, size_t size, size_t alignment, PagePermissions permissions) override; - bool FreePages(Address address, size_t size) override; + void FreePages(Address address, size_t size) override; Address AllocateSharedPages(Address hint, size_t size, PagePermissions permissions, PlatformSharedMemoryHandle handle, uint64_t offset) override; - bool FreeSharedPages(Address address, size_t size) override; + void FreeSharedPages(Address address, size_t size) override; bool SetPagePermissions(Address address, size_t size, PagePermissions permissions) override { @@ -51,8 +51,8 @@ class V8_BASE_EXPORT LsanVirtualAddressSpace final return vas_->AllocateGuardRegion(address, size); } - bool FreeGuardRegion(Address address, size_t size) override { - return vas_->FreeGuardRegion(address, size); + void FreeGuardRegion(Address address, size_t size) override { + vas_->FreeGuardRegion(address, size); } bool CanAllocateSubspaces() override { return vas_->CanAllocateSubspaces(); } diff --git a/src/base/virtual-address-space-page-allocator.cc b/src/base/virtual-address-space-page-allocator.cc index 3412b70ba2..f88afdcc19 100644 --- a/src/base/virtual-address-space-page-allocator.cc +++ b/src/base/virtual-address-space-page-allocator.cc @@ -28,7 +28,7 @@ bool VirtualAddressSpacePageAllocator::FreePages(void* ptr, size_t size) { size = result->second; resized_allocations_.erase(result); } - CHECK(vas_->FreePages(address, size)); + vas_->FreePages(address, size); return true; } diff --git a/src/base/virtual-address-space.cc b/src/base/virtual-address-space.cc index 7d45a1c5e3..6ef95f5ca8 100644 --- a/src/base/virtual-address-space.cc +++ b/src/base/virtual-address-space.cc @@ -85,12 +85,11 @@ Address VirtualAddressSpace::AllocatePages(Address hint, size_t size, static_cast(permissions))); } -bool VirtualAddressSpace::FreePages(Address address, size_t size) { +void VirtualAddressSpace::FreePages(Address address, size_t size) { DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity())); OS::Free(reinterpret_cast(address), size); - return true; } bool VirtualAddressSpace::SetPagePermissions(Address address, size_t size, @@ -115,12 +114,11 @@ bool VirtualAddressSpace::AllocateGuardRegion(Address address, size_t size) { return result == hint; } -bool VirtualAddressSpace::FreeGuardRegion(Address address, size_t size) { +void VirtualAddressSpace::FreeGuardRegion(Address address, size_t size) { DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity())); OS::Free(reinterpret_cast(address), size); - return true; } bool VirtualAddressSpace::CanAllocateSubspaces() { @@ -139,12 +137,11 @@ Address VirtualAddressSpace::AllocateSharedPages( static_cast(permissions), handle, offset)); } -bool VirtualAddressSpace::FreeSharedPages(Address address, size_t size) { +void VirtualAddressSpace::FreeSharedPages(Address address, size_t size) { DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity())); OS::FreeShared(reinterpret_cast(address), size); - return true; } std::unique_ptr VirtualAddressSpace::AllocateSubspace( @@ -178,9 +175,8 @@ bool VirtualAddressSpace::DecommitPages(Address address, size_t size) { return OS::DecommitPages(reinterpret_cast(address), size); } -bool VirtualAddressSpace::FreeSubspace(VirtualAddressSubspace* subspace) { +void VirtualAddressSpace::FreeSubspace(VirtualAddressSubspace* subspace) { OS::FreeAddressSpaceReservation(subspace->reservation_); - return true; } VirtualAddressSubspace::VirtualAddressSubspace( @@ -210,7 +206,7 @@ VirtualAddressSubspace::VirtualAddressSubspace( } VirtualAddressSubspace::~VirtualAddressSubspace() { - CHECK(parent_space_->FreeSubspace(this)); + parent_space_->FreeSubspace(this); } void VirtualAddressSubspace::SetRandomSeed(int64_t seed) { @@ -249,19 +245,16 @@ Address VirtualAddressSubspace::AllocatePages(Address hint, size_t size, return address; } -bool VirtualAddressSubspace::FreePages(Address address, size_t size) { +void VirtualAddressSubspace::FreePages(Address address, size_t size) { DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity())); MutexGuard guard(&mutex_); - if (region_allocator_.CheckRegion(address) != size) return false; - // The order here is important: on Windows, the allocation first has to be // freed to a placeholder before the placeholder can be merged (during the // merge_callback) with any surrounding placeholder mappings. CHECK(reservation_.Free(reinterpret_cast(address), size)); CHECK_EQ(size, region_allocator_.FreeRegion(address)); - return true; } bool VirtualAddressSubspace::SetPagePermissions(Address address, size_t size, @@ -286,13 +279,12 @@ bool VirtualAddressSubspace::AllocateGuardRegion(Address address, size_t size) { return region_allocator_.AllocateRegionAt(address, size); } -bool VirtualAddressSubspace::FreeGuardRegion(Address address, size_t size) { +void VirtualAddressSubspace::FreeGuardRegion(Address address, size_t size) { DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity())); MutexGuard guard(&mutex_); - - return region_allocator_.FreeRegion(address) == size; + CHECK_EQ(size, region_allocator_.FreeRegion(address)); } Address VirtualAddressSubspace::AllocateSharedPages( @@ -318,19 +310,16 @@ Address VirtualAddressSubspace::AllocateSharedPages( return address; } -bool VirtualAddressSubspace::FreeSharedPages(Address address, size_t size) { +void VirtualAddressSubspace::FreeSharedPages(Address address, size_t size) { DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity())); MutexGuard guard(&mutex_); - if (region_allocator_.CheckRegion(address) != size) return false; - // The order here is important: on Windows, the allocation first has to be // freed to a placeholder before the placeholder can be merged (during the // merge_callback) with any surrounding placeholder mappings. CHECK(reservation_.FreeShared(reinterpret_cast(address), size)); CHECK_EQ(size, region_allocator_.FreeRegion(address)); - return true; } std::unique_ptr @@ -376,16 +365,13 @@ bool VirtualAddressSubspace::DecommitPages(Address address, size_t size) { return reservation_.DecommitPages(reinterpret_cast(address), size); } -bool VirtualAddressSubspace::FreeSubspace(VirtualAddressSubspace* subspace) { +void VirtualAddressSubspace::FreeSubspace(VirtualAddressSubspace* subspace) { MutexGuard guard(&mutex_); AddressSpaceReservation reservation = subspace->reservation_; Address base = reinterpret_cast
(reservation.base()); - if (region_allocator_.FreeRegion(base) != reservation.size()) { - return false; - } - - return reservation_.FreeSubReservation(reservation); + CHECK_EQ(reservation.size(), region_allocator_.FreeRegion(base)); + CHECK(reservation_.FreeSubReservation(reservation)); } } // namespace base diff --git a/src/base/virtual-address-space.h b/src/base/virtual-address-space.h index 101b763ebd..3681367777 100644 --- a/src/base/virtual-address-space.h +++ b/src/base/virtual-address-space.h @@ -32,7 +32,7 @@ class VirtualAddressSpaceBase // Called by a subspace during destruction. Responsible for freeing the // address space reservation and any other data associated with the subspace // in the parent space. - virtual bool FreeSubspace(VirtualAddressSubspace* subspace) = 0; + virtual void FreeSubspace(VirtualAddressSubspace* subspace) = 0; }; /* @@ -59,21 +59,21 @@ class V8_BASE_EXPORT VirtualAddressSpace : public VirtualAddressSpaceBase { Address AllocatePages(Address hint, size_t size, size_t alignment, PagePermissions access) override; - bool FreePages(Address address, size_t size) override; + void FreePages(Address address, size_t size) override; bool SetPagePermissions(Address address, size_t size, PagePermissions access) override; bool AllocateGuardRegion(Address address, size_t size) override; - bool FreeGuardRegion(Address address, size_t size) override; + void FreeGuardRegion(Address address, size_t size) override; Address AllocateSharedPages(Address hint, size_t size, PagePermissions permissions, PlatformSharedMemoryHandle handle, uint64_t offset) override; - bool FreeSharedPages(Address address, size_t size) override; + void FreeSharedPages(Address address, size_t size) override; bool CanAllocateSubspaces() override; @@ -86,7 +86,7 @@ class V8_BASE_EXPORT VirtualAddressSpace : public VirtualAddressSpaceBase { bool DecommitPages(Address address, size_t size) override; private: - bool FreeSubspace(VirtualAddressSubspace* subspace) override; + void FreeSubspace(VirtualAddressSubspace* subspace) override; }; /* @@ -104,21 +104,21 @@ class V8_BASE_EXPORT VirtualAddressSubspace : public VirtualAddressSpaceBase { Address AllocatePages(Address hint, size_t size, size_t alignment, PagePermissions permissions) override; - bool FreePages(Address address, size_t size) override; + void FreePages(Address address, size_t size) override; bool SetPagePermissions(Address address, size_t size, PagePermissions permissions) override; bool AllocateGuardRegion(Address address, size_t size) override; - bool FreeGuardRegion(Address address, size_t size) override; + void FreeGuardRegion(Address address, size_t size) override; Address AllocateSharedPages(Address hint, size_t size, PagePermissions permissions, PlatformSharedMemoryHandle handle, uint64_t offset) override; - bool FreeSharedPages(Address address, size_t size) override; + void FreeSharedPages(Address address, size_t size) override; bool CanAllocateSubspaces() override { return true; } @@ -135,7 +135,7 @@ class V8_BASE_EXPORT VirtualAddressSubspace : public VirtualAddressSpaceBase { // allocating sub spaces. friend class v8::base::VirtualAddressSpace; - bool FreeSubspace(VirtualAddressSubspace* subspace) override; + void FreeSubspace(VirtualAddressSubspace* subspace) override; VirtualAddressSubspace(AddressSpaceReservation reservation, VirtualAddressSpaceBase* parent_space, diff --git a/src/sandbox/external-pointer-table-inl.h b/src/sandbox/external-pointer-table-inl.h index 063768889b..cda26fa07e 100644 --- a/src/sandbox/external-pointer-table-inl.h +++ b/src/sandbox/external-pointer-table-inl.h @@ -49,9 +49,8 @@ void ExternalPointerTable::Init(Isolate* isolate) { void ExternalPointerTable::TearDown() { DCHECK(is_initialized()); - CHECK(GetPlatformVirtualAddressSpace()->FreePages( - buffer_, kExternalPointerTableReservationSize)); - + GetPlatformVirtualAddressSpace()->FreePages( + buffer_, kExternalPointerTableReservationSize); delete mutex_; buffer_ = kNullAddress; diff --git a/src/sandbox/sandbox.cc b/src/sandbox/sandbox.cc index c60ee603d3..aaeabc2c8c 100644 --- a/src/sandbox/sandbox.cc +++ b/src/sandbox/sandbox.cc @@ -273,7 +273,7 @@ bool Sandbox::InitializeAsPartiallyReservedSandbox(v8::VirtualAddressSpace* vas, break; // Can't use this base, so free the reservation and try again - CHECK(vas->FreePages(reservation_base_, size_to_reserve)); + vas->FreePages(reservation_base_, size_to_reserve); reservation_base_ = kNullAddress; } DCHECK(reservation_base_); diff --git a/test/unittests/base/virtual-address-space-unittest.cc b/test/unittests/base/virtual-address-space-unittest.cc index 839396d072..bd5a57ca0f 100644 --- a/test/unittests/base/virtual-address-space-unittest.cc +++ b/test/unittests/base/virtual-address-space-unittest.cc @@ -55,7 +55,7 @@ void TestBasicPageAllocation(v8::VirtualAddressSpace* space) { for (Address allocation : allocations) { //... and readable size_t size = *reinterpret_cast(allocation); - EXPECT_TRUE(space->FreePages(allocation, size)); + space->FreePages(allocation, size); } } @@ -75,7 +75,7 @@ void TestPageAllocationAlignment(v8::VirtualAddressSpace* space) { EXPECT_GE(allocation, space->base()); EXPECT_LT(allocation, space->base() + space->size()); - EXPECT_TRUE(space->FreePages(allocation, size)); + space->FreePages(allocation, size); } } @@ -94,7 +94,8 @@ void TestParentSpaceCannotAllocateInChildSpace(v8::VirtualAddressSpace* parent, PagePermissions::kNoAccess); ASSERT_NE(kNullAddress, allocation); EXPECT_TRUE(allocation < start || allocation >= end); - EXPECT_TRUE(parent->FreePages(allocation, chunksize)); + + parent->FreePages(allocation, chunksize); } } @@ -120,8 +121,8 @@ void TestSharedPageAllocation(v8::VirtualAddressSpace* space) { *reinterpret_cast(mapping1) = value; EXPECT_EQ(value, *reinterpret_cast(mapping2)); - EXPECT_TRUE(space->FreeSharedPages(mapping1, size)); - EXPECT_TRUE(space->FreeSharedPages(mapping2, size)); + space->FreeSharedPages(mapping1, size); + space->FreeSharedPages(mapping2, size); OS::DestroySharedMemoryHandle(handle); } @@ -234,14 +235,14 @@ TEST(VirtualAddressSpaceTest, TestEmulatedSubspace) { PagePermissions::kNoAccess); ASSERT_NE(kNullAddress, reservation); hint = reservation; - EXPECT_TRUE(rootspace.FreePages(reservation, kSubspaceSize)); + rootspace.FreePages(reservation, kSubspaceSize); reservation = rootspace.AllocatePages(hint, kSubspaceMappedSize, subspace_alignment, PagePermissions::kNoAccess); if (reservation == hint) { break; } else { - EXPECT_TRUE(rootspace.FreePages(reservation, kSubspaceMappedSize)); + rootspace.FreePages(reservation, kSubspaceMappedSize); reservation = kNullAddress; } } diff --git a/test/unittests/sandbox/sandbox-unittest.cc b/test/unittests/sandbox/sandbox-unittest.cc index aa28b7c931..8818d197c4 100644 --- a/test/unittests/sandbox/sandbox-unittest.cc +++ b/test/unittests/sandbox/sandbox-unittest.cc @@ -120,7 +120,7 @@ void TestPageAllocationInSandbox(Sandbox& sandbox) { for (int i = 0; i < kNumAllocations; i++) { size_t length = allocation_granularity * kAllocatinSizesInPages[i]; - EXPECT_TRUE(vas->FreePages(allocations[i], length)); + vas->FreePages(allocations[i], length); } }