Commit c29395ae authored by Michael Lippautz's avatar Michael Lippautz Committed by V8 LUCI CQ

cppgc: Relax Member checks

Member is sometimes still used from off-heap storage which prohibits
getting the heap from the Member's slot address.

Bug: v8:11756
Change-Id: I61658ce07a8b02a8c400232ff21c75f0d8b95dcb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2886879
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: 's avatarAnton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74496}
parent 8a9129ef
...@@ -50,11 +50,8 @@ struct NoWriteBarrierPolicy { ...@@ -50,11 +50,8 @@ struct NoWriteBarrierPolicy {
static void AssigningBarrier(const void*, const void*) {} static void AssigningBarrier(const void*, const void*) {}
}; };
class V8_EXPORT EnabledCheckingPolicyBase { class V8_EXPORT EnabledCheckingPolicy {
protected: protected:
EnabledCheckingPolicyBase() = default;
explicit EnabledCheckingPolicyBase(void* state) : state_(state) {}
template <typename T> template <typename T>
void CheckPointer(const T* ptr) { void CheckPointer(const T* ptr) {
if (!ptr || (kSentinelPointer == ptr)) return; if (!ptr || (kSentinelPointer == ptr)) return;
...@@ -67,14 +64,14 @@ class V8_EXPORT EnabledCheckingPolicyBase { ...@@ -67,14 +64,14 @@ class V8_EXPORT EnabledCheckingPolicyBase {
template <typename T, bool = IsCompleteV<T>> template <typename T, bool = IsCompleteV<T>>
struct CheckPointersImplTrampoline { struct CheckPointersImplTrampoline {
static void Call(EnabledCheckingPolicyBase* policy, const T* ptr) { static void Call(EnabledCheckingPolicy* policy, const T* ptr) {
policy->CheckPointerImpl(ptr, false); policy->CheckPointerImpl(ptr, false);
} }
}; };
template <typename T> template <typename T>
struct CheckPointersImplTrampoline<T, true> { struct CheckPointersImplTrampoline<T, true> {
static void Call(EnabledCheckingPolicyBase* policy, const T* ptr) { static void Call(EnabledCheckingPolicy* policy, const T* ptr) {
policy->CheckPointerImpl(ptr, IsGarbageCollectedTypeV<T>); policy->CheckPointerImpl(ptr, IsGarbageCollectedTypeV<T>);
} }
}; };
...@@ -82,22 +79,14 @@ class V8_EXPORT EnabledCheckingPolicyBase { ...@@ -82,22 +79,14 @@ class V8_EXPORT EnabledCheckingPolicyBase {
void* state_ = nullptr; void* state_ = nullptr;
}; };
class V8_EXPORT EnabledMemberCheckingPolicy : public EnabledCheckingPolicyBase {
protected:
EnabledMemberCheckingPolicy();
};
class V8_EXPORT EnabledPersistentCheckingPolicy
: public EnabledCheckingPolicyBase {};
class DisabledCheckingPolicy { class DisabledCheckingPolicy {
protected: protected:
void CheckPointer(const void* raw) {} void CheckPointer(const void* raw) {}
}; };
#if V8_ENABLE_CHECKS #if V8_ENABLE_CHECKS
using DefaultMemberCheckingPolicy = EnabledMemberCheckingPolicy; using DefaultMemberCheckingPolicy = EnabledCheckingPolicy;
using DefaultPersistentCheckingPolicy = EnabledPersistentCheckingPolicy; using DefaultPersistentCheckingPolicy = EnabledCheckingPolicy;
#else #else
using DefaultMemberCheckingPolicy = DisabledCheckingPolicy; using DefaultMemberCheckingPolicy = DisabledCheckingPolicy;
using DefaultPersistentCheckingPolicy = DisabledCheckingPolicy; using DefaultPersistentCheckingPolicy = DisabledCheckingPolicy;
......
...@@ -18,39 +18,17 @@ namespace internal { ...@@ -18,39 +18,17 @@ namespace internal {
namespace { namespace {
#if defined(CPPGC_CAGED_HEAP) || defined(DEBUG) #if defined(DEBUG)
bool IsOnStack(const void* address) { bool IsOnStack(const void* address) {
return v8::base::Stack::GetCurrentStackPosition() <= address && return v8::base::Stack::GetCurrentStackPosition() <= address &&
address < v8::base::Stack::GetStackStart(); address < v8::base::Stack::GetStackStart();
} }
#endif // defined(CPPGC_CAGED_HEAP) || defined(DEBUG) #endif // defined(DEBUG)
// Gets the state (HeapBase) for on-heap slots.
void* TryGetStateFromSlot(void* slot) {
#ifdef CPPGC_CAGED_HEAP
if (IsOnStack(slot)) return nullptr;
// `slot` may reside in a regular or large object. Get to the heap using the
// cage.
return reinterpret_cast<CagedHeapLocalData*>(
reinterpret_cast<uintptr_t>(slot) &
~(api_constants::kCagedHeapReservationAlignment - 1))
->heap_base;
#else // !CPPGC_CAGED_HEAP
return nullptr;
#endif // !CPPGC_CAGED_HEAP
}
} // namespace } // namespace
// We know that Member is only allowed on heap and on-stack in rare cases. Use void EnabledCheckingPolicy::CheckPointerImpl(const void* ptr,
// this information to eagerly populate a verification state already on policy bool points_to_payload) {
// creation.
EnabledMemberCheckingPolicy::EnabledMemberCheckingPolicy()
: EnabledCheckingPolicyBase(TryGetStateFromSlot(this)) {}
void EnabledCheckingPolicyBase::CheckPointerImpl(const void* ptr,
bool points_to_payload) {
// `ptr` must not reside on stack. // `ptr` must not reside on stack.
DCHECK(!IsOnStack(ptr)); DCHECK(!IsOnStack(ptr));
auto* base_page = BasePage::FromPayload(ptr); auto* base_page = BasePage::FromPayload(ptr);
......
...@@ -535,21 +535,6 @@ TEST_F(MemberHeapDeathTest, AssignDifferentHeapValues) { ...@@ -535,21 +535,6 @@ TEST_F(MemberHeapDeathTest, AssignDifferentHeapValues) {
} }
} }
#ifdef CPPGC_CAGED_HEAP
TEST_F(MemberHeapDeathTest, VerificationStateDoesNotRequireValue) {
// For caged heap setups the verification state is constructed from Member
// itself and does not require an initial value.
auto* o1 = MakeGarbageCollected<LinkedNode>(GetAllocationHandle(), nullptr);
{
auto tmp_heap = cppgc::Heap::Create(platform_);
EXPECT_DEATH_IF_SUPPORTED(
MakeGarbageCollected<LinkedNode>(tmp_heap->GetAllocationHandle(), o1),
"");
}
}
#endif // CPPGC_CAGED_HEAP
#endif // V8_ENABLE_CHECKS #endif // V8_ENABLE_CHECKS
} // namespace internal } // namespace internal
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment