Bug 1536697 - Fix error handling in base::SharedMemory::Map. r=froydnj
authorJed Davis <jld@mozilla.com>
Wed, 14 Aug 2019 22:48:51 +0000
changeset 488053 86cb672b7000844c4802bc890e7b759e42e0e722
parent 488052 87737e44c8b7537b159dfa0422ee599f85caf2c3
child 488054 d8ac382b5f1790a5da7e2da12e4e1c92c39bb22b
push id36434
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:44:30 +0000
treeherdermozilla-central@144fbfb409b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1536697
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1536697 - Fix error handling in base::SharedMemory::Map. r=froydnj If mmap failed, we'd leave the memory_ member variable set to MAP_FAILED, but everything else in this file checks for nullptr (and only nullptr) to test if the pointer is valid. Also, this removes the debug assertion that the mmap succeeded, to allow writing unit tests where we expect it to fail (e.g., for insufficient permissions). Depends on D26747 Differential Revision: https://phabricator.services.mozilla.com/D26748
ipc/chromium/src/base/shared_memory_posix.cc
--- a/ipc/chromium/src/base/shared_memory_posix.cc
+++ b/ipc/chromium/src/base/shared_memory_posix.cc
@@ -285,39 +285,38 @@ bool SharedMemory::Freeze() {
 
   read_only_ = true;
   freezeable_ = false;
   return true;
 }
 
 bool SharedMemory::Map(size_t bytes, void* fixed_address) {
   if (mapped_file_ == -1) return false;
+  DCHECK(!memory_);
 
   // Don't use MAP_FIXED when a fixed_address was specified, since that can
   // replace pages that are alread mapped at that address.
-  memory_ =
+  void* mem =
       mmap(fixed_address, bytes, PROT_READ | (read_only_ ? 0 : PROT_WRITE),
            MAP_SHARED, mapped_file_, 0);
 
-  bool mmap_succeeded = memory_ != MAP_FAILED;
-
-  DCHECK(mmap_succeeded) << "Call to mmap failed, errno=" << errno;
-
-  if (mmap_succeeded) {
-    if (fixed_address && memory_ != fixed_address) {
-      bool munmap_succeeded = munmap(memory_, bytes) == 0;
-      DCHECK(munmap_succeeded) << "Call to munmap failed, errno=" << errno;
-      memory_ = NULL;
-      return false;
-    }
-
-    mapped_size_ = bytes;
+  if (mem == MAP_FAILED) {
+    CHROMIUM_LOG(WARNING) << "Call to mmap failed: " << strerror(errno);
+    return false;
   }
 
-  return mmap_succeeded;
+  if (fixed_address && mem != fixed_address) {
+    bool munmap_succeeded = munmap(mem, bytes) == 0;
+    DCHECK(munmap_succeeded) << "Call to munmap failed, errno=" << errno;
+    return false;
+  }
+
+  memory_ = mem;
+  mapped_size_ = bytes;
+  return true;
 }
 
 bool SharedMemory::Unmap() {
   if (memory_ == NULL) return false;
 
   munmap(memory_, mapped_size_);
   memory_ = NULL;
   mapped_size_ = 0;