Skip to content

Commit

Permalink
Fix issue with ptr_/ref_.ReportIfDangling when ptr_/ref_ are T*/T&
Browse files Browse the repository at this point in the history
`if constexpr` isn't a replacement of #if, therefore both its
subbranches have to be properly formed.

This CL brings back templated code similar to what we had before
crrev.com/c/4220953

This CL is expected to be a no-op.

Change-Id: I98e6041e732f77623548856bf7c4894048f13a4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4328487
Reviewed-by: Paul Semel <paulsemel@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1116204}
  • Loading branch information
bartekn-chromium authored and Chromium LUCI CQ committed Mar 13, 2023
1 parent db6751d commit 3b565ea
Showing 1 changed file with 41 additions and 31 deletions.
72 changes: 41 additions & 31 deletions base/functional/bind_internal.h
Expand Up @@ -149,20 +149,25 @@ class UnretainedWrapper {
template <typename U = T>
explicit UnretainedWrapper(raw_ptr<U, PtrTraits>&& o) : ptr_(std::move(o)) {}

GetPtrType get() const {
// `ptr_` is either a `raw_ptr` or a regular C++ pointer.
if constexpr (IsRawPtrV<StorageType>) {
if constexpr (std::is_same_v<UnretainedTrait,
unretained_traits::MayNotDangle>) {
ptr_.ReportIfDangling();
}
return ptr_;
} else {
return ptr_;
GetPtrType get() const { return GetInternal(ptr_); }

private:
// `ptr_` is either a `raw_ptr` or a regular C++ pointer.
template <typename U>
static GetPtrType GetInternal(U* ptr) {
static_assert(std::is_same_v<T, U>);
return ptr;
}
template <typename U, RawPtrTraits Traits>
static GetPtrType GetInternal(const raw_ptr<U, Traits>& ptr) {
static_assert(std::is_same_v<T, U>);
if constexpr (std::is_same_v<UnretainedTrait,
unretained_traits::MayNotDangle>) {
ptr.ReportIfDangling();
}
return ptr;
}

private:
#if PA_CONFIG(ENABLE_MTE_CHECKED_PTR_SUPPORT_WITH_64_BITS_POINTERS)
// When `MTECheckedPtr` is enabled as the backing implementation of
// `raw_ptr`, there are too many different types that immediately
Expand Down Expand Up @@ -231,29 +236,34 @@ class UnretainedRefWrapper {
explicit UnretainedRefWrapper(raw_ref<U, PtrTraits>&& o)
: ref_(std::move(o)) {}

T& get() const {
// `ref_` is either a `raw_ref` or a regular C++ reference.
if constexpr (IsRawRefV<StorageType>) {
// The ultimate goal is to crash when a callback is invoked with a
// dangling pointer. This is checked here. For now, it is configured to
// either crash, DumpWithoutCrashing or be ignored. This depends on the
// PartitionAllocUnretainedDanglingPtr feature.
if constexpr (std::is_same_v<UnretainedTrait,
unretained_traits::MayNotDangle>) {
ref_.ReportIfDangling();
}
// We can't use operator* here, we need to use raw_ptr's GetForExtraction
// instead of GetForDereference. If we did use GetForDereference then we'd
// crash in ASAN builds on calling a bound callback with a dangling
// reference parameter even if that parameter is not used. This could hide
// a later unprotected issue that would be reached in release builds.
return ref_.get();
} else {
return ref_;
T& get() const { return GetInternal(ref_); }

private:
// `ref_` is either a `raw_ref` or a regular C++ reference.
template <typename U>
static T& GetInternal(U& ref) {
static_assert(std::is_same_v<T, U>);
return ref;
}
template <typename U, RawPtrTraits Traits>
static T& GetInternal(const raw_ref<U, Traits>& ref) {
static_assert(std::is_same_v<T, U>);
// The ultimate goal is to crash when a callback is invoked with a
// dangling pointer. This is checked here. For now, it is configured to
// either crash, DumpWithoutCrashing or be ignored. This depends on the
// PartitionAllocUnretainedDanglingPtr feature.
if constexpr (std::is_same_v<UnretainedTrait,
unretained_traits::MayNotDangle>) {
ref.ReportIfDangling();
}
// We can't use operator* here, we need to use raw_ptr's GetForExtraction
// instead of GetForDereference. If we did use GetForDereference then we'd
// crash in ASAN builds on calling a bound callback with a dangling
// reference parameter even if that parameter is not used. This could hide
// a later unprotected issue that would be reached in release builds.
return ref.get();
}

private:
#if PA_CONFIG(ENABLE_MTE_CHECKED_PTR_SUPPORT_WITH_64_BITS_POINTERS)
// When `MTECheckedPtr` is enabled as the backing implementation of
// `raw_ptr`, there are too many different types that immediately
Expand Down

0 comments on commit 3b565ea

Please sign in to comment.