PageAllocator: Fix DecommitSystemPages() semantics

Prior to this CL, DecommitSystemPages() performed 2 conceptually
different operations that callers cared about depending on which platorm it was
called on.

There are 2 operations:
  * return storage from ram or swap to the OS
  * make the page inaccessible.

On Windows, "Committed memory" is a first-class concept in the OS Memory
Subsystem API. A region returned via VirtualAlloc() when MEM_COMMIT is
specified is guaranteed to be touchable without causing the program to
crash due to out of memory. Since it is a first class concept,
decommitting is directly supported and calling this function resulted in
the both operations above.

In the POSIX memory API, there is no such thing as "Committed memory."
Until the first touch of an anonymous mmap()ed region, it is unknown if
the pointer deference for that address may crash. Therefore this API
had unclear semantics and was implemented to ONLY return allocated ram
and swap to the OS. Page permissions were left the same meaning subsequent
touches would just reallocate the memory instead of fault.

This divergence in page accessibility caused callers to manually execute
SetSystemPagesAccess() after calling DecommitSystemPages() to ensure
uniform behavior across all platforms which resulted in a wasted syscall
on Windows and confusing API semantics.

This CL changes the contract of DecommitSystemPages() so that it
provides the guarnatees that can be enforced on all platforms. The
naming is still somewhat incorrect as "commit" is not a well defined
concept in POSIX...baby steps...

Bug: 766882
Change-Id: Ib5c8f1c712e080f1003fad628f60012e0feaf9d8
Reviewed-on: https://chromium-review.googlesource.com/696292
Commit-Queue: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509309}

base/allocator/partition_allocator/page_allocator.cc

@@ -292,6 +292,25 @@ bool SetSystemPagesAccess(void* address,
 
 void DecommitSystemPages(void* address, size_t length) {
   DCHECK(!(length & kSystemPageOffsetMask));
+#if defined(OS_POSIX)
+  // In POSIX, there is no decommit concept. Discarding is an effective way of
+  // implementing the Windows semantics where the OS is allows to not swap the
+  // pages in the region.
+  DiscardSystemPages(address, length);
+#endif
+  CHECK(SetSystemPagesAccess(address, length, PageInaccessible));
+}
+
+bool RecommitSystemPages(void* address,
+                         size_t length,
+                         PageAccessibilityConfiguration page_accessibility) {
+  DCHECK(!(length & kSystemPageOffsetMask));
+  DCHECK_NE(PageInaccessible, page_accessibility);
+  return SetSystemPagesAccess(address, length, page_accessibility);
+}
+
+void DiscardSystemPages(void* address, size_t length) {
+  DCHECK_EQ(0UL, length & kSystemPageOffsetMask);
 #if defined(OS_POSIX)
 #if defined(OS_MACOSX)
   // On macOS, MADV_FREE_REUSABLE has comparable behavior to MADV_FREE, but also
@@ -308,34 +327,6 @@ void DecommitSystemPages(void* address, size_t length) {
     ret = madvise(address, length, MADV_DONTNEED);
   }
   CHECK(!ret);
-#else
-  CHECK(SetSystemPagesAccess(address, length, PageInaccessible));
-#endif
-}
-
-bool RecommitSystemPages(void* address,
-                         size_t length,
-                         PageAccessibilityConfiguration page_accessibility) {
-  DCHECK(!(length & kSystemPageOffsetMask));
-  DCHECK_NE(PageInaccessible, page_accessibility);
-#if defined(OS_POSIX)
-  // On POSIX systems, read the memory to recommit. This has the correct
-  // behavior because the API requires the permissions to be the same as before
-  // decommitting and all configurations can read.
-  (void)address;
-  return true;
-#endif
-  return SetSystemPagesAccess(address, length, page_accessibility);
-}
-
-void DiscardSystemPages(void* address, size_t length) {
-  DCHECK(!(length & kSystemPageOffsetMask));
-#if defined(OS_POSIX)
-  // On POSIX, the implementation detail is that discard and decommit are the
-  // same, and lead to pages that are returned to the system immediately and
-  // get replaced with zeroed pages when touched. So we just call
-  // DecommitSystemPages() here to avoid code duplication.
-  DecommitSystemPages(address, length);
 #else
   // On Windows discarded pages are not returned to the system immediately and
   // not guaranteed to be zeroed when returned to the application.

base/allocator/partition_allocator/page_allocator.h

@@ -83,24 +83,29 @@ BASE_EXPORT WARN_UNUSED_RESULT bool SetSystemPagesAccess(
 // Decommit one or more system pages starting at |address| and continuing for
 // |length| bytes. |length| must be a multiple of |kSystemPageSize|.
 //
-// Decommitted means that the physical memory is released to the system, but the
-// virtual address space remains reserved. System pages are re-committed by
-// calling |RecommitSystemPages|. Touching a decommitted page _may_ fault.
+// Decommitted means that physical resources (RAM or swap) backing the allocated
+// virtual address range are released back to the system, but the address space
+// is still allocated to the process (possibly using up page table entries or
+// other accounting resources). Any access to a decommitted region of memory
+// is an error and will generate a fault.
 //
-// Clients should not make any assumptions about the contents of decommitted
-// system pages, before or after they write to the page. The only guarantee
-// provided is that the contents of the system page will be deterministic again
-// after recommitting and writing to it. In particlar note that system pages are
-// not guaranteed to be zero-filled upon re-commit.
+// This operation is not atomic on all platforms.
+//
+// Note: "Committed memory" is a Windows Memory Subsystem concept that ensures
+// processes will not fault when touching a committed memory region. There is
+// no analogue in the POSIX memory API where virtual memory pages are
+// best-effort allocated resources on the first touch. To create a
+// platform-agnostic abstraction, this API simulates the Windows "decommit"
+// state by both discarding the region (allowing the OS to aviod swap
+// operations) and changing the page protections so accesses fault.
 BASE_EXPORT void DecommitSystemPages(void* address, size_t length);
 
 // Recommit one or more system pages, starting at |address| and continuing for
 // |length| bytes with the given |page_accessibility|. |length| must be a
 // multiple of |kSystemPageSize|.
 //
 // Decommitted system pages must be recommitted with their original permissions
-// before they are used again. Note that this operation may be a no-op on some
-// platforms.
+// before they are used again.
 //
 // Returns true if the recommit change succeeded. In most cases you must |CHECK|
 // the result.

third_party/WebKit/Source/platform/heap/HeapPage.cpp

@@ -629,9 +629,11 @@ void NormalPageArena::AllocatePage() {
       // gets a page and add the rest to the page pool.
       if (!page_memory) {
         bool result = memory->Commit();
-        // If you hit the ASSERT, it will mean that you're hitting
-        // the limit of the number of mmapped regions OS can support
-        // (e.g., /proc/sys/vm/max_map_count in Linux).
+        // If you hit the ASSERT, it will mean that you're hitting the limit
+        // of the number of mmapped regions OS can support
+        // (e.g., /proc/sys/vm/max_map_count in Linux) or on that Windows you
+        // have exceeded the max commit charge across all processes for the
+        // system.
         CHECK(result);
         page_memory = memory;
       } else {

third_party/WebKit/Source/platform/heap/PageMemory.cpp

@@ -17,14 +17,12 @@ void MemoryRegion::Release() {
 }
 
 bool MemoryRegion::Commit() {
-  CHECK(WTF::RecommitSystemPages(base_, size_, WTF::PageReadWrite));
-  return WTF::SetSystemPagesAccess(base_, size_, WTF::PageReadWrite);
+  return WTF::RecommitSystemPages(base_, size_, WTF::PageReadWrite);
 }
 
 void MemoryRegion::Decommit() {
   ASAN_UNPOISON_MEMORY_REGION(base_, size_);
   WTF::DecommitSystemPages(base_, size_);
-  CHECK(WTF::SetSystemPagesAccess(base_, size_, WTF::PageInaccessible));
 }
 
 PageMemoryRegion::PageMemoryRegion(Address base,