Commit 95885659 authored by Fergus Dall's avatar Fergus Dall Committed by V8 LUCI CQ

Revert "Reland "cppgc: Enable checks for assignments in prefinalizers""

This reverts commit adb6276f.

Reason for revert: Broke several blink unit tests, see
https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-dbg/25255/overview
for an example

Original change's description:
> Reland "cppgc: Enable checks for assignments in prefinalizers"
>
> This is a reland of edcc8ff5
>
> Cause for previous revert was addressed by crbug.com/1241773.
>
> Original change's description:
> > cppgc: Enable checks for assignments in prefinalizers
> >
> > Bug: v8:11749
> > Change-Id: Ic027f732030fb6a2befeffeca9db2eacfd0830a5
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3099953
> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> > Commit-Queue: Omer Katz <omerkatz@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#76370}
>
> Bug: v8:11749
> Change-Id: I57fc138ace002d41e54f7f70250e4d19bc9262b0
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3122153
> Auto-Submit: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Omer Katz <omerkatz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#76553}

Bug: v8:11749
Change-Id: Icc6a3e56d54c22de943b498c2fd6d57f3ef33f96
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3128562
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Fergus Dall <sidereal@google.com>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: 's avatarOmer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76558}
parent 7c6684af
...@@ -165,6 +165,7 @@ config_setting( ...@@ -165,6 +165,7 @@ config_setting(
# v8_control_flow_integrity # v8_control_flow_integrity
# v8_enable_virtual_memory_cage # v8_enable_virtual_memory_cage
# cppgc_enable_caged_heap # cppgc_enable_caged_heap
# cppgc_enable_check_assignments_in_prefinalizers
# cppgc_enable_object_names # cppgc_enable_object_names
# cppgc_enable_verify_heap # cppgc_enable_verify_heap
# cppgc_enable_young_generation # cppgc_enable_young_generation
......
...@@ -293,6 +293,10 @@ declare_args() { ...@@ -293,6 +293,10 @@ declare_args() {
# Enables additional heap verification phases and checks. # Enables additional heap verification phases and checks.
cppgc_enable_verify_heap = "" cppgc_enable_verify_heap = ""
# Enable assignment checks for Members/Persistents during prefinalizer invocations.
# TODO(v8:11749): Enable by default after fixing any existing issues in Blink.
cppgc_enable_check_assignments_in_prefinalizers = false
# Enable young generation in cppgc. # Enable young generation in cppgc.
cppgc_enable_young_generation = false cppgc_enable_young_generation = false
...@@ -776,6 +780,10 @@ config("features") { ...@@ -776,6 +780,10 @@ config("features") {
defines += [ "CPPGC_VERIFY_HEAP" ] defines += [ "CPPGC_VERIFY_HEAP" ]
} }
if (cppgc_enable_check_assignments_in_prefinalizers) {
defines += [ "CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS" ]
}
if (v8_embedder_string != "") { if (v8_embedder_string != "") {
defines += [ "V8_EMBEDDER_STRING=\"$v8_embedder_string\"" ] defines += [ "V8_EMBEDDER_STRING=\"$v8_embedder_string\"" ]
} }
......
...@@ -68,7 +68,7 @@ void EnabledCheckingPolicy::CheckPointerImpl(const void* ptr, ...@@ -68,7 +68,7 @@ void EnabledCheckingPolicy::CheckPointerImpl(const void* ptr,
DCHECK(!header->IsFree()); DCHECK(!header->IsFree());
} }
#ifdef CPPGC_VERIFY_HEAP #ifdef CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS
if (heap_->prefinalizer_handler()->IsInvokingPreFinalizers()) { if (heap_->prefinalizer_handler()->IsInvokingPreFinalizers()) {
// During prefinalizers invocation, check that |ptr| refers to a live object // During prefinalizers invocation, check that |ptr| refers to a live object
// and that it is assigned to a live slot. // and that it is assigned to a live slot.
...@@ -81,7 +81,7 @@ void EnabledCheckingPolicy::CheckPointerImpl(const void* ptr, ...@@ -81,7 +81,7 @@ void EnabledCheckingPolicy::CheckPointerImpl(const void* ptr,
DCHECK(slot_is_live); DCHECK(slot_is_live);
USE(slot_is_live); USE(slot_is_live);
} }
#endif // CPPGC_VERIFY_HEAP #endif // CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS
} }
PersistentRegion& StrongPersistentPolicy::GetPersistentRegion( PersistentRegion& StrongPersistentPolicy::GetPersistentRegion(
......
...@@ -285,7 +285,7 @@ class GCedHolder : public GarbageCollected<GCedHolder> { ...@@ -285,7 +285,7 @@ class GCedHolder : public GarbageCollected<GCedHolder> {
} // namespace } // namespace
#if V8_ENABLE_CHECKS #if V8_ENABLE_CHECKS
#ifdef CPPGC_VERIFY_HEAP #ifdef CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS
TEST_F(PrefinalizerDeathTest, PrefinalizerCantRewireGraphWithDeadObjects) { TEST_F(PrefinalizerDeathTest, PrefinalizerCantRewireGraphWithDeadObjects) {
Persistent<LinkedNode> root{MakeGarbageCollected<LinkedNode>( Persistent<LinkedNode> root{MakeGarbageCollected<LinkedNode>(
...@@ -318,7 +318,7 @@ TEST_F(PrefinalizerDeathTest, PrefinalizerCantRessurectObjectOnHeap) { ...@@ -318,7 +318,7 @@ TEST_F(PrefinalizerDeathTest, PrefinalizerCantRessurectObjectOnHeap) {
EXPECT_DEATH_IF_SUPPORTED(PreciseGC(), ""); EXPECT_DEATH_IF_SUPPORTED(PreciseGC(), "");
} }
#endif // CPPGC_VERIFY_HEAP #endif // CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS
#endif // V8_ENABLE_CHECKS #endif // V8_ENABLE_CHECKS
} // namespace internal } // namespace internal
......
...@@ -245,6 +245,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON) ...@@ -245,6 +245,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
option(CPPGC_ENABLE_OBJECT_NAMES "Enable object names in cppgc for debug purposes" OFF) option(CPPGC_ENABLE_OBJECT_NAMES "Enable object names in cppgc for debug purposes" OFF)
option(CPPGC_ENABLE_CAGED_HEAP "Enable heap reservation of size 4GB, only possible for 64bit archs" OFF) option(CPPGC_ENABLE_CAGED_HEAP "Enable heap reservation of size 4GB, only possible for 64bit archs" OFF)
option(CPPGC_ENABLE_VERIFY_HEAP "Enables additional heap verification phases and checks" OFF) option(CPPGC_ENABLE_VERIFY_HEAP "Enables additional heap verification phases and checks" OFF)
option(CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS " Enable assignment checks for Members/Persistents during prefinalizer invocations" OFF)
option(CPPGC_ENABLE_YOUNG_GENERATION "Enable young generation in cppgc" OFF) option(CPPGC_ENABLE_YOUNG_GENERATION "Enable young generation in cppgc" OFF)
set(CPPGC_TARGET_ARCH "x64" CACHE STRING "Target architecture, possible options: x64, x86, arm, arm64, ppc64, s390x, mipsel, mips64el") set(CPPGC_TARGET_ARCH "x64" CACHE STRING "Target architecture, possible options: x64, x86, arm, arm64, ppc64, s390x, mipsel, mips64el")
...@@ -437,6 +438,9 @@ endif() ...@@ -437,6 +438,9 @@ endif()
if(CPPGC_ENABLE_VERIFY_HEAP) if(CPPGC_ENABLE_VERIFY_HEAP)
target_compile_definitions({target.name} PRIVATE "-DCPPGC_ENABLE_VERIFY_HEAP") target_compile_definitions({target.name} PRIVATE "-DCPPGC_ENABLE_VERIFY_HEAP")
endif() endif()
if(CPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS)
target_compile_definitions({target.name} PRIVATE "-DCPPGC_CHECK_ASSIGNMENTS_IN_PREFINALIZERS")
endif()
if(CPPGC_ENABLE_YOUNG_GENERATION) if(CPPGC_ENABLE_YOUNG_GENERATION)
target_compile_definitions({target.name} PRIVATE "-DCPPGC_YOUNG_GENERATION") target_compile_definitions({target.name} PRIVATE "-DCPPGC_YOUNG_GENERATION")
endif()""" endif()"""
......
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