Commit f8379afa authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[base] Fail early in OS::Free and OS::Release

Instead of returning a boolean value, and then failing in the caller via
a CHECK, do fail directly inside OS::Free, OS::Release and similar
functions.

The PageAllocator methods still return a bool (which is always true) to
avoid changing the public API.

R=mlippautz@chromium.org

Bug: v8:12656, chromium:1299735
Cq-Include-Trybots: luci.v8.try:v8_fuchsia_compile_rel
Change-Id: Ide02e7d893e1603326c629797a7defac8bf258ef
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3483671Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79258}
parent 227434be
...@@ -132,13 +132,15 @@ void* PageAllocator::RemapShared(void* old_address, void* new_address, ...@@ -132,13 +132,15 @@ void* PageAllocator::RemapShared(void* old_address, void* new_address,
} }
bool PageAllocator::FreePages(void* address, size_t size) { bool PageAllocator::FreePages(void* address, size_t size) {
return base::OS::Free(address, size); base::OS::Free(address, size);
return true;
} }
bool PageAllocator::ReleasePages(void* address, size_t size, size_t new_size) { bool PageAllocator::ReleasePages(void* address, size_t size, size_t new_size) {
DCHECK_LT(new_size, size); DCHECK_LT(new_size, size);
return base::OS::Release(reinterpret_cast<uint8_t*>(address) + new_size, base::OS::Release(reinterpret_cast<uint8_t*>(address) + new_size,
size - new_size); size - new_size);
return true;
} }
bool PageAllocator::SetPermissions(void* address, size_t size, bool PageAllocator::SetPermissions(void* address, size_t size,
......
...@@ -118,7 +118,7 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment, ...@@ -118,7 +118,7 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment,
if (base == aligned_base) return reinterpret_cast<void*>(base); if (base == aligned_base) return reinterpret_cast<void*>(base);
// Otherwise, free it and try a larger allocation. // Otherwise, free it and try a larger allocation.
CHECK(Free(base, size)); Free(base, size);
// Clear the hint. It's unlikely we can allocate at this address. // Clear the hint. It's unlikely we can allocate at this address.
hint = nullptr; hint = nullptr;
...@@ -134,7 +134,7 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment, ...@@ -134,7 +134,7 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment,
// Try to trim the allocation by freeing the padded allocation and then // Try to trim the allocation by freeing the padded allocation and then
// calling VirtualAlloc at the aligned base. // calling VirtualAlloc at the aligned base.
CHECK(Free(base, padded_size)); Free(base, padded_size);
aligned_base = RoundUp(base, alignment); aligned_base = RoundUp(base, alignment);
base = reinterpret_cast<uint8_t*>( base = reinterpret_cast<uint8_t*>(
VirtualAlloc(aligned_base, size, flags, protect)); VirtualAlloc(aligned_base, size, flags, protect));
...@@ -147,18 +147,18 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment, ...@@ -147,18 +147,18 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment,
} }
// static // static
bool OS::Free(void* address, const size_t size) { void OS::Free(void* address, const size_t size) {
DCHECK_EQ(0, static_cast<uintptr_t>(address) % AllocatePageSize()); DCHECK_EQ(0, static_cast<uintptr_t>(address) % AllocatePageSize());
DCHECK_EQ(0, size % AllocatePageSize()); DCHECK_EQ(0, size % AllocatePageSize());
USE(size); USE(size);
return VirtualFree(address, 0, MEM_RELEASE) != 0; CHECK_NE(0, VirtualFree(address, 0, MEM_RELEASE));
} }
// static // static
bool OS::Release(void* address, size_t size) { void OS::Release(void* address, size_t size) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize()); DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize());
DCHECK_EQ(0, size % CommitPageSize()); DCHECK_EQ(0, size % CommitPageSize());
return VirtualFree(address, size, MEM_DECOMMIT) != 0; CHECK_NE(0, VirtualFree(address, size, MEM_DECOMMIT));
} }
// static // static
......
...@@ -254,8 +254,8 @@ void* OS::Allocate(void* address, size_t size, size_t alignment, ...@@ -254,8 +254,8 @@ void* OS::Allocate(void* address, size_t size, size_t alignment,
} }
// static // static
bool OS::Free(void* address, size_t size) { void OS::Free(void* address, size_t size) {
return UnmapVmo(*zx::vmar::root_self(), AllocatePageSize(), address, size); CHECK(UnmapVmo(*zx::vmar::root_self(), AllocatePageSize(), address, size));
} }
// static // static
...@@ -271,12 +271,12 @@ void* OS::AllocateShared(void* address, size_t size, ...@@ -271,12 +271,12 @@ void* OS::AllocateShared(void* address, size_t size,
} }
// static // static
bool OS::FreeShared(void* address, size_t size) { void OS::FreeShared(void* address, size_t size) {
return UnmapVmo(*zx::vmar::root_self(), AllocatePageSize(), address, size); CHECK(UnmapVmo(*zx::vmar::root_self(), AllocatePageSize(), address, size));
} }
// static // static
bool OS::Release(void* address, size_t size) { return Free(address, size); } void OS::Release(void* address, size_t size) { Free(address, size); }
// static // static
bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) { bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
...@@ -320,10 +320,10 @@ Optional<AddressSpaceReservation> OS::CreateAddressSpaceReservation( ...@@ -320,10 +320,10 @@ Optional<AddressSpaceReservation> OS::CreateAddressSpaceReservation(
} }
// static // static
bool OS::FreeAddressSpaceReservation(AddressSpaceReservation reservation) { void OS::FreeAddressSpaceReservation(AddressSpaceReservation reservation) {
// Destroy the vmar and release the handle. // Destroy the vmar and release the handle.
zx::vmar vmar(reservation.vmar_); zx::vmar vmar(reservation.vmar_);
return vmar.destroy() == ZX_OK; CHECK_EQ(ZX_OK, vmar.destroy());
} }
// static // static
...@@ -399,7 +399,8 @@ Optional<AddressSpaceReservation> AddressSpaceReservation::CreateSubReservation( ...@@ -399,7 +399,8 @@ Optional<AddressSpaceReservation> AddressSpaceReservation::CreateSubReservation(
bool AddressSpaceReservation::FreeSubReservation( bool AddressSpaceReservation::FreeSubReservation(
AddressSpaceReservation reservation) { AddressSpaceReservation reservation) {
return OS::FreeAddressSpaceReservation(reservation); OS::FreeAddressSpaceReservation(reservation);
return true;
} }
bool AddressSpaceReservation::Allocate(void* address, size_t size, bool AddressSpaceReservation::Allocate(void* address, size_t size,
......
...@@ -138,7 +138,7 @@ void OS::SignalCodeMovingGC() { ...@@ -138,7 +138,7 @@ void OS::SignalCodeMovingGC() {
void* addr = mmap(OS::GetRandomMmapAddr(), size, PROT_READ | PROT_EXEC, void* addr = mmap(OS::GetRandomMmapAddr(), size, PROT_READ | PROT_EXEC,
MAP_PRIVATE, fileno(f), 0); MAP_PRIVATE, fileno(f), 0);
DCHECK_NE(MAP_FAILED, addr); DCHECK_NE(MAP_FAILED, addr);
CHECK(Free(addr, size)); Free(addr, size);
fclose(f); fclose(f);
} }
......
...@@ -116,7 +116,7 @@ void OS::SignalCodeMovingGC() { ...@@ -116,7 +116,7 @@ void OS::SignalCodeMovingGC() {
void* addr = void* addr =
mmap(NULL, size, PROT_READ | PROT_EXEC, MAP_PRIVATE, fileno(f), 0); mmap(NULL, size, PROT_READ | PROT_EXEC, MAP_PRIVATE, fileno(f), 0);
DCHECK(addr != MAP_FAILED); DCHECK(addr != MAP_FAILED);
CHECK(OS::Free(addr, size)); OS::Free(addr, size);
fclose(f); fclose(f);
} }
......
...@@ -406,14 +406,14 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment, ...@@ -406,14 +406,14 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment,
if (aligned_base != base) { if (aligned_base != base) {
DCHECK_LT(base, aligned_base); DCHECK_LT(base, aligned_base);
size_t prefix_size = static_cast<size_t>(aligned_base - base); size_t prefix_size = static_cast<size_t>(aligned_base - base);
CHECK(Free(base, prefix_size)); Free(base, prefix_size);
request_size -= prefix_size; request_size -= prefix_size;
} }
// Unmap memory allocated after the potentially unaligned end. // Unmap memory allocated after the potentially unaligned end.
if (size != request_size) { if (size != request_size) {
DCHECK_LT(size, request_size); DCHECK_LT(size, request_size);
size_t suffix_size = request_size - size; size_t suffix_size = request_size - size;
CHECK(Free(aligned_base + size, suffix_size)); Free(aligned_base + size, suffix_size);
request_size -= suffix_size; request_size -= suffix_size;
} }
...@@ -428,10 +428,10 @@ void* OS::AllocateShared(size_t size, MemoryPermission access) { ...@@ -428,10 +428,10 @@ void* OS::AllocateShared(size_t size, MemoryPermission access) {
} }
// static // static
bool OS::Free(void* address, size_t size) { void OS::Free(void* address, size_t size) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % AllocatePageSize()); DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % AllocatePageSize());
DCHECK_EQ(0, size % AllocatePageSize()); DCHECK_EQ(0, size % AllocatePageSize());
return munmap(address, size) == 0; CHECK_EQ(0, munmap(address, size));
} }
// macOS specific implementation in platform-macos.cc. // macOS specific implementation in platform-macos.cc.
...@@ -449,16 +449,16 @@ void* OS::AllocateShared(void* hint, size_t size, MemoryPermission access, ...@@ -449,16 +449,16 @@ void* OS::AllocateShared(void* hint, size_t size, MemoryPermission access,
#endif // !defined(V8_OS_MACOS) #endif // !defined(V8_OS_MACOS)
// static // static
bool OS::FreeShared(void* address, size_t size) { void OS::FreeShared(void* address, size_t size) {
DCHECK_EQ(0, size % AllocatePageSize()); DCHECK_EQ(0, size % AllocatePageSize());
return munmap(address, size) == 0; CHECK_EQ(0, munmap(address, size));
} }
// static // static
bool OS::Release(void* address, size_t size) { void OS::Release(void* address, size_t size) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize()); DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize());
DCHECK_EQ(0, size % CommitPageSize()); DCHECK_EQ(0, size % CommitPageSize());
return munmap(address, size) == 0; CHECK_EQ(0, munmap(address, size));
} }
// static // static
...@@ -568,8 +568,8 @@ Optional<AddressSpaceReservation> OS::CreateAddressSpaceReservation( ...@@ -568,8 +568,8 @@ Optional<AddressSpaceReservation> OS::CreateAddressSpaceReservation(
} }
// static // static
bool OS::FreeAddressSpaceReservation(AddressSpaceReservation reservation) { void OS::FreeAddressSpaceReservation(AddressSpaceReservation reservation) {
return Free(reservation.base(), reservation.size()); Free(reservation.base(), reservation.size());
} }
// macOS specific implementation in platform-macos.cc. // macOS specific implementation in platform-macos.cc.
...@@ -719,7 +719,7 @@ OS::MemoryMappedFile* OS::MemoryMappedFile::create(const char* name, ...@@ -719,7 +719,7 @@ OS::MemoryMappedFile* OS::MemoryMappedFile::create(const char* name,
PosixMemoryMappedFile::~PosixMemoryMappedFile() { PosixMemoryMappedFile::~PosixMemoryMappedFile() {
if (memory_) CHECK(OS::Free(memory_, RoundUp(size_, OS::AllocatePageSize()))); if (memory_) OS::Free(memory_, RoundUp(size_, OS::AllocatePageSize()));
fclose(file_); fclose(file_);
} }
......
...@@ -172,14 +172,14 @@ void* OS::Allocate(void* address, size_t size, size_t alignment, ...@@ -172,14 +172,14 @@ void* OS::Allocate(void* address, size_t size, size_t alignment,
if (aligned_base != base) { if (aligned_base != base) {
DCHECK_LT(base, aligned_base); DCHECK_LT(base, aligned_base);
size_t prefix_size = static_cast<size_t>(aligned_base - base); size_t prefix_size = static_cast<size_t>(aligned_base - base);
CHECK(Free(base, prefix_size)); Free(base, prefix_size);
request_size -= prefix_size; request_size -= prefix_size;
} }
// Unmap memory allocated after the potentially unaligned end. // Unmap memory allocated after the potentially unaligned end.
if (size != request_size) { if (size != request_size) {
DCHECK_LT(size, request_size); DCHECK_LT(size, request_size);
size_t suffix_size = request_size - size; size_t suffix_size = request_size - size;
CHECK(Free(aligned_base + size, suffix_size)); Free(aligned_base + size, suffix_size);
request_size -= suffix_size; request_size -= suffix_size;
} }
...@@ -188,13 +188,13 @@ void* OS::Allocate(void* address, size_t size, size_t alignment, ...@@ -188,13 +188,13 @@ void* OS::Allocate(void* address, size_t size, size_t alignment,
} }
// static // static
bool OS::Free(void* address, const size_t size) { void OS::Free(void* address, const size_t size) {
return SbMemoryUnmap(address, size); CHECK(SbMemoryUnmap(address, size));
} }
// static // static
bool OS::Release(void* address, size_t size) { void OS::Release(void* address, size_t size) {
return SbMemoryUnmap(address, size); CHECK(SbMemoryUnmap(address, size));
} }
// static // static
......
...@@ -926,11 +926,11 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment, ...@@ -926,11 +926,11 @@ void* OS::Allocate(void* hint, size_t size, size_t alignment,
} }
// static // static
bool OS::Free(void* address, size_t size) { void OS::Free(void* address, size_t size) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % AllocatePageSize()); DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % AllocatePageSize());
DCHECK_EQ(0, size % AllocatePageSize()); DCHECK_EQ(0, size % AllocatePageSize());
USE(size); USE(size);
return VirtualFree(address, 0, MEM_RELEASE) != 0; CHECK_NE(0, VirtualFree(address, 0, MEM_RELEASE));
} }
// static // static
...@@ -957,15 +957,15 @@ void* OS::AllocateShared(void* hint, size_t size, MemoryPermission permission, ...@@ -957,15 +957,15 @@ void* OS::AllocateShared(void* hint, size_t size, MemoryPermission permission,
} }
// static // static
bool OS::FreeShared(void* address, size_t size) { void OS::FreeShared(void* address, size_t size) {
return UnmapViewOfFile(address); CHECK(UnmapViewOfFile(address));
} }
// static // static
bool OS::Release(void* address, size_t size) { void OS::Release(void* address, size_t size) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize()); DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize());
DCHECK_EQ(0, size % CommitPageSize()); DCHECK_EQ(0, size % CommitPageSize());
return VirtualFree(address, size, MEM_DECOMMIT) != 0; CHECK_NE(0, VirtualFree(address, size, MEM_DECOMMIT));
} }
// static // static
...@@ -1049,8 +1049,8 @@ Optional<AddressSpaceReservation> OS::CreateAddressSpaceReservation( ...@@ -1049,8 +1049,8 @@ Optional<AddressSpaceReservation> OS::CreateAddressSpaceReservation(
} }
// static // static
bool OS::FreeAddressSpaceReservation(AddressSpaceReservation reservation) { void OS::FreeAddressSpaceReservation(AddressSpaceReservation reservation) {
return OS::Free(reservation.base(), reservation.size()); OS::Free(reservation.base(), reservation.size());
} }
// static // static
......
...@@ -341,15 +341,15 @@ class V8_BASE_EXPORT OS { ...@@ -341,15 +341,15 @@ class V8_BASE_EXPORT OS {
void* new_address, void* new_address,
size_t size); size_t size);
V8_WARN_UNUSED_RESULT static bool Free(void* address, size_t size); static void Free(void* address, size_t size);
V8_WARN_UNUSED_RESULT static void* AllocateShared( V8_WARN_UNUSED_RESULT static void* AllocateShared(
void* address, size_t size, OS::MemoryPermission access, void* address, size_t size, OS::MemoryPermission access,
PlatformSharedMemoryHandle handle, uint64_t offset); PlatformSharedMemoryHandle handle, uint64_t offset);
V8_WARN_UNUSED_RESULT static bool FreeShared(void* address, size_t size); static void FreeShared(void* address, size_t size);
V8_WARN_UNUSED_RESULT static bool Release(void* address, size_t size); static void Release(void* address, size_t size);
V8_WARN_UNUSED_RESULT static bool SetPermissions(void* address, size_t size, V8_WARN_UNUSED_RESULT static bool SetPermissions(void* address, size_t size,
MemoryPermission access); MemoryPermission access);
...@@ -365,8 +365,7 @@ class V8_BASE_EXPORT OS { ...@@ -365,8 +365,7 @@ class V8_BASE_EXPORT OS {
CreateAddressSpaceReservation(void* hint, size_t size, size_t alignment, CreateAddressSpaceReservation(void* hint, size_t size, size_t alignment,
MemoryPermission max_permission); MemoryPermission max_permission);
V8_WARN_UNUSED_RESULT static bool FreeAddressSpaceReservation( static void FreeAddressSpaceReservation(AddressSpaceReservation reservation);
AddressSpaceReservation reservation);
static const int msPerSecond = 1000; static const int msPerSecond = 1000;
......
...@@ -89,7 +89,8 @@ bool VirtualAddressSpace::FreePages(Address address, size_t size) { ...@@ -89,7 +89,8 @@ bool VirtualAddressSpace::FreePages(Address address, size_t size) {
DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(address, allocation_granularity()));
DCHECK(IsAligned(size, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity()));
return OS::Free(reinterpret_cast<void*>(address), size); OS::Free(reinterpret_cast<void*>(address), size);
return true;
} }
bool VirtualAddressSpace::SetPagePermissions(Address address, size_t size, bool VirtualAddressSpace::SetPagePermissions(Address address, size_t size,
...@@ -109,7 +110,7 @@ bool VirtualAddressSpace::AllocateGuardRegion(Address address, size_t size) { ...@@ -109,7 +110,7 @@ bool VirtualAddressSpace::AllocateGuardRegion(Address address, size_t size) {
void* result = OS::Allocate(hint, size, allocation_granularity(), void* result = OS::Allocate(hint, size, allocation_granularity(),
OS::MemoryPermission::kNoAccess); OS::MemoryPermission::kNoAccess);
if (result && result != hint) { if (result && result != hint) {
CHECK(OS::Free(result, size)); OS::Free(result, size);
} }
return result == hint; return result == hint;
} }
...@@ -118,7 +119,8 @@ bool VirtualAddressSpace::FreeGuardRegion(Address address, size_t size) { ...@@ -118,7 +119,8 @@ bool VirtualAddressSpace::FreeGuardRegion(Address address, size_t size) {
DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(address, allocation_granularity()));
DCHECK(IsAligned(size, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity()));
return OS::Free(reinterpret_cast<void*>(address), size); OS::Free(reinterpret_cast<void*>(address), size);
return true;
} }
bool VirtualAddressSpace::CanAllocateSubspaces() { bool VirtualAddressSpace::CanAllocateSubspaces() {
...@@ -141,7 +143,8 @@ bool VirtualAddressSpace::FreeSharedPages(Address address, size_t size) { ...@@ -141,7 +143,8 @@ bool VirtualAddressSpace::FreeSharedPages(Address address, size_t size) {
DCHECK(IsAligned(address, allocation_granularity())); DCHECK(IsAligned(address, allocation_granularity()));
DCHECK(IsAligned(size, allocation_granularity())); DCHECK(IsAligned(size, allocation_granularity()));
return OS::FreeShared(reinterpret_cast<void*>(address), size); OS::FreeShared(reinterpret_cast<void*>(address), size);
return true;
} }
std::unique_ptr<v8::VirtualAddressSpace> VirtualAddressSpace::AllocateSubspace( std::unique_ptr<v8::VirtualAddressSpace> VirtualAddressSpace::AllocateSubspace(
...@@ -176,7 +179,8 @@ bool VirtualAddressSpace::DecommitPages(Address address, size_t size) { ...@@ -176,7 +179,8 @@ bool VirtualAddressSpace::DecommitPages(Address address, size_t size) {
} }
bool VirtualAddressSpace::FreeSubspace(VirtualAddressSubspace* subspace) { bool VirtualAddressSpace::FreeSubspace(VirtualAddressSubspace* subspace) {
return OS::FreeAddressSpaceReservation(subspace->reservation_); OS::FreeAddressSpaceReservation(subspace->reservation_);
return true;
} }
VirtualAddressSubspace::VirtualAddressSubspace( VirtualAddressSubspace::VirtualAddressSubspace(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment