From 4ac96c3ff8b60ae2643ad63fdf47c3c97498c7bc Mon Sep 17 00:00:00 2001 From: Shu-yu Guo Date: Tue, 1 Nov 2022 14:44:29 -0700 Subject: [PATCH] [debug] Use context isolate when creating PropertyIterator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Objects in the shared heap do not have a usable Isolate (i.e. it cannot execute code or have HandleScopes). PropertyIterator should be using the currently executing Isolate via the Context instead. Bug: chromium:1379616 Change-Id: I7ac87519ef4aa901ef7b71e00f98c2cba66e725b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3997702 Reviewed-by: Simon Zünd Commit-Queue: Shu-yu Guo Cr-Commit-Position: refs/heads/main@{#84052} --- src/debug/debug-interface.cc | 2 +- .../debug/debug-property-iterator-unittest.cc | 34 +++++++++++++++++++ test/unittests/test-utils.h | 12 +++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/debug/debug-interface.cc b/src/debug/debug-interface.cc index eac16601bf..b7e1e397f9 100644 --- a/src/debug/debug-interface.cc +++ b/src/debug/debug-interface.cc @@ -1404,7 +1404,7 @@ void NotifyDebuggerPausedEventSent(v8::Isolate* v8_isolate) { std::unique_ptr PropertyIterator::Create( Local context, Local object, bool skip_indices) { internal::Isolate* isolate = - reinterpret_cast(object->GetIsolate()); + reinterpret_cast(context->GetIsolate()); if (isolate->is_execution_terminating()) { return nullptr; } diff --git a/test/unittests/debug/debug-property-iterator-unittest.cc b/test/unittests/debug/debug-property-iterator-unittest.cc index feb4f4f2ad..54fc761355 100644 --- a/test/unittests/debug/debug-property-iterator-unittest.cc +++ b/test/unittests/debug/debug-property-iterator-unittest.cc @@ -148,6 +148,40 @@ TEST_F(DebugPropertyIteratorTest, SkipsIndicesOnTypedArrays) { } } +#if V8_CAN_CREATE_SHARED_HEAP_BOOL + +using SharedObjectDebugPropertyIteratorTest = TestJSSharedMemoryWithContext; + +TEST_F(SharedObjectDebugPropertyIteratorTest, SharedStruct) { + TryCatch try_catch(isolate()); + + const char source_text[] = + "let S = new SharedStructType(['field', 'another_field']);" + "new S();"; + + auto shared_struct = + RunJS(context(), source_text)->ToObject(context()).ToLocalChecked(); + auto iterator = PropertyIterator::Create(context(), shared_struct); + + ASSERT_NE(iterator, nullptr); + ASSERT_FALSE(iterator->Done()); + EXPECT_TRUE(iterator->is_own()); + char name_buffer[64]; + iterator->name().As()->WriteUtf8(isolate(), name_buffer); + EXPECT_EQ("field", std::string(name_buffer)); + ASSERT_TRUE(iterator->Advance().FromMaybe(false)); + + ASSERT_FALSE(iterator->Done()); + EXPECT_TRUE(iterator->is_own()); + iterator->name().As()->WriteUtf8(isolate(), name_buffer); + EXPECT_EQ("another_field", std::string(name_buffer)); + ASSERT_TRUE(iterator->Advance().FromMaybe(false)); + + ASSERT_FALSE(iterator->Done()); +} + +#endif // V8_CAN_CREATE_SHARED_HEAP_BOOL + } // namespace } // namespace debug } // namespace v8 diff --git a/test/unittests/test-utils.h b/test/unittests/test-utils.h index bb693f1695..7860ed5815 100644 --- a/test/unittests/test-utils.h +++ b/test/unittests/test-utils.h @@ -300,6 +300,18 @@ using TestWithContext = // WithDefaultPlatformMixin< // ::testing::Test>>>>; +// Use v8::internal::TestJSSharedMemoryWithNativeContext if you are testing +// internals, aka. directly work with Handles. +// +// Using this will FATAL when !V8_CAN_CREATE_SHARED_HEAP_BOOL +using TestJSSharedMemoryWithContext = // + WithContextMixin< // + WithIsolateScopeMixin< // + WithIsolateMixin< // + WithDefaultPlatformMixin< // + WithJSSharedMemoryFeatureFlagsMixin< // + ::testing::Test>>>>>; + class PrintExtension : public v8::Extension { public: PrintExtension() : v8::Extension("v8/print", "native function print();") {}