From 118d84fedd134d7cb960dfcd3550ea943a45d28b Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Fri, 9 Jan 2026 07:44:49 -0800 Subject: [PATCH] [vm/unsendable] Fix field handling in finding retaining path for unsendable values. BUG=https://github.com/dart-lang/sdk/issues/62371 TEST=ci Change-Id: I0fa7a931c4866cb43faee228f2d60f4e701e751a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471621 Commit-Queue: Alexander Aprelev Reviewed-by: Slava Egorov --- .../send_unsupported_objects_test.dart | 8 +- runtime/vm/object_graph_copy.cc | 256 ++++++++---------- 2 files changed, 124 insertions(+), 140 deletions(-) diff --git a/runtime/tests/vm/dart/isolates/send_unsupported_objects_test.dart b/runtime/tests/vm/dart/isolates/send_unsupported_objects_test.dart index a431173f361..d0a2323f90b 100644 --- a/runtime/tests/vm/dart/isolates/send_unsupported_objects_test.dart +++ b/runtime/tests/vm/dart/isolates/send_unsupported_objects_test.dart @@ -28,8 +28,10 @@ class Baz { @pragma('vm:entry-point') // prevent obfuscation class Fu { + static final f = 42; String label; Bar bar = Bar(); + @pragma('vm:entry-point') // prevent tree-shaking of the field. Baz baz = Baz(); Fu(this.label); @@ -70,7 +72,11 @@ main() async { for (final pair in [ [ () => Fu.unsendable('fu'), - ["NativeClass", "Baz", "Fu"], + [ + "Class: NativeClass", + "nativeClass in Instance of 'Baz'", + "baz in Instance of 'Fu'", + ], ], [ () => Future.value(123), diff --git a/runtime/vm/object_graph_copy.cc b/runtime/vm/object_graph_copy.cc index 6ecbeb81ed2..5f55955a86c 100644 --- a/runtime/vm/object_graph_copy.cc +++ b/runtime/vm/object_graph_copy.cc @@ -1112,38 +1112,6 @@ class RetainingPath { const Object& to_; TraversalRules traversal_rules_; - class FindObjectVisitor : public ObjectPointerVisitor { - public: - FindObjectVisitor(IsolateGroup* isolate_group, ObjectPtr target) - : ObjectPointerVisitor(isolate_group), target_(target), index_(0) {} - - void VisitPointers(ObjectPtr* from, ObjectPtr* to) override { - for (ObjectPtr* ptr = from; ptr <= to; ptr++, index_++) { - if (*ptr == target_) { - break; - } - } - } - -#if defined(DART_COMPRESSED_POINTERS) - void VisitCompressedPointers(uword heap_base, - CompressedObjectPtr* from, - CompressedObjectPtr* to) override { - for (CompressedObjectPtr* ptr = from; ptr <= to; ptr++, index_++) { - if (ptr->Decompress(heap_base) == target_) { - break; - } - } - } -#endif - - intptr_t index() { return index_; } - - private: - ObjectPtr target_; - intptr_t index_; - }; - const char* CollectPath(MallocGrowableArray* const working_list) { Object& previous_object = Object::Handle(zone_); Object& object = Object::Handle(zone_); @@ -1152,6 +1120,7 @@ class RetainingPath { Library& library = Library::Handle(zone_); String& library_url = String::Handle(zone_); Context& context = Context::Handle(zone_); + Instance& instance = Instance::Handle(zone_); Closure& closure = Closure::Handle(zone_); Function& function = Function::Handle(zone_); #if !defined(DART_PRECOMPILED_RUNTIME) && !defined(PRODUCT) @@ -1168,130 +1137,139 @@ class RetainingPath { intptr_t saved_context_depth = 0; const char* retaining_path = ""; - ObjectPtr raw = to_.ptr(); + object = to_.ptr(); do { - previous_object = raw; + previous_object = object.ptr(); // Skip all remaining children until null-separator, so we get the parent do { - raw = working_list->RemoveLast(); - } while (raw != Object::null() && raw != from_.ptr()); - if (raw == Object::null()) { - raw = working_list->RemoveLast(); - object = raw; - klass = object.clazz(); + object = working_list->RemoveLast(); + } while (!object.IsNull() && object.ptr() != from_.ptr()); - const char* location = object.ToCString(); + if (!object.IsNull()) { + RELEASE_ASSERT(object.ptr() == from_.ptr()); + break; + } + RELEASE_ASSERT(object.IsNull()); + object = working_list->RemoveLast(); + klass = object.clazz(); - if (object.IsContext()) { - context ^= raw; - if (saved_context_object_index == -1) { - // If this is the first context, remember index of the - // [previous_object] in the Context. - // We will need it later if get to see the Closure next. - saved_context_depth = 0; - for (intptr_t i = 0; i < context.num_variables(); i++) { - if (context.At(i) == previous_object.ptr()) { - saved_context_object_index = i; + const char* location = object.ToCString(); + + if (object.IsContext()) { + context ^= object.ptr(); + if (saved_context_object_index == -1) { + // If this is the first context, remember index of the + // [previous_object] in the Context. + // We will need it later if get to see the Closure next. + saved_context_depth = 0; + for (intptr_t i = 0; i < context.num_variables(); i++) { + if (context.At(i) == previous_object.ptr()) { + saved_context_object_index = i; + break; + } + } + } else { + // Keep track of context depths in case of nested contexts; + saved_context_depth++; + } + } else { + if (object.IsInstance()) { + if (object.IsClosure()) { + closure ^= object.ptr(); + function = closure.function(); + // Use function's class when looking for a library information. + klass = function.Owner(); +#if defined(DART_PRECOMPILED_RUNTIME) || defined(PRODUCT) + // Use function's name instead of closure's. + location = function.QualifiedUserVisibleNameCString(); +#else + // Attempt to convert "instance <- Context+ <- Closure" into + // "instance <- local var name in Closure". + if (function.is_declared_in_bytecode()) { +#if defined(DART_DYNAMIC_MODULES) + bytecode = function.GetBytecode(); + var_descriptors = bytecode.GetLocalVarDescriptors(); +#else + UNREACHABLE(); +#endif // defined(DART_DYNAMIC_MODULES) + } else { + if (!function.ForceOptimize()) { + function.EnsureHasCompiledUnoptimizedCode(); + } + code = function.unoptimized_code(); + ASSERT(!code.IsNull()); + var_descriptors = code.GetLocalVarDescriptors(); + } + for (intptr_t i = 0; i < var_descriptors.Length(); i++) { + UntaggedLocalVarDescriptors::VarInfo info; + var_descriptors.GetInfo(i, &info); + if (info.scope_id == -saved_context_depth && + info.kind() == + UntaggedLocalVarDescriptors::VarInfoKind::kContextVar && + info.index() == saved_context_object_index) { + name ^= var_descriptors.GetName(i); + location = + OS::SCreate(zone_, "field %s in %s", name.ToCString(), + function.QualifiedUserVisibleNameCString()); + // Won't need saved context location after all. + saved_context_location = nullptr; break; } } - } else { - // Keep track of context depths in case of nested contexts; - saved_context_depth++; - } - } else { - if (object.IsInstance()) { - if (object.IsClosure()) { - closure ^= raw; - function = closure.function(); - // Use function's class when looking for a library information. - klass = function.Owner(); -#if defined(DART_PRECOMPILED_RUNTIME) || defined(PRODUCT) - // Use function's name instead of closure's. - location = function.QualifiedUserVisibleNameCString(); -#else - // Attempt to convert "instance <- Context+ <- Closure" into - // "instance <- local var name in Closure". - if (function.is_declared_in_bytecode()) { -#if defined(DART_DYNAMIC_MODULES) - bytecode = function.GetBytecode(); - var_descriptors = bytecode.GetLocalVarDescriptors(); -#else - UNREACHABLE(); -#endif // defined(DART_DYNAMIC_MODULES) - } else { - if (!function.ForceOptimize()) { - function.EnsureHasCompiledUnoptimizedCode(); - } - code = function.unoptimized_code(); - ASSERT(!code.IsNull()); - var_descriptors = code.GetLocalVarDescriptors(); - } - for (intptr_t i = 0; i < var_descriptors.Length(); i++) { - UntaggedLocalVarDescriptors::VarInfo info; - var_descriptors.GetInfo(i, &info); - if (info.scope_id == -saved_context_depth && - info.kind() == - UntaggedLocalVarDescriptors::VarInfoKind::kContextVar && - info.index() == saved_context_object_index) { - name ^= var_descriptors.GetName(i); - location = - OS::SCreate(zone_, "field %s in %s", name.ToCString(), - function.QualifiedUserVisibleNameCString()); - // Won't need saved context location after all. - saved_context_location = nullptr; - break; - } - } #endif // defined(DART_PRECOMPILED_RUNTIME) || defined(PRODUCT) - } else { - // Attempt to find field name for the field that holds the - // [previous_object] instance. - FindObjectVisitor visitor(thread_->isolate_group(), - previous_object.ptr()); - raw->untag()->VisitPointers(&visitor); - field ^= klass.FieldFromIndex(visitor.index()); - if (!field.IsNull()) { + } else { + instance ^= object.ptr(); + // Attempt to find field name for the field that holds the + // [previous_object] instance. + const Array& fields = Array::Handle(klass.OffsetToFieldMap()); + for (intptr_t i = 0; i < fields.Length(); i++) { + if (fields.At(i) == Field::null()) { + continue; + } + field ^= fields.At(i); + + if (instance.GetField(field) == previous_object.ptr()) { location = OS::SCreate(zone_, "%s in %s", field.UserVisibleNameCString(), location); + break; } } } - // Saved context object index stays up for only one cycle - just to - // accommodate short chains Closure -> Context -> instance. - saved_context_object_index = -1; - saved_context_depth = -1; - } - // Add library url to the location if library is available. - library = klass.library(); - if (!library.IsNull()) { - library_url = library.url(); - location = OS::SCreate(zone_, "%s (from %s)", location, - library_url.ToCString()); - } - - if (object.IsContext()) { - // Save context string placeholder in case we don't find closure next - if (saved_context_location == nullptr) { - saved_context_location = location; - } else { - // Append saved contexts - saved_context_location = OS::SCreate( - zone_, "%s <- %s\n", saved_context_location, location); - } - } else { - if (saved_context_location != nullptr) { - // Could not use saved context, insert it into retaining path now. - retaining_path = OS::SCreate(zone_, "%s <- %s", retaining_path, - saved_context_location); - saved_context_location = nullptr; - } - retaining_path = - OS::SCreate(zone_, "%s <- %s\n", retaining_path, location); } + // Saved context object index stays up for only one cycle - just to + // accommodate short chains Closure -> Context -> instance. + saved_context_object_index = -1; + saved_context_depth = -1; } - } while (raw != from_.ptr()); + // Add library url to the location if library is available. + library = klass.library(); + if (!library.IsNull()) { + library_url = library.url(); + location = OS::SCreate(zone_, "%s (from %s)", location, + library_url.ToCString()); + } + + if (object.IsContext()) { + // Save context string placeholder in case we don't find closure next + if (saved_context_location == nullptr) { + saved_context_location = location; + } else { + // Append saved contexts + saved_context_location = OS::SCreate( + zone_, "%s <- %s\n", saved_context_location, location); + } + } else { + if (saved_context_location != nullptr) { + // Could not use saved context, insert it into retaining path now. + retaining_path = OS::SCreate(zone_, "%s <- %s", retaining_path, + saved_context_location); + saved_context_location = nullptr; + } + retaining_path = + OS::SCreate(zone_, "%s <- %s\n", retaining_path, location); + } + } while (object.ptr() != from_.ptr()); ASSERT(working_list->is_empty()); return retaining_path; }