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

cppgc: Implement basic Member and Persistent checks

Adds check for
- same heap on assignment
- header and containment

The verification state is eagerly created for on-heap Member
references using caged heap and lazily created on first assignment for
all others.

Bug: chromium:1056170
Change-Id: I38ee18eeb7ac489f69a46670cc5e5abe07f62dfa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2878745Reviewed-by: 's avatarOmer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74449}
parent 1f504c36
......@@ -139,7 +139,7 @@ class BasicCrossThreadPersistent final : public PersistentBase,
GetNode()->UpdateOwner(this);
other.SetValue(nullptr);
other.SetNode(nullptr);
this->CheckPointer(GetValue());
this->CheckPointer(Get());
return *this;
}
......
......@@ -9,7 +9,9 @@
#include <type_traits>
#include "cppgc/internal/write-barrier.h"
#include "cppgc/sentinel-pointer.h"
#include "cppgc/source-location.h"
#include "cppgc/type-traits.h"
#include "v8config.h" // NOLINT(build/include_directory)
namespace cppgc {
......@@ -48,24 +50,57 @@ struct NoWriteBarrierPolicy {
static void AssigningBarrier(const void*, const void*) {}
};
class V8_EXPORT EnabledCheckingPolicy {
class V8_EXPORT EnabledCheckingPolicyBase {
protected:
EnabledCheckingPolicy();
void CheckPointer(const void* ptr);
EnabledCheckingPolicyBase() = default;
explicit EnabledCheckingPolicyBase(void* state) : state_(state) {}
template <typename T>
void CheckPointer(const T* ptr) {
if (!ptr || (kSentinelPointer == ptr)) return;
CheckPointersImplTrampoline<T>::Call(this, ptr);
}
private:
void* impl_;
void CheckPointerImpl(const void* ptr, bool points_to_payload);
template <typename T, bool = IsCompleteV<T>>
struct CheckPointersImplTrampoline {
static void Call(EnabledCheckingPolicyBase* policy, const T* ptr) {
policy->CheckPointerImpl(ptr, false);
}
};
template <typename T>
struct CheckPointersImplTrampoline<T, true> {
static void Call(EnabledCheckingPolicyBase* policy, const T* ptr) {
policy->CheckPointerImpl(ptr, IsGarbageCollectedTypeV<T>);
}
};
void* state_ = nullptr;
};
class V8_EXPORT EnabledMemberCheckingPolicy : public EnabledCheckingPolicyBase {
protected:
EnabledMemberCheckingPolicy();
};
class V8_EXPORT EnabledPersistentCheckingPolicy
: public EnabledCheckingPolicyBase {};
class DisabledCheckingPolicy {
protected:
void CheckPointer(const void* raw) {}
};
#if V8_ENABLE_CHECKS
using DefaultCheckingPolicy = EnabledCheckingPolicy;
using DefaultMemberCheckingPolicy = EnabledMemberCheckingPolicy;
using DefaultPersistentCheckingPolicy = EnabledPersistentCheckingPolicy;
#else
using DefaultCheckingPolicy = DisabledCheckingPolicy;
using DefaultMemberCheckingPolicy = DisabledCheckingPolicy;
using DefaultPersistentCheckingPolicy = DisabledCheckingPolicy;
#endif
class KeepLocationPolicy {
......@@ -133,10 +168,10 @@ template <typename T, typename WeaknessPolicy,
class BasicCrossThreadPersistent;
template <typename T, typename WeaknessPolicy,
typename LocationPolicy = DefaultLocationPolicy,
typename CheckingPolicy = DefaultCheckingPolicy>
typename CheckingPolicy = DefaultPersistentCheckingPolicy>
class BasicPersistent;
template <typename T, typename WeaknessTag, typename WriteBarrierPolicy,
typename CheckingPolicy = DefaultCheckingPolicy>
typename CheckingPolicy = DefaultMemberCheckingPolicy>
class BasicMember;
} // namespace internal
......
......@@ -7,6 +7,7 @@
// This file should stay with minimal dependencies to allow embedder to check
// against Oilpan types without including any other parts.
#include <cstddef>
#include <type_traits>
namespace cppgc {
......@@ -164,6 +165,18 @@ struct IsUntracedMemberType : std::false_type {};
template <typename T>
struct IsUntracedMemberType<T, true> : std::true_type {};
template <typename T>
struct IsComplete {
private:
template <typename U, size_t = sizeof(U)>
static std::true_type IsSizeOfKnown(U*);
static std::false_type IsSizeOfKnown(...);
public:
static constexpr bool value =
decltype(IsSizeOfKnown(std::declval<T*>()))::value;
};
} // namespace internal
/**
......@@ -223,6 +236,12 @@ constexpr bool IsWeakMemberTypeV = internal::IsWeakMemberType<T>::value;
template <typename T>
constexpr bool IsWeakV = internal::IsWeak<T>::value;
/**
* Value is true for types that are complete, and false otherwise.
*/
template <typename T>
constexpr bool IsCompleteV = internal::IsComplete<T>::value;
} // namespace cppgc
#endif // INCLUDE_CPPGC_TYPE_TRAITS_H_
......@@ -4,21 +4,85 @@
#include "include/cppgc/internal/pointer-policies.h"
#include "include/cppgc/internal/caged-heap-local-data.h"
#include "include/cppgc/internal/persistent-node.h"
#include "src/base/logging.h"
#include "src/base/macros.h"
#include "src/base/platform/platform.h"
#include "src/heap/cppgc/heap-object-header.h"
#include "src/heap/cppgc/heap-page.h"
#include "src/heap/cppgc/heap.h"
namespace cppgc {
namespace internal {
EnabledCheckingPolicy::EnabledCheckingPolicy() {
USE(impl_);
// TODO(chromium:1056170): Save creating heap state.
namespace {
// Gets the state (HeapBase) for on-heap slots.
void* TryGetStateFromSlot(void* slot) {
#ifdef CPPGC_CAGED_HEAP
if (v8::base::Stack::GetCurrentStackPosition() <= slot &&
slot < v8::base::Stack::GetStackStart())
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
}
void EnabledCheckingPolicy::CheckPointer(const void* ptr) {
// TODO(chromium:1056170): Provide implementation.
} // namespace
// We know that Member is only allowed on heap and on-stack in rare cases. Use
// this information to eagerly populate a verification state already on policy
// creation.
EnabledMemberCheckingPolicy::EnabledMemberCheckingPolicy()
: EnabledCheckingPolicyBase(TryGetStateFromSlot(this)) {}
void EnabledCheckingPolicyBase::CheckPointerImpl(const void* ptr,
bool points_to_payload) {
auto* base_page = BasePage::FromPayload(ptr);
// Large objects do not support mixins. This also means that `base_page` is
// valid for large objects.
DCHECK_IMPLIES(base_page->is_large(), points_to_payload);
if (!state_) {
state_ = base_page->heap();
// Member references are used from within objects that cannot change their
// heap association which means that state is immutable once it is set.
//
// TODO(chromium:1056170): Binding state late allows for getting the initial
// state wrong which requires a check that `this` is contained in heap that
// is itself expensive. Investigate options on non-caged builds to improve
// coverage.
}
HeapBase* heap = static_cast<HeapBase*>(state_);
if (!heap) return;
// Member references should never mix heaps.
DCHECK_EQ(heap, base_page->heap());
// Header checks.
const HeapObjectHeader* header = nullptr;
if (points_to_payload) {
header = &HeapObjectHeader::FromPayload(ptr);
} else if (!heap->sweeper().IsSweepingInProgress()) {
// Mixin case.
header = &base_page->ObjectHeaderFromInnerAddress(ptr);
DCHECK_LE(header->Payload(), ptr);
DCHECK_GT(header->PayloadEnd(), ptr);
}
if (header) {
DCHECK(!header->IsFree());
}
// TODO(v8:11749): Check mark bits when during pre-finalizer phase.
}
PersistentRegion& StrongPersistentPolicy::GetPersistentRegion(
......
......@@ -84,6 +84,17 @@ TEST(GarbageCollectedTest, GarbageCollectedWithMixinTrait) {
STATIC_ASSERT(IsGarbageCollectedWithMixinTypeV<GCWithMergedMixins>);
}
namespace {
class ForwardDeclaredType;
} // namespace
TEST(GarbageCollectedTest, CompleteTypeTrait) {
STATIC_ASSERT(IsCompleteV<GCed>);
STATIC_ASSERT(!IsCompleteV<ForwardDeclaredType>);
}
TEST_F(GarbageCollectedTestWithHeap, GetObjectStartReturnsCurrentAddress) {
GCed* gced = MakeGarbageCollected<GCed>(GetAllocationHandle());
GCedWithMixin* gced_with_mixin =
......
......@@ -506,5 +506,51 @@ TEST_F(MemberHeapTest, ConstWeakRefIsClearedOnGC) {
EXPECT_FALSE(persistent->weak_member());
}
#if V8_ENABLE_CHECKS
namespace {
class MemberHeapDeathTest : public testing::TestWithHeap {};
class LinkedNode final : public GarbageCollected<LinkedNode> {
public:
explicit LinkedNode(LinkedNode* next) : next_(next) {}
void Trace(Visitor* v) const { v->Trace(next_); }
void SetNext(LinkedNode* next) { next_ = next; }
private:
Member<LinkedNode> next_;
};
} // namespace
TEST_F(MemberHeapDeathTest, AssignDifferentHeapValues) {
auto* o1 = MakeGarbageCollected<LinkedNode>(GetAllocationHandle(), nullptr);
auto* o2 = MakeGarbageCollected<LinkedNode>(GetAllocationHandle(), o1);
{
auto tmp_heap = cppgc::Heap::Create(platform_);
auto* o3 = MakeGarbageCollected<LinkedNode>(tmp_heap->GetAllocationHandle(),
nullptr);
EXPECT_DEATH_IF_SUPPORTED(o2->SetNext(o3), "");
}
}
#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
} // namespace internal
} // namespace cppgc
......@@ -74,12 +74,12 @@ template <typename T>
using LocalizedPersistent =
internal::BasicPersistent<T, internal::StrongPersistentPolicy,
internal::KeepLocationPolicy,
internal::DefaultCheckingPolicy>;
internal::DefaultPersistentCheckingPolicy>;
template <typename T>
using LocalizedCrossThreadPersistent = internal::BasicCrossThreadPersistent<
T, internal::StrongCrossThreadPersistentPolicy,
internal::KeepLocationPolicy, internal::DefaultCheckingPolicy>;
internal::KeepLocationPolicy, internal::DisabledCheckingPolicy>;
class RootVisitor final : public VisitorBase {
public:
......
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