[PartitionAlloc] Add partition_alloc_notreached.h.

tasak · Chromium LUCI CQ · commit 77e5e50d3753 · 2021-09-08T09:27:24.000Z
When PartitionAlloc is used as the default allocator, we cannot use the regular (D)CHECK() macros, as they allocate internally. (c.f. //base/allocator/partition_allocator/partition_alloc_check.h) So PA_NOTREACHED() uses PA_DCHECK() instead of DCHECK(). Change-Id: I95fc01bfa7af0f79b1eafd62f4711efe0ff5de84 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3141323 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Benoit L <lizeb@chromium.org> Reviewed-by: Bartek Nowierski <bartekn@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Takashi Sakamoto <tasak@google.com> Cr-Commit-Position: refs/heads/main@{#919171}
diff --git a/base/BUILD.gn b/base/BUILD.gn
@@ -1892,6 +1892,7 @@ component("base") {
         "allocator/partition_allocator/partition_alloc_forward.h",
         "allocator/partition_allocator/partition_alloc_hooks.cc",
         "allocator/partition_allocator/partition_alloc_hooks.h",
+        "allocator/partition_allocator/partition_alloc_notreached.h",
         "allocator/partition_allocator/partition_bucket.cc",
         "allocator/partition_allocator/partition_bucket.h",
         "allocator/partition_allocator/partition_bucket_lookup.h",
diff --git a/base/allocator/partition_allocator/DEPS b/base/allocator/partition_allocator/DEPS
@@ -25,6 +25,7 @@ include_rules = [
     "+base/lazy_instance.h",
     "+base/location.h",
     "+base/logging.h",
+    "+base/logging_buildflags.h",
     "+base/mac/foundation_util.h",
     "+base/mac/mac_util.h",
     "+base/mac/scoped_cftyperef.h",
@@ -33,7 +34,6 @@ include_rules = [
     "+base/memory/tagging.h",
     "+base/metrics/histogram_functions.h",
     "+base/no_destructor.h",
-    "+base/notreached.h",
     "+base/numerics/ranges.h",
     "+base/posix/eintr_wrapper.h",
     "+base/process/memory.h",
diff --git a/base/allocator/partition_allocator/address_pool_manager.cc b/base/allocator/partition_allocator/address_pool_manager.cc
@@ -17,10 +17,10 @@
 #include "base/allocator/partition_allocator/page_allocator_internal.h"
 #include "base/allocator/partition_allocator/partition_alloc_check.h"
 #include "base/allocator/partition_allocator/partition_alloc_constants.h"
+#include "base/allocator/partition_allocator/partition_alloc_notreached.h"
 #include "base/allocator/partition_allocator/reservation_offset_table.h"
 #include "base/cxx17_backports.h"
 #include "base/lazy_instance.h"
-#include "base/notreached.h"
 
 namespace base {
 namespace internal {
@@ -61,7 +61,7 @@ pool_handle AddressPoolManager::Add(uintptr_t ptr, size_t length) {
       return i + 1;
     }
   }
-  NOTREACHED();
+  PA_NOTREACHED();
   return 0;
 }
 
@@ -206,7 +206,7 @@ uintptr_t AddressPoolManager::Pool::FindChunk(size_t requested_size) {
     }
   }
 
-  NOTREACHED();
+  PA_NOTREACHED();
   return 0;
 }
 
diff --git a/base/allocator/partition_allocator/page_allocator_internals_fuchsia.h b/base/allocator/partition_allocator/page_allocator_internals_fuchsia.h
@@ -17,8 +17,8 @@
 
 #include "base/allocator/partition_allocator/page_allocator.h"
 #include "base/allocator/partition_allocator/partition_alloc_check.h"
+#include "base/allocator/partition_allocator/partition_alloc_notreached.h"
 #include "base/fuchsia/fuchsia_logging.h"
-#include "base/notreached.h"
 
 namespace base {
 
@@ -55,7 +55,7 @@ zx_vm_option_t PageAccessibilityToZxVmOptions(
     case PageReadWriteExecute:
       return ZX_VM_PERM_READ | ZX_VM_PERM_WRITE | ZX_VM_PERM_EXECUTE;
     default:
-      NOTREACHED();
+      PA_NOTREACHED();
       FALLTHROUGH;
     case PageInaccessible:
       return 0;
diff --git a/base/allocator/partition_allocator/page_allocator_internals_posix.cc b/base/allocator/partition_allocator/page_allocator_internals_posix.cc
@@ -3,8 +3,8 @@
 // found in the LICENSE file.
 
 #include "base/allocator/partition_allocator/page_allocator.h"
+#include "base/allocator/partition_allocator/partition_alloc_notreached.h"
 #include "base/cpu.h"
-#include "base/notreached.h"
 
 #include <sys/mman.h>
 
@@ -52,7 +52,7 @@ int GetAccessFlags(PageAccessibilityConfiguration accessibility) {
     case PageReadWriteExecute:
       return PROT_READ | PROT_WRITE | PROT_EXEC;
     default:
-      NOTREACHED();
+      PA_NOTREACHED();
       FALLTHROUGH;
     case PageInaccessible:
       return PROT_NONE;
diff --git a/base/allocator/partition_allocator/page_allocator_internals_win.h b/base/allocator/partition_allocator/page_allocator_internals_win.h
@@ -8,7 +8,7 @@
 #include "base/allocator/partition_allocator/oom.h"
 #include "base/allocator/partition_allocator/page_allocator_internal.h"
 #include "base/allocator/partition_allocator/partition_alloc_check.h"
-#include "base/notreached.h"
+#include "base/allocator/partition_allocator/partition_alloc_notreached.h"
 
 namespace base {
 
@@ -29,7 +29,7 @@ int GetAccessFlags(PageAccessibilityConfiguration accessibility) {
     case PageReadWriteExecute:
       return PAGE_EXECUTE_READWRITE;
     default:
-      NOTREACHED();
+      PA_NOTREACHED();
       FALLTHROUGH;
     case PageInaccessible:
       return PAGE_NOACCESS;
diff --git a/base/allocator/partition_allocator/page_allocator_unittest.cc b/base/allocator/partition_allocator/page_allocator_unittest.cc
@@ -13,9 +13,9 @@
 #include "base/cpu.h"
 #include "base/logging.h"
 #include "base/memory/tagging.h"
-#include "base/notreached.h"
 
 #include "base/allocator/partition_allocator/address_space_randomization.h"
+#include "base/allocator/partition_allocator/partition_alloc_notreached.h"
 #include "build/build_config.h"
 #if defined(OS_ANDROID)
 #include "base/debug/proc_maps_linux.h"
@@ -242,7 +242,7 @@ TEST(PartitionAllocPageAllocatorTest,
               "");  // Should crash with SIGILL.
   FreePages(buffer, PageAllocationGranularity());
 #else
-  NOTREACHED();
+  PA_NOTREACHED();
 #endif
 }
 
@@ -298,7 +298,7 @@ TEST(PartitionAllocPageAllocatorTest,
             parent_tagging_mode);
   FreePages(buffer, PageAllocationGranularity());
 #else
-  NOTREACHED();
+  PA_NOTREACHED();
 #endif
 }
 
@@ -351,7 +351,7 @@ TEST(PartitionAllocPageAllocatorTest,
   EXPECT_EQ(memory::GetMemoryTaggingModeForCurrentThread(),
             parent_tagging_mode);
 #else
-  NOTREACHED();
+  PA_NOTREACHED();
 #endif
 }
 
diff --git a/base/allocator/partition_allocator/partition_address_space.h b/base/allocator/partition_allocator/partition_address_space.h
@@ -15,10 +15,10 @@
 #include "base/allocator/partition_allocator/partition_alloc_config.h"
 #include "base/allocator/partition_allocator/partition_alloc_constants.h"
 #include "base/allocator/partition_allocator/partition_alloc_forward.h"
+#include "base/allocator/partition_allocator/partition_alloc_notreached.h"
 #include "base/base_export.h"
 #include "base/bits.h"
 #include "base/compiler_specific.h"
-#include "base/notreached.h"
 #include "build/build_config.h"
 #include "build/buildflag.h"
 
@@ -102,7 +102,7 @@ class BASE_EXPORT PartitionAddressSpace {
       base = brp_pool_base_address_;
 #endif  // BUILDFLAG(USE_BACKUP_REF_PTR)
     } else {
-      NOTREACHED();
+      PA_NOTREACHED();
     }
     uintptr_t address_as_uintptr = reinterpret_cast<uintptr_t>(address);
     return std::make_pair(pool, address_as_uintptr - base);
diff --git a/base/allocator/partition_allocator/partition_alloc_notreached.h b/base/allocator/partition_allocator/partition_alloc_notreached.h
@@ -0,0 +1,25 @@
+// Copyright 2021 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.
+
+#ifndef BASE_ALLOCATOR_PARTITION_ALLOCATOR_PARTITION_ALLOC_NOTREACHED_H_
+#define BASE_ALLOCATOR_PARTITION_ALLOCATOR_PARTITION_ALLOC_NOTREACHED_H_
+
+#include "base/allocator/partition_allocator/partition_alloc_check.h"
+#include "base/logging_buildflags.h"
+
+// When PartitionAlloc is used as the default allocator, we cannot use the
+// regular (D)CHECK() macros, as they allocate internally. (c.f. //
+// base/allocator/partition_allocator/partition_alloc_check.h)
+// So PA_NOTREACHED() uses PA_DCHECK() instead of DCHECK().
+
+#if BUILDFLAG(ENABLE_LOG_ERROR_NOT_REACHED)
+#define PA_NOTREACHED()                                                      \
+  true ? logging::RawError(__FILE__                                          \
+                           "(" PA_STRINGIFY(__LINE__) ") NOTREACHED() hit.") \
+       : EAT_CHECK_STREAM_PARAMS()
+#else
+#define PA_NOTREACHED() PA_DCHECK(false)
+#endif
+
+#endif  // BASE_ALLOCATOR_PARTITION_ALLOCATOR_PARTITION_ALLOC_NOTREACHED_H_
diff --git a/base/allocator/partition_allocator/partition_root.h b/base/allocator/partition_allocator/partition_root.h
@@ -45,6 +45,7 @@
 #include "base/allocator/partition_allocator/partition_alloc_features.h"
 #include "base/allocator/partition_allocator/partition_alloc_forward.h"
 #include "base/allocator/partition_allocator/partition_alloc_hooks.h"
+#include "base/allocator/partition_allocator/partition_alloc_notreached.h"
 #include "base/allocator/partition_allocator/partition_bucket_lookup.h"
 #include "base/allocator/partition_allocator/partition_direct_map_extent.h"
 #include "base/allocator/partition_allocator/partition_lock.h"
@@ -1590,7 +1591,7 @@ ALWAYS_INLINE void* PartitionRoot<thread_safe>::AlignedAllocFlags(
       // PartitionExcessiveAllocationSize).
       internal::PartitionExcessiveAllocationSize(requested_size);
       // internal::PartitionExcessiveAllocationSize(size) causes OOM_CRASH.
-      NOTREACHED();
+      PA_NOTREACHED();
     }
   }
 
diff --git a/base/check.cc b/base/check.cc
@@ -112,4 +112,8 @@ void RawCheck(const char* message) {
   RawLog(LOGGING_FATAL, message);
 }
 
+void RawError(const char* message) {
+  RawLog(LOGGING_ERROR, message);
+}
+
 }  // namespace logging
diff --git a/base/check.h b/base/check.h
@@ -148,6 +148,7 @@ class BASE_EXPORT CheckError {
 
 // Async signal safe checking mechanism.
 BASE_EXPORT void RawCheck(const char* message);
+BASE_EXPORT void RawError(const char* message);
 #define RAW_CHECK(condition)                                 \
   do {                                                       \
     if (!(condition))                                        \

Original file line number	Diff line number	Diff line change
`@@ -17,10 +17,10 @@`
`17`	`17`	`#include "base/allocator/partition_allocator/page_allocator_internal.h"`
`18`	`18`	`#include "base/allocator/partition_allocator/partition_alloc_check.h"`
`19`	`19`	`#include "base/allocator/partition_allocator/partition_alloc_constants.h"`
	`20`	`+#include "base/allocator/partition_allocator/partition_alloc_notreached.h"`
`20`	`21`	`#include "base/allocator/partition_allocator/reservation_offset_table.h"`
`21`	`22`	`#include "base/cxx17_backports.h"`
`22`	`23`	`#include "base/lazy_instance.h"`
`23`		`-#include "base/notreached.h"`
`24`	`24`
`25`	`25`	`namespace base {`
`26`	`26`	`namespace internal {`
`@@ -61,7 +61,7 @@ pool_handle AddressPoolManager::Add(uintptr_t ptr, size_t length) {`
`61`	`61`	`return i + 1;`
`62`	`62`	`}`
`63`	`63`	`}`
`64`		`- NOTREACHED();`
	`64`	`+ PA_NOTREACHED();`
`65`	`65`	`return 0;`
`66`	`66`	`}`
`67`	`67`
`@@ -206,7 +206,7 @@ uintptr_t AddressPoolManager::Pool::FindChunk(size_t requested_size) {`
`206`	`206`	`}`
`207`	`207`	`}`
`208`	`208`
`209`		`- NOTREACHED();`
	`209`	`+ PA_NOTREACHED();`
`210`	`210`	`return 0;`
`211`	`211`	`}`
`212`	`212`