cppgc: Prevent misuse of explicit_management.h

The methods in explicit_management.h should be called via the public
variants in the subtle namespace. Calling the variants in the internal
namespace directly skips asserts and required size coversions. Doing so
may cause misuse of the api that may break GC inernals

Change-Id: I58a0f324ca1ee0839bb85eb9b53ce57785dc7b91
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3435187
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78920}
This commit is contained in:
Omer Katz 2022-02-03 10:33:39 +01:00 committed by V8 LUCI CQ
parent 9ae463bc43
commit ef53e0a0d2
3 changed files with 26 additions and 7 deletions

View File

@ -15,11 +15,27 @@ namespace cppgc {
class HeapHandle; class HeapHandle;
namespace subtle {
template <typename T>
void FreeUnreferencedObject(HeapHandle& heap_handle, T& object);
template <typename T>
bool Resize(T& object, AdditionalBytes additional_bytes);
} // namespace subtle
namespace internal { namespace internal {
V8_EXPORT void FreeUnreferencedObject(HeapHandle&, void*); class ExplicitManagementImpl final {
V8_EXPORT bool Resize(void*, size_t); private:
V8_EXPORT static void FreeUnreferencedObject(HeapHandle&, void*);
V8_EXPORT static bool Resize(void*, size_t);
template <typename T>
friend void subtle::FreeUnreferencedObject(HeapHandle&, T&);
template <typename T>
friend bool subtle::Resize(T&, AdditionalBytes);
};
} // namespace internal } // namespace internal
namespace subtle { namespace subtle {
@ -45,7 +61,8 @@ template <typename T>
void FreeUnreferencedObject(HeapHandle& heap_handle, T& object) { void FreeUnreferencedObject(HeapHandle& heap_handle, T& object) {
static_assert(IsGarbageCollectedTypeV<T>, static_assert(IsGarbageCollectedTypeV<T>,
"Object must be of type GarbageCollected."); "Object must be of type GarbageCollected.");
internal::FreeUnreferencedObject(heap_handle, &object); internal::ExplicitManagementImpl::FreeUnreferencedObject(heap_handle,
&object);
} }
/** /**
@ -73,7 +90,8 @@ template <typename T>
bool Resize(T& object, AdditionalBytes additional_bytes) { bool Resize(T& object, AdditionalBytes additional_bytes) {
static_assert(IsGarbageCollectedTypeV<T>, static_assert(IsGarbageCollectedTypeV<T>,
"Object must be of type GarbageCollected."); "Object must be of type GarbageCollected.");
return internal::Resize(&object, sizeof(T) + additional_bytes.value); return internal::ExplicitManagementImpl::Resize(
&object, sizeof(T) + additional_bytes.value);
} }
} // namespace subtle } // namespace subtle

View File

@ -47,7 +47,8 @@ void InvalidateRememberedSlots(HeapBase& heap, void* begin, void* end) {
} // namespace } // namespace
void FreeUnreferencedObject(HeapHandle& heap_handle, void* object) { void ExplicitManagementImpl::FreeUnreferencedObject(HeapHandle& heap_handle,
void* object) {
if (InGC(heap_handle)) { if (InGC(heap_handle)) {
return; return;
} }
@ -144,7 +145,7 @@ bool Shrink(HeapObjectHeader& header, BasePage& base_page, size_t new_size,
} // namespace } // namespace
bool Resize(void* object, size_t new_object_size) { bool ExplicitManagementImpl::Resize(void* object, size_t new_object_size) {
// `object` is guaranteed to be of type GarbageCollected, so getting the // `object` is guaranteed to be of type GarbageCollected, so getting the
// BasePage is okay for regular and large objects. // BasePage is okay for regular and large objects.
BasePage* base_page = BasePage::FromPayload(object); BasePage* base_page = BasePage::FromPayload(object);

View File

@ -113,7 +113,7 @@ TEST_F(UnifiedHeapTest, FreeUnreferencedDuringNoGcScope) {
cpp_heap().stats_collector()->NotifySafePointForTesting(); cpp_heap().stats_collector()->NotifySafePointForTesting();
{ {
cppgc::subtle::NoGarbageCollectionScope no_gc_scope(cpp_heap()); cppgc::subtle::NoGarbageCollectionScope no_gc_scope(cpp_heap());
cppgc::internal::FreeUnreferencedObject(cpp_heap(), unreferenced); cppgc::subtle::FreeUnreferencedObject(cpp_heap(), *unreferenced);
// Force safepoint to make sure allocated size decrease due to freeing // Force safepoint to make sure allocated size decrease due to freeing
// unreferenced object is reported to CppHeap. Due to // unreferenced object is reported to CppHeap. Due to
// NoGarbageCollectionScope, CppHeap will cache the reported decrease and // NoGarbageCollectionScope, CppHeap will cache the reported decrease and