AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[base] Don't return bool from VirtualAddressSpace::Free* routines

Browse Source 
Instead of returning a boolean success/failure value, the Free* methods
of the VirtualAddressSpace API now terminate the process on failure, as
this implies a bug in the caller. This is simpler than CHECKing for
success in all callers and also provides more details about the possible
cause of the failure.

Bug: v8:12656
Change-Id: I5b469ae2c564068cff74e60b7e98f6a4776a239d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3506992
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79388}
This commit is contained in:
Samuel Groß 2022-03-07 17:01:11 +01:00 committed by V8 LUCI CQ
parent 589036411c
commit f43f8a0bb5
12 changed files with 79 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
include/v8-platform.h
View File

@ -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.

30
src/base/emulated-virtual-address-subspace.cc
View File

@ -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() {

6
src/base/emulated-virtual-address-subspace.h
View File

@ -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;

18
src/base/sanitizer/lsan-virtual-address-space.cc
View File

@ -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<void*>(address), size);
}
__lsan_unregister_root_region(reinterpret_cast<void*>(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<void*>(address), size);
}
__lsan_unregister_root_region(reinterpret_cast<void*>(address), size);
#endif // defined(LEAK_SANITIZER)
return result;
}
std::unique_ptr<VirtualAddressSpace> LsanVirtualAddressSpace::AllocateSubspace(

8
src/base/sanitizer/lsan-virtual-address-space.h
View File

@ -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(); }

2
src/base/virtual-address-space-page-allocator.cc
View File

@ -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;
}

38
src/base/virtual-address-space.cc
View File

@ -85,12 +85,11 @@ Address VirtualAddressSpace::AllocatePages(Address hint, size_t size,
static_cast<OS::MemoryPermission>(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<void*>(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<void*>(address), size);
return true;
}
bool VirtualAddressSpace::CanAllocateSubspaces() {
@ -139,12 +137,11 @@ Address VirtualAddressSpace::AllocateSharedPages(
static_cast<OS::MemoryPermission>(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<void*>(address), size);
return true;
}
std::unique_ptr<v8::VirtualAddressSpace> VirtualAddressSpace::AllocateSubspace(
@ -178,9 +175,8 @@ bool VirtualAddressSpace::DecommitPages(Address address, size_t size) {
return OS::DecommitPages(reinterpret_cast<void*>(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<void*>(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<void*>(address), size));
CHECK_EQ(size, region_allocator_.FreeRegion(address));
return true;
}
std::unique_ptr<v8::VirtualAddressSpace>
@ -376,16 +365,13 @@ bool VirtualAddressSubspace::DecommitPages(Address address, size_t size) {
return reservation_.DecommitPages(reinterpret_cast<void*>(address), size);
}
bool VirtualAddressSubspace::FreeSubspace(VirtualAddressSubspace* subspace) {
void VirtualAddressSubspace::FreeSubspace(VirtualAddressSubspace* subspace) {
MutexGuard guard(&mutex_);
AddressSpaceReservation reservation = subspace->reservation_;
Address base = reinterpret_cast<Address>(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

18
src/base/virtual-address-space.h
View File

@ -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,

5
src/sandbox/external-pointer-table-inl.h
View File

@ -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;

2
src/sandbox/sandbox.cc
View File

@ -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_);

15
test/unittests/base/virtual-address-space-unittest.cc
View File

@ -55,7 +55,7 @@ void TestBasicPageAllocation(v8::VirtualAddressSpace* space) {
for (Address allocation : allocations) {
//... and readable
size_t size = *reinterpret_cast<size_t*>(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<int*>(mapping1) = value;
EXPECT_EQ(value, *reinterpret_cast<int*>(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;
}
}

2
test/unittests/sandbox/sandbox-unittest.cc
View File

@ -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);
}
}