Commit bc2056c3 authored by Will Harris's avatar Will Harris Committed by Commit Bot

Change size in memory mapped region to size_t

Subsequently, remove some static_casts that are no longer needed.

On Windows, while it is possible to create a file mapping of a file >
32-bits long it is not possible to create the view to the file
mapping, so changing this to size_t should have no loss of functionality.

BUG=778316

Change-Id: I81cd2e0808794fdb83fc21c796367369ea2da220
Reviewed-on: https://chromium-review.googlesource.com/755114Reviewed-by: default avatarMark Mentovai <mark@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515257}
parent ad4a09f2
...@@ -1280,12 +1280,10 @@ bool GlobalActivityTracker::CreateWithFile(const FilePath& file_path, ...@@ -1280,12 +1280,10 @@ bool GlobalActivityTracker::CreateWithFile(const FilePath& file_path,
// Create and map the file into memory and make it globally available. // Create and map the file into memory and make it globally available.
std::unique_ptr<MemoryMappedFile> mapped_file(new MemoryMappedFile()); std::unique_ptr<MemoryMappedFile> mapped_file(new MemoryMappedFile());
bool success = bool success = mapped_file->Initialize(
mapped_file->Initialize(File(file_path, File(file_path, File::FLAG_CREATE_ALWAYS | File::FLAG_READ |
File::FLAG_CREATE_ALWAYS | File::FLAG_READ | File::FLAG_WRITE | File::FLAG_SHARE_DELETE),
File::FLAG_WRITE | File::FLAG_SHARE_DELETE), {0, size}, MemoryMappedFile::READ_WRITE_EXTEND);
{0, static_cast<int64_t>(size)},
MemoryMappedFile::READ_WRITE_EXTEND);
if (!success) if (!success)
return false; return false;
if (!FilePersistentMemoryAllocator::IsFileAcceptable(*mapped_file, false)) if (!FilePersistentMemoryAllocator::IsFileAcceptable(*mapped_file, false))
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/numerics/safe_math.h"
#include "base/sys_info.h" #include "base/sys_info.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -72,10 +73,13 @@ bool MemoryMappedFile::Initialize(File file, ...@@ -72,10 +73,13 @@ bool MemoryMappedFile::Initialize(File file,
switch (access) { switch (access) {
case READ_WRITE_EXTEND: case READ_WRITE_EXTEND:
DCHECK(Region::kWholeFile != region); DCHECK(Region::kWholeFile != region);
// Ensure that the extended size is within limits of File. {
if (region.size > std::numeric_limits<int64_t>::max() - region.offset) { CheckedNumeric<int64_t> region_end(region.offset);
DLOG(ERROR) << "Region bounds exceed maximum for base::File."; region_end += region.size;
return false; if (!region_end.IsValid()) {
DLOG(ERROR) << "Region bounds exceed maximum for base::File.";
return false;
}
} }
// Fall through. // Fall through.
case READ_ONLY: case READ_ONLY:
...@@ -91,10 +95,8 @@ bool MemoryMappedFile::Initialize(File file, ...@@ -91,10 +95,8 @@ bool MemoryMappedFile::Initialize(File file,
if (IsValid()) if (IsValid())
return false; return false;
if (region != Region::kWholeFile) { if (region != Region::kWholeFile)
DCHECK_GE(region.offset, 0); DCHECK_GE(region.offset, 0);
DCHECK_GT(region.size, 0);
}
file_ = std::move(file); file_ = std::move(file);
...@@ -112,18 +114,17 @@ bool MemoryMappedFile::IsValid() const { ...@@ -112,18 +114,17 @@ bool MemoryMappedFile::IsValid() const {
// static // static
void MemoryMappedFile::CalculateVMAlignedBoundaries(int64_t start, void MemoryMappedFile::CalculateVMAlignedBoundaries(int64_t start,
int64_t size, size_t size,
int64_t* aligned_start, int64_t* aligned_start,
int64_t* aligned_size, size_t* aligned_size,
int32_t* offset) { int32_t* offset) {
// Sadly, on Windows, the mmap alignment is not just equal to the page size. // Sadly, on Windows, the mmap alignment is not just equal to the page size.
const int64_t mask = auto mask = SysInfo::VMAllocationGranularity() - 1;
static_cast<int64_t>(SysInfo::VMAllocationGranularity()) - 1; DCHECK(IsValueInRangeForNumericType<int32_t>(mask));
DCHECK_LT(mask, std::numeric_limits<int32_t>::max());
*offset = start & mask; *offset = start & mask;
*aligned_start = start & ~mask; *aligned_start = start & ~mask;
*aligned_size = (size + *offset + mask) & ~mask; *aligned_size = (size + *offset + mask) & ~mask;
} }
#endif #endif // !defined(OS_NACL)
} // namespace base } // namespace base
...@@ -61,7 +61,7 @@ class BASE_EXPORT MemoryMappedFile { ...@@ -61,7 +61,7 @@ class BASE_EXPORT MemoryMappedFile {
int64_t offset; int64_t offset;
// Length of the region in bytes. // Length of the region in bytes.
int64_t size; size_t size;
}; };
// Opens an existing file and maps it into memory. |access| can be read-only // Opens an existing file and maps it into memory. |access| can be read-only
...@@ -108,9 +108,9 @@ class BASE_EXPORT MemoryMappedFile { ...@@ -108,9 +108,9 @@ class BASE_EXPORT MemoryMappedFile {
// - |aligned_size| is a multiple of the VM granularity and >= |size|. // - |aligned_size| is a multiple of the VM granularity and >= |size|.
// - |offset| is the displacement of |start| w.r.t |aligned_start|. // - |offset| is the displacement of |start| w.r.t |aligned_start|.
static void CalculateVMAlignedBoundaries(int64_t start, static void CalculateVMAlignedBoundaries(int64_t start,
int64_t size, size_t size,
int64_t* aligned_start, int64_t* aligned_start,
int64_t* aligned_size, size_t* aligned_size,
int32_t* offset); int32_t* offset);
// Map the file to memory, set data_ to that memory address. Return true on // Map the file to memory, set data_ to that memory address. Return true on
......
...@@ -40,6 +40,8 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -40,6 +40,8 @@ bool MemoryMappedFile::MapFileRegionToMemory(
DPLOG(ERROR) << "fstat " << file_.GetPlatformFile(); DPLOG(ERROR) << "fstat " << file_.GetPlatformFile();
return false; return false;
} }
if (!IsValueInRangeForNumericType<size_t>(static_cast<uint64_t>(file_len)))
return false;
map_size = static_cast<size_t>(file_len); map_size = static_cast<size_t>(file_len);
length_ = map_size; length_ = map_size;
} else { } else {
...@@ -48,7 +50,7 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -48,7 +50,7 @@ bool MemoryMappedFile::MapFileRegionToMemory(
// outer region [|aligned_start|, |aligned_start| + |size|] which contains // outer region [|aligned_start|, |aligned_start| + |size|] which contains
// |region| and then add up the |data_offset| displacement. // |region| and then add up the |data_offset| displacement.
int64_t aligned_start = 0; int64_t aligned_start = 0;
int64_t aligned_size = 0; size_t aligned_size = 0;
CalculateVMAlignedBoundaries(region.offset, CalculateVMAlignedBoundaries(region.offset,
region.size, region.size,
&aligned_start, &aligned_start,
...@@ -56,14 +58,15 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -56,14 +58,15 @@ bool MemoryMappedFile::MapFileRegionToMemory(
&data_offset); &data_offset);
// Ensure that the casts in the mmap call below are sane. // Ensure that the casts in the mmap call below are sane.
if (aligned_start < 0 || aligned_size < 0) { if (aligned_start < 0 ||
!IsValueInRangeForNumericType<off_t>(aligned_start)) {
DLOG(ERROR) << "Region bounds are not valid for mmap"; DLOG(ERROR) << "Region bounds are not valid for mmap";
return false; return false;
} }
map_start = base::checked_cast<off_t>(aligned_start); map_start = static_cast<off_t>(aligned_start);
map_size = base::checked_cast<size_t>(aligned_size); map_size = aligned_size;
length_ = base::checked_cast<size_t>(region.size); length_ = region.size;
} }
int flags = 0; int flags = 0;
......
...@@ -27,8 +27,7 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -27,8 +27,7 @@ bool MemoryMappedFile::MapFileRegionToMemory(
return false; return false;
int flags = 0; int flags = 0;
uint32_t size_low = 0; ULARGE_INTEGER size = {};
uint32_t size_high = 0;
switch (access) { switch (access) {
case READ_ONLY: case READ_ONLY:
flags |= PAGE_READONLY; flags |= PAGE_READONLY;
...@@ -38,13 +37,12 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -38,13 +37,12 @@ bool MemoryMappedFile::MapFileRegionToMemory(
break; break;
case READ_WRITE_EXTEND: case READ_WRITE_EXTEND:
flags |= PAGE_READWRITE; flags |= PAGE_READWRITE;
size_high = static_cast<uint32_t>(region.size >> 32); size.QuadPart = region.size;
size_low = static_cast<uint32_t>(region.size & 0xFFFFFFFF);
break; break;
} }
file_mapping_.Set(::CreateFileMapping(file_.GetPlatformFile(), NULL, flags, file_mapping_.Set(::CreateFileMapping(file_.GetPlatformFile(), NULL, flags,
size_high, size_low, NULL)); size.HighPart, size.LowPart, NULL));
if (!file_mapping_.IsValid()) if (!file_mapping_.IsValid())
return false; return false;
...@@ -55,7 +53,8 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -55,7 +53,8 @@ bool MemoryMappedFile::MapFileRegionToMemory(
if (region == MemoryMappedFile::Region::kWholeFile) { if (region == MemoryMappedFile::Region::kWholeFile) {
DCHECK_NE(READ_WRITE_EXTEND, access); DCHECK_NE(READ_WRITE_EXTEND, access);
int64_t file_len = file_.GetLength(); int64_t file_len = file_.GetLength();
if (file_len <= 0 || file_len > std::numeric_limits<int32_t>::max()) if (file_len <= 0 ||
!IsValueInRangeForNumericType<size_t>(static_cast<uint64_t>(file_len)))
return false; return false;
length_ = static_cast<size_t>(file_len); length_ = static_cast<size_t>(file_len);
} else { } else {
...@@ -67,20 +66,20 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -67,20 +66,20 @@ bool MemoryMappedFile::MapFileRegionToMemory(
// We map here the outer region [|aligned_start|, |aligned_start+size|] // We map here the outer region [|aligned_start|, |aligned_start+size|]
// which contains |region| and then add up the |data_offset| displacement. // which contains |region| and then add up the |data_offset| displacement.
int64_t aligned_start = 0; int64_t aligned_start = 0;
int64_t ignored = 0; size_t ignored = 0U;
CalculateVMAlignedBoundaries( CalculateVMAlignedBoundaries(
region.offset, region.size, &aligned_start, &ignored, &data_offset); region.offset, region.size, &aligned_start, &ignored, &data_offset);
int64_t size = region.size + data_offset; int64_t size = region.size + data_offset;
// Ensure that the casts below in the MapViewOfFile call are sane. // Ensure that the casts below in the MapViewOfFile call are sane.
if (aligned_start < 0 || size < 0 || if (aligned_start < 0 || size < 0 ||
static_cast<uint64_t>(size) > std::numeric_limits<SIZE_T>::max()) { !IsValueInRangeForNumericType<SIZE_T>(static_cast<uint64_t>(size))) {
DLOG(ERROR) << "Region bounds are not valid for MapViewOfFile"; DLOG(ERROR) << "Region bounds are not valid for MapViewOfFile";
return false; return false;
} }
map_start.QuadPart = aligned_start; map_start.QuadPart = aligned_start;
map_size = static_cast<SIZE_T>(size); map_size = static_cast<SIZE_T>(size);
length_ = static_cast<size_t>(region.size); length_ = region.size;
} }
data_ = static_cast<uint8_t*>( data_ = static_cast<uint8_t*>(
......
...@@ -787,7 +787,7 @@ bool GlobalHistogramAllocator::CreateWithFile( ...@@ -787,7 +787,7 @@ bool GlobalHistogramAllocator::CreateWithFile(
size = saturated_cast<size_t>(file.GetLength()); size = saturated_cast<size_t>(file.GetLength());
mmfile->Initialize(std::move(file), MemoryMappedFile::READ_WRITE); mmfile->Initialize(std::move(file), MemoryMappedFile::READ_WRITE);
} else { } else {
mmfile->Initialize(std::move(file), {0, static_cast<int64_t>(size)}, mmfile->Initialize(std::move(file), {0, size},
MemoryMappedFile::READ_WRITE_EXTEND); MemoryMappedFile::READ_WRITE_EXTEND);
} }
if (!mmfile->IsValid() || if (!mmfile->IsValid() ||
...@@ -941,7 +941,7 @@ bool GlobalHistogramAllocator::CreateSpareFile(const FilePath& spare_path, ...@@ -941,7 +941,7 @@ bool GlobalHistogramAllocator::CreateSpareFile(const FilePath& spare_path,
return false; return false;
MemoryMappedFile mmfile; MemoryMappedFile mmfile;
mmfile.Initialize(std::move(spare_file), {0, static_cast<int64_t>(size)}, mmfile.Initialize(std::move(spare_file), {0, size},
MemoryMappedFile::READ_WRITE_EXTEND); MemoryMappedFile::READ_WRITE_EXTEND);
success = mmfile.IsValid(); success = mmfile.IsValid();
} }
......
...@@ -40,9 +40,8 @@ MappedFileWriter::MappedFileWriter(const base::FilePath& file_path, ...@@ -40,9 +40,8 @@ MappedFileWriter::MappedFileWriter(const base::FilePath& file_path,
} }
#endif // defined(OS_WIN) #endif // defined(OS_WIN)
bool is_ok = bool is_ok = buffer_.Initialize(std::move(file), {0, length},
buffer_.Initialize(std::move(file), {0, static_cast<int64_t>(length)}, base::MemoryMappedFile::READ_WRITE_EXTEND);
base::MemoryMappedFile::READ_WRITE_EXTEND);
LOG_IF(ERROR, !is_ok) << "Can't map file: " << file_path_.value(); LOG_IF(ERROR, !is_ok) << "Can't map file: " << file_path_.value();
} }
......
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