[CheckedPtr] Inline EnabledForPtr

This CL allows the CheckedPtr constructor to quickly skip unsupported
pointers.

Change-Id: Ibabb8802f4feb3734e1e7740fd86bd8df9a4e657
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379831Reviewed-by: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Sergei Glazunov <glazunov@google.com>
Cr-Commit-Position: refs/heads/master@{#803067}

base/BUILD.gn

@@ -314,7 +314,6 @@ component("base") {
     "macros.h",
     "memory/aligned_memory.cc",
     "memory/aligned_memory.h",
-    "memory/checked_ptr.cc",
     "memory/checked_ptr.h",
     "memory/discardable_memory.cc",
     "memory/discardable_memory.h",
@@ -1746,6 +1745,7 @@ component("base") {
         # PartitionAlloc uses SpinLock, which doesn't work in NaCl (see below).
         "allocator/partition_allocator/address_pool_manager.cc",
         "allocator/partition_allocator/address_pool_manager.h",
+        "allocator/partition_allocator/address_pool_manager_types.h",
         "allocator/partition_allocator/address_space_randomization.cc",
         "allocator/partition_allocator/address_space_randomization.h",
         "allocator/partition_allocator/checked_ptr_support.h",

base/allocator/partition_allocator/address_pool_manager.h

@@ -7,6 +7,7 @@
 
 #include <bitset>
 
+#include "base/allocator/partition_allocator/address_pool_manager_types.h"
 #include "base/allocator/partition_allocator/partition_alloc_constants.h"
 #include "base/atomicops.h"
 #include "base/no_destructor.h"
@@ -18,8 +19,6 @@ namespace base {
 
 namespace internal {
 
-using pool_handle = unsigned;
-
 // The feature is not applicable to 32-bit address space.
 // ARCH_CPU_64_BITS implies 64-bit instruction set, but not necessarily 64-bit
 // address space. The only known case where address space is 32-bit is NaCl, so

base/allocator/partition_allocator/address_pool_manager_types.h

+// Copyright 2020 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_ADDRESS_POOL_MANAGER_TYPES_H_
+#define BASE_ALLOCATOR_PARTITION_ALLOCATOR_ADDRESS_POOL_MANAGER_TYPES_H_
+
+namespace base {
+
+namespace internal {
+
+using pool_handle = unsigned;
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_ALLOCATOR_PARTITION_ALLOCATOR_ADDRESS_POOL_MANAGER_TYPES_H_

base/allocator/partition_allocator/partition_address_space.cc

@@ -4,6 +4,7 @@
 
 #include "base/allocator/partition_allocator/partition_address_space.h"
 
+#include "base/allocator/partition_allocator/address_pool_manager.h"
 #include "base/allocator/partition_allocator/page_allocator.h"
 #include "base/allocator/partition_allocator/page_allocator_internal.h"
 #include "base/allocator/partition_allocator/partition_alloc_check.h"

base/allocator/partition_allocator/partition_address_space.h

@@ -7,13 +7,12 @@
 
 #include <algorithm>
 
-#include "base/allocator/partition_allocator/address_pool_manager.h"
+#include "base/allocator/partition_allocator/address_pool_manager_types.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_features.h"
+#include "base/allocator/partition_allocator/partition_alloc_forward.h"
 #include "base/base_export.h"
 #include "base/bits.h"
-#include "base/feature_list.h"
 #include "base/notreached.h"
 #include "base/partition_alloc_buildflags.h"
 #include "build/build_config.h"
@@ -144,12 +143,17 @@ class BASE_EXPORT PartitionAddressSpace {
 };
 
 ALWAYS_INLINE internal::pool_handle GetDirectMapPool() {
-  PA_DCHECK(IsPartitionAllocGigaCageEnabled());
+  // This file is included from checked_ptr.h. This will result in a cycle if it
+  // includes partition_alloc_features.h where IsPartitionAllocGigaCageEnabled
+  // resides, because it includes Finch headers which may include checked_ptr.h.
+  // TODO(bartekn): Uncomment once Finch is no longer used there.
+  // PA_DCHECK(IsPartitionAllocGigaCageEnabled());
   return PartitionAddressSpace::GetDirectMapPool();
 }
 
 ALWAYS_INLINE internal::pool_handle GetNormalBucketPool() {
-  PA_DCHECK(IsPartitionAllocGigaCageEnabled());
+  // TODO(bartekn): Uncomment once Finch is no longer used there (see above).
+  // PA_DCHECK(IsPartitionAllocGigaCageEnabled());
   return PartitionAddressSpace::GetNormalBucketPool();
 }
 

base/allocator/partition_allocator/partition_alloc.cc

@@ -854,6 +854,32 @@ void DCheckIfManagedByPartitionAllocNormalBuckets(const void* ptr) {
 }
 #endif
 
+// Gets the offset from the beginning of the allocated slot, adjusted for cookie
+// (if any).
+// CAUTION! Use only for normal buckets. Using on direct-mapped allocations may
+// lead to undefined behavior.
+//
+// This function is not a template, and can be used on either variant
+// (thread-safe or not) of the allocator. This relies on the two PartitionRoot<>
+// having the same layout, which is enforced by static_assert().
+BASE_EXPORT size_t PartitionAllocGetSlotOffset(void* ptr) {
+  internal::DCheckIfManagedByPartitionAllocNormalBuckets(ptr);
+  // The only allocations that don't use tag are allocated outside of GigaCage,
+  // hence we'd never get here in the use_tag=false case.
+  ptr = internal::PartitionPointerAdjustSubtract(true /* use_tag */, ptr);
+  auto* page =
+      internal::PartitionAllocGetPageForSize<internal::ThreadSafe>(ptr);
+  PA_DCHECK(PartitionRoot<internal::ThreadSafe>::FromPage(page)->allow_extras);
+
+  // Get the offset from the beginning of the slot span.
+  uintptr_t ptr_addr = reinterpret_cast<uintptr_t>(ptr);
+  uintptr_t slot_span_start = reinterpret_cast<uintptr_t>(
+      internal::PartitionPage<internal::ThreadSafe>::ToPointer(page));
+  size_t offset_in_slot_span = ptr_addr - slot_span_start;
+
+  return page->bucket->GetSlotOffset(offset_in_slot_span);
+}
+
 }  // namespace internal
 
 }  // namespace base

base/allocator/partition_allocator/partition_alloc.h

@@ -802,32 +802,6 @@ ALWAYS_INLINE void DCheckIfManagedByPartitionAllocNormalBuckets(const void*) {}
 #endif
 }  // namespace internal
 
-// Gets the offset from the beginning of the allocated slot, adjusted for cookie
-// (if any).
-// CAUTION! Use only for normal buckets. Using on direct-mapped allocations may
-// lead to undefined behavior.
-//
-// This function is not a template, and can be used on either variant
-// (thread-safe or not) of the allocator. This relies on the two PartitionRoot<>
-// having the same layout, which is enforced by static_assert().
-ALWAYS_INLINE size_t PartitionAllocGetSlotOffset(void* ptr) {
-  internal::DCheckIfManagedByPartitionAllocNormalBuckets(ptr);
-  // The only allocations that don't use tag are allocated outside of GigaCage,
-  // hence we'd never get here in the use_tag=false case.
-  ptr = internal::PartitionPointerAdjustSubtract(true /* use_tag */, ptr);
-  auto* page =
-      internal::PartitionAllocGetPageForSize<internal::ThreadSafe>(ptr);
-  PA_DCHECK(PartitionRoot<internal::ThreadSafe>::FromPage(page)->allow_extras);
-
-  // Get the offset from the beginning of the slot span.
-  uintptr_t ptr_addr = reinterpret_cast<uintptr_t>(ptr);
-  uintptr_t slot_span_start = reinterpret_cast<uintptr_t>(
-      internal::PartitionPage<internal::ThreadSafe>::ToPointer(page));
-  size_t offset_in_slot_span = ptr_addr - slot_span_start;
-
-  return page->bucket->GetSlotOffset(offset_in_slot_span);
-}
-
 #endif  // BUILDFLAG(USE_PARTITION_ALLOC)
 
 template <bool thread_safe>

base/allocator/partition_allocator/partition_alloc_forward.h

@@ -5,12 +5,16 @@
 #ifndef BASE_ALLOCATOR_PARTITION_ALLOCATOR_PARTITION_ALLOC_FORWARD_H_
 #define BASE_ALLOCATOR_PARTITION_ALLOCATOR_PARTITION_ALLOC_FORWARD_H_
 
+#include "base/base_export.h"
+
 namespace base {
 namespace internal {
 
 template <bool thread_safe>
 struct PartitionPage;
 
+BASE_EXPORT size_t PartitionAllocGetSlotOffset(void* ptr);
+
 constexpr bool ThreadSafe = true;
 constexpr bool NotThreadSafe = false;
 

base/memory/checked_ptr.cc

-// Copyright 2020 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/memory/checked_ptr.h"
-
-#include "base/allocator/partition_allocator/checked_ptr_support.h"
-#include "base/allocator/partition_allocator/partition_address_space.h"
-#include "base/allocator/partition_allocator/partition_alloc.h"
-#include "base/partition_alloc_buildflags.h"
-#include "build/build_config.h"
-#include "build/buildflag.h"
-
-namespace base {
-namespace internal {
-
-#if defined(ARCH_CPU_64_BITS) && !defined(OS_NACL) && \
-    BUILDFLAG(USE_PARTITION_ALLOC) && ENABLE_CHECKED_PTR2_OR_MTE_IMPL
-
-BASE_EXPORT bool CheckedPtr2OrMTEImplPartitionAllocSupport::EnabledForPtr(
-    void* ptr) {
-  // CheckedPtr2 and MTECheckedPtr algorithms work only when memory is allocated
-  // by PartitionAlloc, from normal buckets pool. CheckedPtr2 additionally
-  // requires that the pointer points to the beginning of the allocated slot.
-  //
-  // TODO(bartekn): Allow direct-map buckets for MTECheckedPtr, once
-  // PartitionAlloc supports it. (Currently not implemented for simplicity, but
-  // there are no technological obstacles preventing it; whereas in case of
-  // CheckedPtr2, PartitionAllocGetSlotOffset won't work with direct-map.)
-  return IsManagedByPartitionAllocNormalBuckets(ptr)
-  // Checking offset is not needed for ENABLE_TAG_FOR_SINGLE_TAG_CHECKED_PTR,
-  // but call it anyway for apples-to-apples comparison with
-  // ENABLE_TAG_FOR_CHECKED_PTR2.
-#if ENABLE_TAG_FOR_CHECKED_PTR2 || ENABLE_TAG_FOR_SINGLE_TAG_CHECKED_PTR
-         && PartitionAllocGetSlotOffset(ptr) == 0
-#endif
-      ;
-}
-
-#endif
-
-}  // namespace internal
-}  // namespace base

base/memory/checked_ptr.h

@@ -11,6 +11,8 @@
 #include <utility>
 
 #include "base/allocator/partition_allocator/checked_ptr_support.h"
+#include "base/allocator/partition_allocator/partition_address_space.h"
+#include "base/allocator/partition_allocator/partition_alloc_forward.h"
 #include "base/allocator/partition_allocator/partition_tag.h"
 #include "base/check_op.h"
 #include "base/compiler_specific.h"
@@ -116,11 +118,25 @@ static_assert((kTopBit & kGenerationMask) > 0,
 // overridden by tests.
 struct CheckedPtr2OrMTEImplPartitionAllocSupport {
   // Checks if the necessary support is enabled in PartitionAlloc for |ptr|.
-  //
-  // The implementation is in the .cc file, because including partition_alloc.h
-  // here could lead to cyclic includes.
-  // TODO(bartekn): Check if this function gets inlined.
-  BASE_EXPORT static bool EnabledForPtr(void* ptr);
+  static ALWAYS_INLINE bool EnabledForPtr(void* ptr) {
+    // CheckedPtr2 and MTECheckedPtr algorithms work only when memory is
+    // allocated by PartitionAlloc, from normal buckets pool. CheckedPtr2
+    // additionally requires that the pointer points to the beginning of the
+    // allocated slot.
+    //
+    // TODO(bartekn): Allow direct-map buckets for MTECheckedPtr, once
+    // PartitionAlloc supports it. (Currently not implemented for simplicity,
+    // but there are no technological obstacles preventing it; whereas in case
+    // of CheckedPtr2, PartitionAllocGetSlotOffset won't work with direct-map.)
+    return IsManagedByPartitionAllocNormalBuckets(ptr)
+    // Checking offset is not needed for ENABLE_TAG_FOR_SINGLE_TAG_CHECKED_PTR,
+    // but call it anyway for apples-to-apples comparison with
+    // ENABLE_TAG_FOR_CHECKED_PTR2.
+#if ENABLE_TAG_FOR_CHECKED_PTR2 || ENABLE_TAG_FOR_SINGLE_TAG_CHECKED_PTR
+           && base::internal::PartitionAllocGetSlotOffset(ptr) == 0
+#endif
+        ;
+  }
 
   // Returns pointer to the tag that protects are pointed by |ptr|.
   static ALWAYS_INLINE void* TagPointer(void* ptr) {