Ensure that memory-maped files are fully realized.

base/files/memory_mapped_file_posix.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/files/memory_mapped_file.h"
 
+#define _GNU_SOURCE 1  // Needed to get fallocate out of fcntl.h.
+
+#include <fcntl.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
 #include "base/logging.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 
@@ skipping 47 matching lines @@
     map_start = static_cast<off_t>(aligned_start);
     map_size = static_cast<size_t>(aligned_size);
     length_ = static_cast<size_t>(region.size);
   }
 
   int flags = 0;
   switch (access) {
     case READ_ONLY:
       flags |= PROT_READ;
       break;
+
     case READ_WRITE:
       flags |= PROT_READ | PROT_WRITE;
       break;
+
     case READ_WRITE_EXTEND:
       // POSIX won't auto-extend the file when it is written so it must first
       // be explicitly extended to the maximum size. Zeros will fill the new
       // space.
       auto file_len = file_.GetLength();
       if (file_len < 0) {
         DPLOG(ERROR) << "fstat " << file_.GetPlatformFile();
         return false;
       }
-      file_.SetLength(std::max(file_len, region.offset + region.size));
       flags |= PROT_READ | PROT_WRITE;
+
+      // Increase the actual length of the file, if necessary. This can fail if
+      // the disk is full and the OS doesn't support sparse files.
+      if (!file_.SetLength(std::max(file_len, region.offset + region.size))) {

[Mark Mentovai, 2017/05/04 21:00:47]

This isn’t necessary if you’re going to call [posix_]fallocate(). You can move
it into the OS_MACOSX case, or (depending on how you handle fallocate() failure)
you can use it as a fallback if fallocate() fails with EOPNOTSUPP.

[bcwhite, 2017/05/05 18:06:59]

Acknowledged.

+        DPLOG(ERROR) << "ftruncate " << file_.GetPlatformFile();
+        return false;
+      }
+
+// Realize the extent of the file so that it can't fail (and crash) later

[Mark Mentovai, 2017/05/04 21:00:47]

This is one of clang-format’s problems, these comments were probably intended to
have been bumped over to line up with the code, not the #if.

[bcwhite, 2017/05/05 18:06:59]

Done.

+// when trying to write to a memory page that can't be created. This can
+// fail if the disk is full and the file is sparse.
+#if defined(OS_MACOSX)
+// Mac OSX doesn't support this call but the primary filesystem doesn't
+// support sparse files so is unneeded.
+#elif defined(OS_ANDROID)
+      // Android doesn't support the POSIX call so use the native Linux one.

[Mark Mentovai, 2017/05/04 21:00:46]

Curious about this. It looks like both fallocate() and posix_fallocate() were
added to Bionic in
https://android.googlesource.com/platform/bionic/+/f64b8ea09db3bdd84eed59f772....
Surprised that you found that one worked but the other didn’t. I’d expect either
both to be present, or both not to be.

Since they both showed up in API 21 (Android 5.0/“L”), I’d expect this to work
for our 64-bit Android builds (for which we target API 21), but not to for our
32-bit builds (for which we target API 16). The trybots ought to know for sure.

[bcwhite, 2017/05/05 15:48:00]

Yeah, the Android builds all fail with either call.  <sigh>

And unfortunately, it's the most important platform.  I'm thinking that, for
32-bit builds, I should just do the write of bytes.

I could try the low-level system call and just write the bytes if it returns
not-implemented.

What do you think?

[Primiano Tucci (use gerrit), 2017/05/05 16:23:33]

FYI we currently ship 32 bit only chrome also on 64 bit devices.
The only case where we ship a 64 bit binary is:
- webview (which I think is not going to be affected by this)
- canary (to make sure our 64 bit chrome doesn't rot)

[bcwhite, 2017/05/05 18:06:59]

Acknowledged.

+      if (fallocate(file_.GetPlatformFile(), 0, region.offset, region.size) !=

[Mark Mentovai, 2017/05/04 21:00:46]

Anyway, whether it’s fallocate() or posix_fallocate(), Bionic doesn’t seem to
fall back to writing to the file to force allocation if the underlying
filesystem doesn’t support this call. In contrast, glibc does this for
posix_fallocate():

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/posi...

where internal_fallocate() winds up being:

https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/posix/posix_fallocat...

ext4 will support fallocate(). ext3 won’t. I’m not positive that we’re assured
of ext4, or that we’re assured of a filesystem that supports fallocate().
Primiano may know.

[bcwhite, 2017/05/05 18:06:59]

Done.

+          0) {
+        DPLOG(ERROR) << "fallocate " << file_.GetPlatformFile();
+        return false;
+      }
+#else
+      if (posix_fallocate(file_.GetPlatformFile(), region.offset,
+                          region.size) != 0) {
+        DPLOG(ERROR) << "posix_fallocate " << file_.GetPlatformFile();
+        return false;
+      }
+#endif
+
       break;
   }
+
   data_ = static_cast<uint8_t*>(mmap(NULL, map_size, flags, MAP_SHARED,
                                      file_.GetPlatformFile(), map_start));
   if (data_ == MAP_FAILED) {
     DPLOG(ERROR) << "mmap " << file_.GetPlatformFile();
     return false;
   }
 
   data_ += data_offset;
   return true;
 }
 #endif
 
 void MemoryMappedFile::CloseHandles() {
   ThreadRestrictions::AssertIOAllowed();
 
   if (data_ != NULL)
     munmap(data_, length_);
   file_.Close();
 
   data_ = NULL;
   length_ = 0;
 }
 
 }  // namespace base