[windows] Release the Win32 address space reservation on OOM.

On Windows 32 bit, Chrome reserves a significant chunk of address space
when a renderer starts, to make large contiguous allocations easier to
fulfill. Once a memory allocation fails, the reservation is released,
then the allocation retried. This only works if *all* the allocators are
aware that this mechanism exists. Unfortunately, malloc() in particular
is not aware of this, meaning that if a large malloc() allocation bumps
into this issue first, then the renderer crashes.

As this seems to be the case, from looking at crash reports (see linked
bugs for instance), malloc() should be made aware of this.

On Windows, allocations use the allocator shim, and call the "new
handler", which should return a non-0 value when it is able to free some
memory. Currently, this new handler always crashes the process. We
update it to drop the reservation if it exists, and crash otherwise.

This should hopefully solve some crashes we are seeing, especially some
related to video decoding, frequently allocating buffers of ~1MB, which
is large enough to trigger the issue, as seen in crash reports.

This is not a complete fix, since it does not address all allocators in
a process, e.g. allocations by DLLs loaded by the process will not pass
through the new-handler hook. We do not (yet) see these as a significant
cause of crashes in practice though.

Note that this CL also adds the reservation dropping behavior to Linux
builds as well. There is no address space reservation in actual Chrome
builds on Linux, but adding this allows to exercise the tests on Linux
builders as well, which is preferable.

Bug: 1028086, 1023804
Change-Id: I12ad4aa0c7bf518c3cf1df93c966e8631a69c280
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1989747Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730994}

base/allocator/partition_allocator/page_allocator.cc

@@ -239,14 +239,21 @@ bool ReserveAddressSpace(size_t size) {
   return false;
 }
 
-void ReleaseReservation() {
+bool ReleaseReservation() {
   // To avoid deadlock, call only FreePages.
   subtle::SpinLock::Guard guard(GetReserveLock());
-  if (s_reservation_address != nullptr) {
-    FreePages(s_reservation_address, s_reservation_size);
-    s_reservation_address = nullptr;
-    s_reservation_size = 0;
-  }
+  if (!s_reservation_address)
+    return false;
+
+  FreePages(s_reservation_address, s_reservation_size);
+  s_reservation_address = nullptr;
+  s_reservation_size = 0;
+  return true;
+}
+
+bool HasReservationForTesting() {
+  subtle::SpinLock::Guard guard(GetReserveLock());
+  return s_reservation_address != nullptr;
 }
 
 uint32_t GetAllocPageErrorCode() {

base/allocator/partition_allocator/page_allocator.h

@@ -182,7 +182,12 @@ BASE_EXPORT bool ReserveAddressSpace(size_t size);
 
 // Releases any reserved address space. |AllocPages| calls this automatically on
 // an allocation failure. External allocators may also call this on failure.
-BASE_EXPORT void ReleaseReservation();
+//
+// Returns true when an existing reservation was released.
+BASE_EXPORT bool ReleaseReservation();
+
+// Returns true if there is currently an address space reservation.
+BASE_EXPORT bool HasReservationForTesting();
 
 // Returns |errno| (POSIX) or the result of |GetLastError| (Windows) when |mmap|
 // (POSIX) or |VirtualAlloc| (Windows) fails.

base/process/memory_linux.cc

@@ -10,6 +10,7 @@
 
 #include "base/allocator/allocator_shim.h"
 #include "base/allocator/buildflags.h"
+#include "base/allocator/partition_allocator/page_allocator.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
@@ -37,10 +38,18 @@ void OnNoMemorySize(size_t size) {
   LOG(FATAL) << "Out of memory.";
 }
 
-void OnNoMemory() {
+// NOINLINE as base::`anonymous namespace`::OnNoMemory() is recognized by the
+// crash server.
+NOINLINE void OnNoMemory() {
   OnNoMemorySize(0);
 }
 
+void ReleaseReservationOrTerminate() {
+  if (ReleaseReservation())
+    return;
+  OnNoMemory();
+}
+
 }  // namespace
 
 void EnableTerminationOnHeapCorruption() {
@@ -49,7 +58,7 @@ void EnableTerminationOnHeapCorruption() {
 
 void EnableTerminationOnOutOfMemory() {
   // Set the new-out of memory handler.
-  std::set_new_handler(&OnNoMemory);
+  std::set_new_handler(&ReleaseReservationOrTerminate);
   // If we're using glibc's allocator, the above functions will override
   // malloc and friends and make them die on out of memory.
 

base/process/memory_unittest.cc

@@ -9,9 +9,11 @@
 #include <stddef.h>
 
 #include <limits>
+#include <vector>
 
 #include "base/allocator/allocator_check.h"
 #include "base/allocator/buildflags.h"
+#include "base/allocator/partition_allocator/page_allocator.h"
 #include "base/compiler_specific.h"
 #include "base/debug/alias.h"
 #include "base/memory/aligned_memory.h"
@@ -497,6 +499,66 @@ TEST_F(OutOfMemoryTest, TerminateBecauseOutOfMemoryReportsAllocSize) {
 }
 #endif  // OS_WIN
 
+#if defined(ARCH_CPU_32_BITS) && (defined(OS_WIN) || defined(OS_LINUX))
+
+void TestAllocationsReleaseReservation(void* (*alloc_fn)(size_t),
+                                       void (*free_fn)(void*)) {
+  base::ReleaseReservation();
+  base::EnableTerminationOnOutOfMemory();
+
+  constexpr size_t kMiB = 1 << 20;
+  constexpr size_t kReservationSize = 512 * kMiB;  // MiB.
+
+  size_t reservation_size = kReservationSize;
+  while (!base::ReserveAddressSpace(reservation_size)) {
+    reservation_size -= 16 * kMiB;
+  }
+  ASSERT_TRUE(base::HasReservationForTesting());
+  ASSERT_GT(reservation_size, 0u);
+
+  // Allocate a large area at a time to bump into address space exhaustion
+  // before other limits. It is important not to do a larger allocation, to
+  // verify that we can allocate without removing the reservation. On the other
+  // hand, must be large enough to make the underlying implementation call
+  // mmap()/VirtualAlloc().
+  size_t allocation_size = reservation_size / 2;
+
+  std::vector<void*> areas;
+  // Pre-reserve the vector to make sure that we don't hit the address space
+  // limit while resizing the array.
+  areas.reserve(((2 * 4096 * kMiB) / allocation_size) + 1);
+
+  while (true) {
+    void* area = alloc_fn(allocation_size / 2);
+    ASSERT_TRUE(area);
+    areas.push_back(area);
+
+    // Working as intended, the allocation was successful, and the reservation
+    // was dropped instead of crashing.
+    //
+    // Meaning that the test is either successful, or crashes.
+    if (!base::HasReservationForTesting())
+      break;
+  }
+
+  EXPECT_GE(areas.size(), 2u)
+      << "Should be able to allocate without releasing the reservation";
+
+  for (void* ptr : areas)
+    free_fn(ptr);
+}
+
+TEST_F(OutOfMemoryHandledTest, MallocReleasesReservation) {
+  TestAllocationsReleaseReservation(malloc, free);
+}
+
+TEST_F(OutOfMemoryHandledTest, NewReleasesReservation) {
+  TestAllocationsReleaseReservation(
+      [](size_t size) { return static_cast<void*>(new char[size]); },
+      [](void* ptr) { delete[] static_cast<char*>(ptr); });
+}
+#endif  // defined(ARCH_CPU_32_BITS) && (defined(OS_WIN) || defined(OS_LINUX))
+
 // TODO(b.kelemen): make UncheckedMalloc and UncheckedCalloc work
 // on Windows as well.
 TEST_F(OutOfMemoryHandledTest, UncheckedMalloc) {

base/process/memory_win.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/allocator/partition_allocator/page_allocator.h"
 #include "base/process/memory.h"
 #include "base/stl_util.h"
 
@@ -43,7 +44,7 @@ namespace {
 #pragma warning(push)
 #pragma warning(disable: 4702)  // Unreachable code after the _exit.
 
-NOINLINE int OnNoMemory(size_t size) {
+[[noreturn]] NOINLINE int OnNoMemory(size_t size) {
   // Kill the process. This is important for security since most of code
   // does not check the result of memory allocation.
   // https://msdn.microsoft.com/en-us/library/het71c37.aspx
@@ -54,11 +55,18 @@ NOINLINE int OnNoMemory(size_t size) {
 
   // Safety check, make sure process exits here.
   _exit(win::kOomExceptionCode);
-  return 0;
 }
 
 #pragma warning(pop)
 
+// Return a non-0 value to retry the allocation.
+int ReleaseReservationOrTerminate(size_t size) {
+  constexpr int kRetryAllocation = 1;
+  if (ReleaseReservation())
+    return kRetryAllocation;
+  OnNoMemory(size);
+}
+
 }  // namespace
 
 void TerminateBecauseOutOfMemory(size_t size) {
@@ -71,8 +79,9 @@ void EnableTerminationOnHeapCorruption() {
 }
 
 void EnableTerminationOnOutOfMemory() {
-  _set_new_handler(&OnNoMemory);
-  _set_new_mode(1);
+  constexpr int kCallNewHandlerOnAllocationFailure = 1;
+  _set_new_handler(&ReleaseReservationOrTerminate);
+  _set_new_mode(kCallNewHandlerOnAllocationFailure);
 }
 
 // Implemented using a weak symbol.