Make SharedMemory track the size that was actually mapped

In the process I also had to fix several dependencies, and
some major issues in transport DIBs.

BUG=60819

Review URL: https://chromiumcodereview.appspot.com/12537014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191098 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 74 insertions, 46 deletions

diff --git a/base/shared_memory.h b/base/shared_memory.h
index da6f5b7b..4600194 100644
--- a/base/shared_memory.h
+++ b/base/shared_memory.h
@@ -159,15 +159,11 @@ class BASE_EXPORT SharedMemory {
   // memory is not mapped.
   bool Unmap();
 
-  // Get the size of the shared memory backing file.
-  // Note:  This size is only available to the creator of the
-  // shared memory, and not to those that opened shared memory
-  // created externally.
-  // Returns 0 if not created or unknown.
-  // Deprecated method, please keep track of the size yourself if you created
-  // it.
-  // http://crbug.com/60821
-  size_t created_size() const { return created_size_; }
+  // The size requested when the map is first created.
+  size_t requested_size() const { return requested_size_; }
+
+  // The actual size of the mapped memory (may be larger than requested).
+  size_t mapped_size() const { return mapped_size_; }
 
   // Gets a pointer to the opened memory space if it has been
   // Mapped via Map().  Returns NULL if it is not mapped.
@@ -246,12 +242,12 @@ class BASE_EXPORT SharedMemory {
   HANDLE             mapped_file_;
 #elif defined(OS_POSIX)
   int                mapped_file_;
-  size_t             mapped_size_;
   ino_t              inode_;
 #endif
+  size_t             mapped_size_;
   void*              memory_;
   bool               read_only_;
-  size_t             created_size_;
+  size_t             requested_size_;
 #if !defined(OS_POSIX)
   SharedMemoryLock   lock_;
 #endif
diff --git a/base/shared_memory_android.cc b/base/shared_memory_android.cc
index e2c683c..084904a 100644
--- a/base/shared_memory_android.cc
+++ b/base/shared_memory_android.cc
@@ -37,7 +37,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
     DLOG(ERROR) << "Error " << err << " when setting protection of ashmem";
     return false;
   }
-  created_size_ = options.size;
+  requested_size_ = options.size;
 
   return true;
 }
diff --git a/base/shared_memory_nacl.cc b/base/shared_memory_nacl.cc
index 291527e..5522b8f 100644
--- a/base/shared_memory_nacl.cc
+++ b/base/shared_memory_nacl.cc
@@ -18,30 +18,30 @@ namespace base {
 
 SharedMemory::SharedMemory()
     : mapped_file_(-1),
-      mapped_size_(0),
       inode_(0),
+      mapped_size_(0),
       memory_(NULL),
       read_only_(false),
-      created_size_(0) {
+      requested_size_(0) {
 }
 
 SharedMemory::SharedMemory(SharedMemoryHandle handle, bool read_only)
     : mapped_file_(handle.fd),
-      mapped_size_(0),
       inode_(0),
+      mapped_size_(0),
       memory_(NULL),
       read_only_(read_only),
-      created_size_(0) {
+      requested_size_(0) {
 }
 
 SharedMemory::SharedMemory(SharedMemoryHandle handle, bool read_only,
                            ProcessHandle process)
     : mapped_file_(handle.fd),
-      mapped_size_(0),
       inode_(0),
+      mapped_size_(0),
       memory_(NULL),
       read_only_(read_only),
-      created_size_(0) {
+      requested_size_(0) {
   NOTREACHED();
 }
 
diff --git a/base/shared_memory_posix.cc b/base/shared_memory_posix.cc
index 25d141f..607d665 100644
--- a/base/shared_memory_posix.cc
+++ b/base/shared_memory_posix.cc
@@ -42,20 +42,20 @@ LazyInstance<Lock>::Leaky g_thread_lock_ = LAZY_INSTANCE_INITIALIZER;
 
 SharedMemory::SharedMemory()
     : mapped_file_(-1),
-      mapped_size_(0),
       inode_(0),
+      mapped_size_(0),
       memory_(NULL),
       read_only_(false),
-      created_size_(0) {
+      requested_size_(0) {
 }
 
 SharedMemory::SharedMemory(SharedMemoryHandle handle, bool read_only)
     : mapped_file_(handle.fd),
-      mapped_size_(0),
       inode_(0),
+      mapped_size_(0),
       memory_(NULL),
       read_only_(read_only),
-      created_size_(0) {
+      requested_size_(0) {
   struct stat st;
   if (fstat(handle.fd, &st) == 0) {
     // If fstat fails, then the file descriptor is invalid and we'll learn this
@@ -67,11 +67,11 @@ SharedMemory::SharedMemory(SharedMemoryHandle handle, bool read_only)
 SharedMemory::SharedMemory(SharedMemoryHandle handle, bool read_only,
                            ProcessHandle process)
     : mapped_file_(handle.fd),
-      mapped_size_(0),
       inode_(0),
+      mapped_size_(0),
       memory_(NULL),
       read_only_(read_only),
-      created_size_(0) {
+      requested_size_(0) {
   // We don't handle this case yet (note the ignored parameter); let's die if
   // someone comes calling.
   NOTREACHED();
@@ -164,7 +164,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
         return false;
       }
     }
-    created_size_ = options.size;
+    requested_size_ = options.size;
   }
   if (fp == NULL) {
 #if !defined(OS_MACOSX)
@@ -235,7 +235,7 @@ bool SharedMemory::MapAt(off_t offset, size_t bytes) {
     // TODO(port): we set the created size here so that it is available in
     // transport_dib_android.cc. We should use ashmem_get_size_region()
     // in transport_dib_android.cc.
-    created_size_ = ashmem_bytes;
+    mapped_size_ = ashmem_bytes;
   }
 #endif
 
@@ -244,7 +244,9 @@ bool SharedMemory::MapAt(off_t offset, size_t bytes) {
 
   bool mmap_succeeded = memory_ != (void*)-1 && memory_ != NULL;
   if (mmap_succeeded) {
+#if !defined(OS_ANDROID)
     mapped_size_ = bytes;
+#endif
     DCHECK_EQ(0U, reinterpret_cast<uintptr_t>(memory_) &
         (SharedMemory::MAP_MINIMUM_ALIGNMENT - 1));
   } else {
diff --git a/base/shared_memory_unittest.cc b/base/shared_memory_unittest.cc
index 3f83248..a9318ad 100644
--- a/base/shared_memory_unittest.cc
+++ b/base/shared_memory_unittest.cc
@@ -197,11 +197,18 @@ TEST(SharedMemoryTest, OpenExclusive) {
   EXPECT_TRUE(rv);
 
   // Memory1 knows it's size because it created it.
-  EXPECT_EQ(memory1.created_size(), kDataSize);
+  EXPECT_EQ(memory1.requested_size(), kDataSize);
 
   rv = memory1.Map(kDataSize);
   EXPECT_TRUE(rv);
 
+  // The mapped memory1 must be at least the size we asked for.
+  EXPECT_GE(memory1.mapped_size(), kDataSize);
+
+  // The mapped memory1 shouldn't exceed rounding for allocation granularity.
+  EXPECT_LT(memory1.mapped_size(),
+            kDataSize + base::SysInfo::VMAllocationGranularity());
+
   memset(memory1.memory(), 'G', kDataSize);
 
   SharedMemory memory2;
@@ -214,12 +221,19 @@ TEST(SharedMemoryTest, OpenExclusive) {
   EXPECT_TRUE(rv);
 
   // Memory2 shouldn't know the size because we didn't create it.
-  EXPECT_EQ(memory2.created_size(), 0U);
+  EXPECT_EQ(memory2.requested_size(), 0U);
 
   // We should be able to map the original size.
   rv = memory2.Map(kDataSize);
   EXPECT_TRUE(rv);
 
+  // The mapped memory2 must be at least the size of the original.
+  EXPECT_GE(memory2.mapped_size(), kDataSize);
+
+  // The mapped memory2 shouldn't exceed rounding for allocation granularity.
+  EXPECT_LT(memory2.mapped_size(),
+            kDataSize2 + base::SysInfo::VMAllocationGranularity());
+
   // Verify that opening memory2 didn't truncate or delete memory 1.
   char *start_ptr = static_cast<char *>(memory2.memory());
   char *end_ptr = start_ptr + kDataSize;
@@ -382,9 +396,9 @@ TEST(SharedMemoryTest, AnonymousExecutable) {
   options.executable = true;
 
   EXPECT_TRUE(shared_memory.Create(options));
-  EXPECT_TRUE(shared_memory.Map(shared_memory.created_size()));
+  EXPECT_TRUE(shared_memory.Map(shared_memory.requested_size()));
 
-  EXPECT_EQ(0, mprotect(shared_memory.memory(), shared_memory.created_size(),
+  EXPECT_EQ(0, mprotect(shared_memory.memory(), shared_memory.requested_size(),
                         PROT_READ | PROT_EXEC));
 }
 #endif
diff --git a/base/shared_memory_win.cc b/base/shared_memory_win.cc
index f64e3c8..defad6e 100644
--- a/base/shared_memory_win.cc
+++ b/base/shared_memory_win.cc
@@ -7,13 +7,27 @@
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
 
+namespace {
+
+// Returns the length of the memory section starting at the supplied address.
+size_t GetMemorySectionSize(void* address) {
+  MEMORY_BASIC_INFORMATION memory_info;
+  if (!::VirtualQuery(address, &memory_info, sizeof(memory_info)))
+    return 0;
+  return memory_info.RegionSize - (static_cast<char*>(address) -
+         static_cast<char*>(memory_info.AllocationBase));
+}
+
+}  // namespace.
+
 namespace base {
 
 SharedMemory::SharedMemory()
     : mapped_file_(NULL),
       memory_(NULL),
       read_only_(false),
-      created_size_(0),
+      mapped_size_(0),
+      requested_size_(0),
       lock_(NULL) {
 }
 
@@ -21,7 +35,8 @@ SharedMemory::SharedMemory(const std::wstring& name)
     : mapped_file_(NULL),
       memory_(NULL),
       read_only_(false),
-      created_size_(0),
+      requested_size_(0),
+      mapped_size_(0),
       lock_(NULL),
       name_(name) {
 }
@@ -30,7 +45,8 @@ SharedMemory::SharedMemory(SharedMemoryHandle handle, bool read_only)
     : mapped_file_(handle),
       memory_(NULL),
       read_only_(read_only),
-      created_size_(0),
+      requested_size_(0),
+      mapped_size_(0),
       lock_(NULL) {
 }
 
@@ -39,7 +55,8 @@ SharedMemory::SharedMemory(SharedMemoryHandle handle, bool read_only,
     : mapped_file_(NULL),
       memory_(NULL),
       read_only_(read_only),
-      created_size_(0),
+      requested_size_(0),
+      mapped_size_(0),
       lock_(NULL) {
   ::DuplicateHandle(process, handle,
                     GetCurrentProcess(), &mapped_file_,
@@ -75,22 +92,20 @@ bool SharedMemory::CreateAndMapAnonymous(size_t size) {
 }
 
 bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
+  // TODO(bsy,sehr): crbug.com/210609 NaCl forces us to round up 64k here,
+  // wasting 32k per mapping on average.
+  static const size_t kSectionMask = 65536 - 1;
   DCHECK(!options.executable);
   DCHECK(!mapped_file_);
   if (options.size == 0)
     return false;
 
-  if (options.size > static_cast<size_t>(std::numeric_limits<int>::max()))
+  // Check maximum accounting for overflow.
+  if (options.size >
+      static_cast<size_t>(std::numeric_limits<int>::max()) - kSectionMask)
     return false;
 
-  // NaCl's memory allocator requires 0mod64K alignment and size for
-  // shared memory objects.  To allow passing shared memory to NaCl,
-  // therefore we round the size actually created to the nearest 64K unit.
-  // To avoid client impact, we continue to retain the size as the
-  // actual requested size.
-  uint32 rounded_size = (options.size + 0xffff) & ~0xffff;
-  if (rounded_size < options.size)
-    return false;
+  size_t rounded_size = (options.size + kSectionMask) & ~kSectionMask;
   name_ = ASCIIToWide(options.name == NULL ? "" : *options.name);
   mapped_file_ = CreateFileMapping(INVALID_HANDLE_VALUE, NULL,
       PAGE_READWRITE, 0, static_cast<DWORD>(rounded_size),
@@ -98,13 +113,13 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
   if (!mapped_file_)
     return false;
 
-  created_size_ = options.size;
+  requested_size_ = options.size;
 
   // Check if the shared memory pre-exists.
   if (GetLastError() == ERROR_ALREADY_EXISTS) {
-    // If the file already existed, set created_size_ to 0 to show that
+    // If the file already existed, set requested_size_ to 0 to show that
     // we don't know the size.
-    created_size_ = 0;
+    requested_size_ = 0;
     if (!options.open_existing) {
       Close();
       return false;
@@ -149,6 +164,7 @@ bool SharedMemory::MapAt(off_t offset, size_t bytes) {
   if (memory_ != NULL) {
     DCHECK_EQ(0U, reinterpret_cast<uintptr_t>(memory_) &
         (SharedMemory::MAP_MINIMUM_ALIGNMENT - 1));
+    mapped_size_ = GetMemorySectionSize(memory_);
     return true;
   }
   return false;