[PartitionAlloc] Make some error handling more concise

Also fix a bug: `PCHECK` may itself allocate, creating re-entrancy
problems, so we can't use it as a way to learn the `errno`. Instead,
create a `PA_PCHECK` macro that uses `base::debug::Alias`.

Bug: None
Change-Id: Ic17e7c09d620e8f1f38da8b64a45fbfeeeac3b5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546084
Commit-Queue: Chris Palmer <palmer@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829859}

base/allocator/partition_allocator/page_allocator_internals_posix.h

@@ -216,29 +216,12 @@ void SetSystemPagesAccessInternal(
     void* address,
     size_t length,
     PageAccessibilityConfiguration accessibility) {
-  if (!HANDLE_EINTR(mprotect(address, length, GetAccessFlags(accessibility))))
-    return;
-
-  // mprotect() failed, let's get more data on errno in crash reports. Values
-  // taken from man mprotect(2) on Linux:
-  switch (errno) {
-    case EACCES:
-      PCHECK(false);
-      break;
-    case EINVAL:
-      PCHECK(false);
-      break;
-    case ENOMEM:
-      PCHECK(false);
-      break;
-    default:
-      PCHECK(false);
-      break;
-  }
+  PA_PCHECK(0 == HANDLE_EINTR(
+                     mprotect(address, length, GetAccessFlags(accessibility))));
 }
 
 void FreePagesInternal(void* address, size_t length) {
-  PCHECK(!munmap(address, length));
+  PA_PCHECK(0 == munmap(address, length));
 }
 
 void* TrimMappingInternal(void* base,
@@ -294,7 +277,7 @@ void DiscardSystemPagesInternal(void* address, size_t length) {
     // MADV_FREE_REUSABLE sometimes fails, so fall back to MADV_DONTNEED.
     ret = madvise(address, length, MADV_DONTNEED);
   }
-  PCHECK(0 == ret);
+  PA_PCHECK(ret == 0);
 #else
   // We have experimented with other flags, but with suboptimal results.
   //
@@ -302,7 +285,7 @@ void DiscardSystemPagesInternal(void* address, size_t length) {
   // performance benefits unclear.
   //
   // Therefore, we just do the simple thing: MADV_DONTNEED.
-  PCHECK(!madvise(address, length, MADV_DONTNEED));
+  PA_PCHECK(0 == madvise(address, length, MADV_DONTNEED));
 #endif
 }
 

base/allocator/partition_allocator/partition_alloc_check.h

@@ -8,10 +8,12 @@
 #include "base/allocator/buildflags.h"
 #include "base/allocator/partition_allocator/page_allocator_constants.h"
 #include "base/check.h"
+#include "base/debug/alias.h"
+#include "base/immediate_crash.h"
 
 // When PartitionAlloc is used as the default allocator, we cannot use the
 // regular (D)CHECK() macros, as they allocate internally. When an assertion is
-// triggered, they format strings, leading to reentrency in the code, which none
+// triggered, they format strings, leading to reentrancy in the code, which none
 // of PartitionAlloc is designed to support (and especially not for error
 // paths).
 //
@@ -29,9 +31,17 @@
 #define PA_DCHECK(condition) EAT_CHECK_STREAM_PARAMS(!(condition))
 #endif  // DCHECK_IS_ON()
 
+#define PA_PCHECK(condition)    \
+  if (!(condition)) {           \
+    int error = errno;          \
+    base::debug::Alias(&error); \
+    IMMEDIATE_CRASH();          \
+  }
+
 #else
 #define PA_CHECK(condition) CHECK(condition)
 #define PA_DCHECK(condition) DCHECK(condition)
+#define PA_PCHECK(condition) PCHECK(condition)
 #endif  // BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC)
 
 #if defined(PAGE_ALLOCATOR_CONSTANTS_ARE_CONSTEXPR)