Skip to content

Commit a25ec88

Browse files
tsepezcopybara-github
authored andcommitted
Propagate UNSAFETY through partition alloc raw pointers
Remove the file-wide pragmas, and mark on an expression by expression basis. Fixed: 435068772 Change-Id: I2ec753cdc931e56f82d43f3fcc7760ac80108483 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6806664 Commit-Queue: Tom Sepez <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Commit-Position: refs/heads/main@{#1496622} NOKEYCHECK=True GitOrigin-RevId: ff17a643261da04611e1a0692cf70afe9a224e7e
1 parent 4e92c91 commit a25ec88

File tree

4 files changed

+66
-49
lines changed

4 files changed

+66
-49
lines changed

src/partition_alloc/pointers/raw_ptr.h

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#ifdef UNSAFE_BUFFERS_BUILD
6-
// TODO(crbug.com/40284755): Remove this and spanify to fix the errors.
7-
#pragma allow_unsafe_buffers
8-
#endif
9-
105
// IWYU pragma: private, include "base/memory/raw_ptr.h"
116

127
#ifndef PARTITION_ALLOC_POINTERS_RAW_PTR_H_
@@ -680,20 +675,23 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
680675
return static_cast<U*>(GetForExtraction());
681676
}
682677

678+
// PRECONDITIONS: `this` must not be at the end of the range.
683679
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE constexpr raw_ptr& operator++() {
684680
static_assert(
685681
raw_ptr_traits::IsPtrArithmeticAllowed(Traits),
686682
"cannot increment raw_ptr unless AllowPtrArithmetic trait is present.");
687-
wrapped_ptr_ = Impl::Advance(wrapped_ptr_, 1, true);
683+
wrapped_ptr_ = PA_UNSAFE_TODO(Impl::Advance(wrapped_ptr_, 1, true));
688684
return *this;
689685
}
686+
// PRECONDITIONS: `this` must not be at the start of the range.
690687
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE constexpr raw_ptr& operator--() {
691688
static_assert(
692689
raw_ptr_traits::IsPtrArithmeticAllowed(Traits),
693690
"cannot decrement raw_ptr unless AllowPtrArithmetic trait is present.");
694-
wrapped_ptr_ = Impl::Retreat(wrapped_ptr_, 1, true);
691+
wrapped_ptr_ = PA_UNSAFE_TODO(Impl::Retreat(wrapped_ptr_, 1, true));
695692
return *this;
696693
}
694+
// PRECONDITIONS: `this` must not be at the end of the range.
697695
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE constexpr raw_ptr operator++(
698696
int /* post_increment */) {
699697
static_assert(
@@ -703,6 +701,7 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
703701
++(*this);
704702
return result;
705703
}
704+
// PRECONDITIONS: `this` must not be at the start of the range.
706705
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE constexpr raw_ptr operator--(
707706
int /* post_decrement */) {
708707
static_assert(
@@ -712,6 +711,7 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
712711
--(*this);
713712
return result;
714713
}
714+
// PRECONDITIONS: `this` must be at least `delta_elems` before range end.
715715
template <
716716
typename Z,
717717
typename = std::enable_if_t<partition_alloc::internal::is_offset_type<Z>>>
@@ -720,9 +720,11 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
720720
static_assert(
721721
raw_ptr_traits::IsPtrArithmeticAllowed(Traits),
722722
"cannot increment raw_ptr unless AllowPtrArithmetic trait is present.");
723-
wrapped_ptr_ = Impl::Advance(wrapped_ptr_, delta_elems, true);
723+
wrapped_ptr_ =
724+
PA_UNSAFE_TODO(Impl::Advance(wrapped_ptr_, delta_elems, true));
724725
return *this;
725726
}
727+
// PRECONDITIONS: `this` must be at least `delta_elems` after range start.
726728
template <
727729
typename Z,
728730
typename = std::enable_if_t<partition_alloc::internal::is_offset_type<Z>>>
@@ -731,10 +733,12 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
731733
static_assert(
732734
raw_ptr_traits::IsPtrArithmeticAllowed(Traits),
733735
"cannot decrement raw_ptr unless AllowPtrArithmetic trait is present.");
734-
wrapped_ptr_ = Impl::Retreat(wrapped_ptr_, delta_elems, true);
736+
wrapped_ptr_ =
737+
PA_UNSAFE_TODO(Impl::Retreat(wrapped_ptr_, delta_elems, true));
735738
return *this;
736739
}
737740

741+
// PRECONDITIONS: `delta_elems` must be an index inside the range.
738742
template <typename Z,
739743
typename U = T,
740744
typename = std::enable_if_t<
@@ -748,7 +752,7 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
748752
// Call SafelyUnwrapPtrForDereference() to simulate what GetForDereference()
749753
// does, but without creating a temporary.
750754
return *Impl::SafelyUnwrapPtrForDereference(
751-
Impl::Advance(wrapped_ptr_, delta_elems, false));
755+
PA_UNSAFE_TODO(Impl::Advance(wrapped_ptr_, delta_elems, false)));
752756
}
753757

754758
// Do not disable operator+() and operator-().
@@ -764,6 +768,8 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
764768
// operators for Z=uint64_t on 32-bit systems. The compiler instead would
765769
// generate code that converts `raw_ptr<T>` to `T*` and adds uint64_t to that,
766770
// bypassing the OOB protection entirely.
771+
//
772+
// PRECONDITIONS: `this` must be at least `delta_elems` before range end.
767773
template <typename Z>
768774
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE friend constexpr raw_ptr operator+(
769775
const raw_ptr& p,
@@ -773,15 +779,18 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
773779
static_assert(
774780
raw_ptr_traits::IsPtrArithmeticAllowed(Traits),
775781
"cannot add to raw_ptr unless AllowPtrArithmetic trait is present.");
776-
raw_ptr result = Impl::Advance(p.wrapped_ptr_, delta_elems, false);
782+
raw_ptr result =
783+
PA_UNSAFE_TODO(Impl::Advance(p.wrapped_ptr_, delta_elems, false));
777784
return result;
778785
}
786+
// PRECONDITIONS: `this` must be at least `delta_elems` before range end.
779787
template <typename Z>
780788
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE friend constexpr raw_ptr operator+(
781789
Z delta_elems,
782790
const raw_ptr& p) {
783791
return p + delta_elems;
784792
}
793+
// PRECONDITIONS: `this` must be at least `delta_elems` after range start.
785794
template <typename Z>
786795
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE friend constexpr raw_ptr operator-(
787796
const raw_ptr& p,
@@ -791,7 +800,8 @@ class PA_TRIVIAL_ABI PA_GSL_POINTER raw_ptr {
791800
static_assert(raw_ptr_traits::IsPtrArithmeticAllowed(Traits),
792801
"cannot subtract from raw_ptr unless AllowPtrArithmetic "
793802
"trait is present.");
794-
raw_ptr result = Impl::Retreat(p.wrapped_ptr_, delta_elems, false);
803+
raw_ptr result =
804+
PA_UNSAFE_TODO(Impl::Retreat(p.wrapped_ptr_, delta_elems, false));
795805
return result;
796806
}
797807

src/partition_alloc/pointers/raw_ptr_backup_ref_impl.h

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#ifdef UNSAFE_BUFFERS_BUILD
6-
// TODO(crbug.com/40284755): Remove this and spanify to fix the errors.
7-
#pragma allow_unsafe_buffers
8-
#endif
9-
105
#ifndef PARTITION_ALLOC_POINTERS_RAW_PTR_BACKUP_REF_IMPL_H_
116
#define PARTITION_ALLOC_POINTERS_RAW_PTR_BACKUP_REF_IMPL_H_
127

@@ -340,15 +335,18 @@ struct RawPtrBackupRefImpl {
340335
// Advance the wrapped pointer by `delta_elems`.
341336
// `is_in_pointer_modification` means that the result is intended to modify
342337
// the pointer (as opposed to creating a new one).
338+
// PRECONDITIONS: `wrapped_ptr` must be at least `delta_elems` before the
339+
// end of the range.
343340
template <
344341
typename T,
345342
typename Z,
346343
typename =
347344
std::enable_if_t<partition_alloc::internal::is_offset_type<Z>, void>>
348-
PA_ALWAYS_INLINE static constexpr T*
345+
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE static constexpr T*
349346
Advance(T* wrapped_ptr, Z delta_elems, bool is_in_pointer_modification) {
347+
// SAFETY: Preconditions enforced by PA_UNSAFE_BUFFER_USAGE.
350348
if (partition_alloc::internal::base::is_constant_evaluated()) {
351-
return wrapped_ptr + delta_elems;
349+
return PA_UNSAFE_BUFFERS(wrapped_ptr + delta_elems);
352350
}
353351
T* unpoisoned_ptr = UnpoisonPtr(wrapped_ptr);
354352
// When modifying the pointer, we have to make sure it doesn't migrate to a
@@ -357,24 +355,29 @@ struct RawPtrBackupRefImpl {
357355
// properly. Do it anyway if extra OOB checks are enabled.
358356
if (PA_BUILDFLAG(BACKUP_REF_PTR_EXTRA_OOB_CHECKS) ||
359357
is_in_pointer_modification) {
358+
// SAFETY: Preconditions enforced by PA_UNSAFE_BUFFER_USAGE.
360359
return VerifyAndPoisonPointerAfterAdvanceOrRetreat(
361-
unpoisoned_ptr, unpoisoned_ptr + delta_elems);
360+
unpoisoned_ptr, PA_UNSAFE_BUFFERS(unpoisoned_ptr + delta_elems));
362361
}
363-
return unpoisoned_ptr + delta_elems;
362+
// SAFETY: Preconditions enforced by PA_UNSAFE_BUFFER_USAGE.
363+
return PA_UNSAFE_BUFFERS(unpoisoned_ptr + delta_elems);
364364
}
365365

366366
// Retreat the wrapped pointer by `delta_elems`.
367367
// `is_in_pointer_modification` means that the result is intended to modify
368368
// the pointer (as opposed to creating a new one).
369+
// PRECONDITIONS: `wrapped_ptr` must be at least `delta_elems` after
370+
// the start of the range.
369371
template <
370372
typename T,
371373
typename Z,
372374
typename =
373375
std::enable_if_t<partition_alloc::internal::is_offset_type<Z>, void>>
374-
PA_ALWAYS_INLINE static constexpr T*
376+
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE static constexpr T*
375377
Retreat(T* wrapped_ptr, Z delta_elems, bool is_in_pointer_modification) {
376378
if (partition_alloc::internal::base::is_constant_evaluated()) {
377-
return wrapped_ptr - delta_elems;
379+
// SAFETY: Preconditions enforced by PA_UNSAFE_BUFFER_USAGE.
380+
return PA_UNSAFE_BUFFERS(wrapped_ptr - delta_elems);
378381
}
379382
T* unpoisoned_ptr = UnpoisonPtr(wrapped_ptr);
380383
// When modifying the pointer, we have to make sure it doesn't migrate to a
@@ -383,10 +386,12 @@ struct RawPtrBackupRefImpl {
383386
// properly. Do it anyway if extra OOB checks are enabled.
384387
if (PA_BUILDFLAG(BACKUP_REF_PTR_EXTRA_OOB_CHECKS) ||
385388
is_in_pointer_modification) {
389+
// SAFETY: Preconditions enforced by PA_UNSAFE_BUFFER_USAGE.
386390
return VerifyAndPoisonPointerAfterAdvanceOrRetreat(
387-
unpoisoned_ptr, unpoisoned_ptr - delta_elems);
391+
unpoisoned_ptr, PA_UNSAFE_BUFFERS(unpoisoned_ptr - delta_elems));
388392
}
389-
return unpoisoned_ptr - delta_elems;
393+
// SAFETY: Preconditions enforced by PA_UNSAFE_BUFFER_USAGE.
394+
return PA_UNSAFE_BUFFERS(unpoisoned_ptr - delta_elems);
390395
}
391396

392397
template <typename T>

src/partition_alloc/pointers/raw_ptr_hookable_impl.h

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#ifdef UNSAFE_BUFFERS_BUILD
6-
// TODO(crbug.com/40284755): Remove this and spanify to fix the errors.
7-
#pragma allow_unsafe_buffers
8-
#endif
9-
105
#ifndef PARTITION_ALLOC_POINTERS_RAW_PTR_HOOKABLE_IMPL_H_
116
#define PARTITION_ALLOC_POINTERS_RAW_PTR_HOOKABLE_IMPL_H_
127

@@ -132,39 +127,45 @@ struct RawPtrHookableImpl {
132127
}
133128

134129
// Advance the wrapped pointer by `delta_elems`.
130+
// PRECONDITIONS: `wrapped_ptr` must be at least `delta_elems` before the
131+
// end of the range.
135132
template <
136133
typename T,
137134
typename Z,
138135
typename =
139136
std::enable_if_t<partition_alloc::internal::is_offset_type<Z>, void>>
140-
PA_ALWAYS_INLINE static constexpr T*
137+
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE static constexpr T*
141138
Advance(T* wrapped_ptr, Z delta_elems, bool /*is_in_pointer_modification*/) {
139+
// SAFETY: required from caller, enforced by PA_UNSAFE_BUFFER_USAGE.
142140
if (!partition_alloc::internal::base::is_constant_evaluated()) {
143141
if (EnableHooks) {
144-
GetRawPtrHooks()->advance(
145-
reinterpret_cast<uintptr_t>(wrapped_ptr),
146-
reinterpret_cast<uintptr_t>(wrapped_ptr + delta_elems));
142+
GetRawPtrHooks()->advance(reinterpret_cast<uintptr_t>(wrapped_ptr),
143+
reinterpret_cast<uintptr_t>(PA_UNSAFE_BUFFERS(
144+
wrapped_ptr + delta_elems)));
147145
}
148146
}
149-
return wrapped_ptr + delta_elems;
147+
return PA_UNSAFE_BUFFERS(wrapped_ptr + delta_elems);
150148
}
151149

152150
// Retreat the wrapped pointer by `delta_elems`.
151+
// PRECONDITIONS: `wrapped_ptr` must be at least `delta_elems` after
152+
// the start of the range.
153153
template <
154154
typename T,
155155
typename Z,
156156
typename =
157157
std::enable_if_t<partition_alloc::internal::is_offset_type<Z>, void>>
158-
PA_ALWAYS_INLINE static constexpr T*
158+
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE static constexpr T*
159159
Retreat(T* wrapped_ptr, Z delta_elems, bool /*is_in_pointer_modification*/) {
160+
// SAFETY: required from caller, enforced by PA_UNSAFE_BUFFER_USAGE.
160161
if (!partition_alloc::internal::base::is_constant_evaluated()) {
161162
if (EnableHooks) {
162-
GetRawPtrHooks()->advance(
163-
reinterpret_cast<uintptr_t>(wrapped_ptr),
164-
reinterpret_cast<uintptr_t>(wrapped_ptr - delta_elems));
163+
GetRawPtrHooks()->advance(reinterpret_cast<uintptr_t>(wrapped_ptr),
164+
reinterpret_cast<uintptr_t>(PA_UNSAFE_BUFFERS(
165+
wrapped_ptr - delta_elems)));
165166
}
166167
}
167-
return wrapped_ptr - delta_elems;
168+
return PA_UNSAFE_BUFFERS(wrapped_ptr - delta_elems);
168169
}
169170

170171
template <typename T>

src/partition_alloc/pointers/raw_ptr_noop_impl.h

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
#ifdef UNSAFE_BUFFERS_BUILD
6-
// TODO(crbug.com/40284755): Remove this and spanify to fix the errors.
7-
#pragma allow_unsafe_buffers
8-
#endif
9-
105
#ifndef PARTITION_ALLOC_POINTERS_RAW_PTR_NOOP_IMPL_H_
116
#define PARTITION_ALLOC_POINTERS_RAW_PTR_NOOP_IMPL_H_
127

@@ -68,25 +63,31 @@ struct RawPtrNoOpImpl {
6863
}
6964

7065
// Advance the wrapped pointer by `delta_elems`.
66+
// PRECONDITIONS: `wrapped_ptr` must be at least `delta_elems` before the
67+
// end of the range.
7168
template <
7269
typename T,
7370
typename Z,
7471
typename =
7572
std::enable_if_t<partition_alloc::internal::is_offset_type<Z>, void>>
76-
PA_ALWAYS_INLINE static constexpr T*
73+
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE static constexpr T*
7774
Advance(T* wrapped_ptr, Z delta_elems, bool /*is_in_pointer_modification*/) {
78-
return wrapped_ptr + delta_elems;
75+
// SAFETY: required from caller, enforced by PA_UNSAFE_BUFFER_USAGE.
76+
return PA_UNSAFE_BUFFERS(wrapped_ptr + delta_elems);
7977
}
8078

8179
// Retreat the wrapped pointer by `delta_elems`.
80+
// PRECONDITIONS: `wrapped_ptr` must be at least `delta_elems` after
81+
// the start of the range.
8282
template <
8383
typename T,
8484
typename Z,
8585
typename =
8686
std::enable_if_t<partition_alloc::internal::is_offset_type<Z>, void>>
87-
PA_ALWAYS_INLINE static constexpr T*
87+
PA_UNSAFE_BUFFER_USAGE PA_ALWAYS_INLINE static constexpr T*
8888
Retreat(T* wrapped_ptr, Z delta_elems, bool /*is_in_pointer_modification*/) {
89-
return wrapped_ptr - delta_elems;
89+
// SAFETY: required from caller, enforced by PA_UNSAFE_BUFFER_USAGE.
90+
return PA_UNSAFE_BUFFERS(wrapped_ptr - delta_elems);
9091
}
9192

9293
template <typename T>

0 commit comments

Comments
 (0)