From cdf7bf9c7884193aac78a478fb843bfc484d8745 Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Fri, 27 Feb 2026 07:43:14 -0800 Subject: [PATCH] [vm/shared] Allow const maps in deeply immutable classes. This relies on runtime check of Map-typed variable initialization. TEST=kernel_binary_flowgraph_test Change-Id: Ia9be2644208883739f5896a223dcbe2b59c98114 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482021 Reviewed-by: Slava Egorov --- .../transformations/deeply_immutable.dart | 19 +++- runtime/lib/growable_array.cc | 24 +++++ .../dart/deeply_immutable_const_map_test.dart | 34 ++++++ runtime/vm/bootstrap_natives.cc | 5 + runtime/vm/bootstrap_natives.h | 1 + runtime/vm/class_id.h | 3 +- .../frontend/kernel_binary_flowgraph_test.cc | 102 ++++++++++++++++++ runtime/vm/object.cc | 15 ++- .../_internal/vm_shared/lib/compact_hash.dart | 9 +- 9 files changed, 206 insertions(+), 6 deletions(-) create mode 100644 runtime/tests/vm/dart/deeply_immutable_const_map_test.dart diff --git a/pkg/vm/lib/modular/transformations/deeply_immutable.dart b/pkg/vm/lib/modular/transformations/deeply_immutable.dart index 5b4ced672e7..983f0143096 100644 --- a/pkg/vm/lib/modular/transformations/deeply_immutable.dart +++ b/pkg/vm/lib/modular/transformations/deeply_immutable.dart @@ -60,6 +60,7 @@ class DeeplyImmutableValidator { // Can be null if ffi library is not available. final Class? structClass; final Class? unionClass; + final Class mapClass; DeeplyImmutableValidator( LibraryIndex index, @@ -72,7 +73,8 @@ class DeeplyImmutableValidator { 'NativeFieldWrapperClass1', ), structClass = index.tryGetClass('dart:ffi', 'Struct'), - unionClass = index.tryGetClass('dart:ffi', 'Union'); + unionClass = index.tryGetClass('dart:ffi', 'Union'), + mapClass = coreTypes.mapClass; void visitLibrary(Library library) { for (final cls in library.classes) { @@ -111,6 +113,11 @@ class DeeplyImmutableValidator { return node != null; } + bool _isConstMap(Class node) { + return node.name == "_ConstMap" && + node.enclosingLibrary.name == "dart._compact_hash"; + } + void _validateDeeplyImmutable(Class node) { if (!_isDeeplyImmutableClass(node)) { // If class is not marked deeply immutable, check that none of the super @@ -143,7 +150,11 @@ class DeeplyImmutableValidator { superClass != coreTypes.objectClass && node != structClass && node != unionClass && - !_isOrExtendsNativeFieldWrapper1Class(superClass)) { + !_isOrExtendsNativeFieldWrapper1Class(superClass) && + // ConstMap extends mutable class, but has all mutating functions + // disabled. Further, during construction the map is verified to have + // only deeply immutable values. + !_isConstMap(node)) { if (!_isDeeplyImmutableClass(superClass)) { diagnosticReporter.report( diag.ffiDeeplyImmutableSupertypeMustBeDeeplyImmutable, @@ -207,6 +218,10 @@ class DeeplyImmutableValidator { } if (dartType is InterfaceType) { final classNode = dartType.classNode; + if (classNode == mapClass) { + // Relies on dynamic check of whether map is actually const map. + return _CheckResult(isImmutable: true, requiresRuntimeCheck: true); + } return _CheckResult( isImmutable: _isDeeplyImmutableClass(classNode), requiresRuntimeCheck: false, diff --git a/runtime/lib/growable_array.cc b/runtime/lib/growable_array.cc index 531fc071bc0..63b4f58e932 100644 --- a/runtime/lib/growable_array.cc +++ b/runtime/lib/growable_array.cc @@ -82,4 +82,28 @@ DEFINE_NATIVE_ENTRY(Internal_makeFixedListUnmodifiable, 0, 1) { return array.ptr(); } +DEFINE_NATIVE_ENTRY(createConstMapFromMapOfDeeplyImmutables, 0, 1) { + GET_NON_NULL_NATIVE_ARGUMENT(Map, map, arguments->NativeArgAt(0)); + const Instance& instance = + Instance::Handle(zone, ConstMap::NewUninitialized()); + const LinkedHashBase& const_map = LinkedHashBase::Cast(instance); + + const_map.SetTypeArguments( + TypeArguments::Handle(zone, map.GetTypeArguments())); + intptr_t used_data = map.Length() << 1; + const_map.set_used_data(used_data); + const auto& data = Array::Handle(zone, map.data()); + const auto& new_data = Array::Handle(zone, Array::New(used_data)); + const_map.set_data(new_data); + const_map.set_deleted_keys(0); + const_map.ComputeAndSetHashMask(); + Object& object = Object::Handle(zone); + for (intptr_t i = 0; i < used_data; i++) { + object = data.At(i); + object.EnsureDeeplyImmutable(zone); + new_data.SetAt(i, object); + } + return instance.ptr(); +} + } // namespace dart diff --git a/runtime/tests/vm/dart/deeply_immutable_const_map_test.dart b/runtime/tests/vm/dart/deeply_immutable_const_map_test.dart new file mode 100644 index 00000000000..d32ad7dbd04 --- /dev/null +++ b/runtime/tests/vm/dart/deeply_immutable_const_map_test.dart @@ -0,0 +1,34 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import "dart:_compact_hash" show createConstMapFromMapOfDeeplyImmutables; + +import "package:expect/expect.dart"; + +@pragma('vm:deeply-immutable') +final class Foo { + final Map bar; + + Foo(this.bar); +} + +main() { + final foo = Foo(const {"abc": "def"}); + Expect.equals(1, foo.bar.length); + + Expect.throws(() => Foo({"abc": "def"}), (e) => e is ArgumentError); + + final foo1 = Foo(createConstMapFromMapOfDeeplyImmutables({"abc": "def"})); + Expect.equals(1, foo1.bar.length); + + Expect.throws( + () => Foo({"ghi": foo, "jkl": foo1}), + (e) => e is ArgumentError, + ); + + final foo2 = Foo( + createConstMapFromMapOfDeeplyImmutables({"ghi": foo, "jkl": foo1}), + ); + Expect.equals(2, foo2.bar.length); +} diff --git a/runtime/vm/bootstrap_natives.cc b/runtime/vm/bootstrap_natives.cc index 44b975f2545..da421d01d69 100644 --- a/runtime/vm/bootstrap_natives.cc +++ b/runtime/vm/bootstrap_natives.cc @@ -114,6 +114,11 @@ void Bootstrap::SetupNativeResolver() { library.set_native_entry_resolver(resolver); library.set_native_entry_symbol_resolver(symbol_resolver); + library = Library::CompactHashLibrary(); + ASSERT(!library.IsNull()); + library.set_native_entry_resolver(resolver); + library.set_native_entry_symbol_resolver(symbol_resolver); + library = Library::ConvertLibrary(); ASSERT(!library.IsNull()); library.set_native_entry_resolver(resolver); diff --git a/runtime/vm/bootstrap_natives.h b/runtime/vm/bootstrap_natives.h index 7acc1a06486..7b6ad9a3e3d 100644 --- a/runtime/vm/bootstrap_natives.h +++ b/runtime/vm/bootstrap_natives.h @@ -55,6 +55,7 @@ namespace dart { V(Capability_factory, 1) \ V(Capability_equals, 2) \ V(Capability_get_hashcode, 1) \ + V(createConstMapFromMapOfDeeplyImmutables, 1) \ V(RawReceivePort_factory, 2) \ V(RawReceivePort_get_id, 1) \ V(RawReceivePort_closeInternal, 1) \ diff --git a/runtime/vm/class_id.h b/runtime/vm/class_id.h index 057b00b02fa..65c0d7f0bcb 100644 --- a/runtime/vm/class_id.h +++ b/runtime/vm/class_id.h @@ -591,7 +591,8 @@ inline bool IsDeeplyImmutableCid(intptr_t predefined_cid) { predefined_cid == kNullCid || predefined_cid == kPointerCid || predefined_cid == kTypeCid || predefined_cid == kTypeArgumentsCid || predefined_cid == kTypeParameterCid || - predefined_cid == kRecordTypeCid || predefined_cid == kFunctionTypeCid; + predefined_cid == kRecordTypeCid || + predefined_cid == kFunctionTypeCid || predefined_cid == kConstMapCid; } inline bool IsShallowlyImmutableCid(intptr_t predefined_cid) { diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph_test.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph_test.cc index 9ebdbd21b3a..8d4bc6a6128 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph_test.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph_test.cc @@ -482,4 +482,106 @@ ISOLATE_UNIT_TEST_CASE( })); } +ISOLATE_UNIT_TEST_CASE( + StreamingFlowGraphBuilder_MapDeeplyImmutableTypeCheckPresentAndFailing) { + const char* kScript = R"( + @pragma("vm:deeply-immutable") + final class Foo { + final Map map; + Foo(this.map); + } + + @pragma("vm:entry-point", "call") + test() { + final map = {1: 2, 3: 4}; + bool caught = false; + try { + Foo(map); + } on ArgumentError catch(_) { + caught = true; + } + // plain Map should not pass deep-immutability check at this point. + if (!caught) { + throw Exception(); + } + } + )"; + + const auto& root_library = Library::Handle(LoadTestScript(kScript)); + EXPECT(ClassFinalizer::ProcessPendingClasses()); + const Class& foo = Class::Handle(GetClass(root_library, "Foo")); + const auto& error = foo.EnsureIsFinalized(thread); + const auto& constructor = Function::Handle( + foo.LookupConstructor(String::Handle(String::New("Foo.")))); + + EXPECT(error == Error::null()); + Invoke(root_library, "test"); + + TestPipeline pipeline(constructor, CompilerPass::kJIT); + FlowGraph* flow_graph = pipeline.RunPasses({ + CompilerPass::kComputeSSA, + }); + + auto entry = flow_graph->graph_entry()->normal_entry(); + EXPECT(entry != nullptr); + + ILMatcher cursor(flow_graph, entry); + RELEASE_ASSERT(cursor.TryMatch({ + kMatchAndMoveFunctionEntry, + kMatchAndMoveCheckStackOverflow, + kMoveDebugStepChecks, + kMatchAndMoveGuardFieldClass, + kMoveGlob, + kMatchAndMoveCheckFieldImmutability, + kMatchAndMoveStoreField, + })); +} + +ISOLATE_UNIT_TEST_CASE( + StreamingFlowGraphBuilder_ConstMapDeeplyImmutableTypeCheckPresent) { + const char* kScript = R"( + @pragma("vm:deeply-immutable") + final class Foo { + final Map map; + Foo(this.map); + } + + @pragma("vm:entry-point", "call") + test() { + const map = const {1: 2, 3: 4}; + // const Map should pass deep-immutability check. + Foo(map); + } + )"; + + const auto& root_library = Library::Handle(LoadTestScript(kScript)); + EXPECT(ClassFinalizer::ProcessPendingClasses()); + const Class& foo = Class::Handle(GetClass(root_library, "Foo")); + const auto& error = foo.EnsureIsFinalized(thread); + const auto& constructor = Function::Handle( + foo.LookupConstructor(String::Handle(String::New("Foo.")))); + + EXPECT(error == Error::null()); + Invoke(root_library, "test"); + + TestPipeline pipeline(constructor, CompilerPass::kJIT); + FlowGraph* flow_graph = pipeline.RunPasses({ + CompilerPass::kComputeSSA, + }); + + auto entry = flow_graph->graph_entry()->normal_entry(); + EXPECT(entry != nullptr); + + ILMatcher cursor(flow_graph, entry); + RELEASE_ASSERT(cursor.TryMatch({ + kMatchAndMoveFunctionEntry, + kMatchAndMoveCheckStackOverflow, + kMoveDebugStepChecks, + kMatchAndMoveGuardFieldClass, + kMoveGlob, + kMatchAndMoveCheckFieldImmutability, + kMatchAndMoveStoreField, + })); +} + } // namespace dart diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index f6ef6e24b03..ff525c5ecb7 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -221,10 +221,17 @@ PRECOMPILER_WSR_FIELD_DEFINITION(Function, FunctionType, signature) #undef PRECOMPILER_WSR_FIELD_DEFINITION +#if defined(_MSC_VER) +#define TRACE_TYPE_CHECKS_VERBOSE(format, ...) \ + if (FLAG_trace_type_checks_verbose) { \ + OS::PrintErr(format, __VA_ARGS__); \ + } +#else #define TRACE_TYPE_CHECKS_VERBOSE(format, ...) \ if (FLAG_trace_type_checks_verbose) { \ OS::PrintErr(format, ##__VA_ARGS__); \ } +#endif // Takes a vm internal name and makes it suitable for external user. // @@ -1867,6 +1874,10 @@ void Object::EnsureDeeplyImmutable(Zone* zone) const { continue; } + if (current.GetClassId() == ConstMap::kClassId) { + continue; + } + Exceptions::ThrowArgumentError(String::Handle( String::NewFormatted("Only trivially-immutable values are allowed: %s.", current.ToCString()))); @@ -2990,7 +3001,7 @@ ObjectPtr Object::Allocate(intptr_t cls_id, Heap* heap = thread->heap(); uword address = heap->Allocate(thread, size, space); - if (address == 0) [[unlikely]] { + if (UNLIKELY(address == 0)) { // SuspendLongJumpScope during Dart entry ensures that if a longjmp base is // available, it is the innermost error handler, so check for a longjmp base // before checking for an exit frame. @@ -3012,7 +3023,7 @@ ObjectPtr Object::Allocate(intptr_t cls_id, ptr_field_end_offset); raw_obj = static_cast(address + kHeapObjectTag); ASSERT(cls_id == UntaggedObject::ClassIdTag::decode(raw_obj->untag()->tags_)); - if (raw_obj->IsOldObject() && thread->is_marking()) [[unlikely]] { + if (raw_obj->IsOldObject() && UNLIKELY(thread->is_marking())) { // Black allocation. Prevents a data race between the mutator and // concurrent marker on ARM and ARM64 (the marker may observe a // publishing store of this object before the stores that initialize its diff --git a/sdk/lib/_internal/vm_shared/lib/compact_hash.dart b/sdk/lib/_internal/vm_shared/lib/compact_hash.dart index 189d1d4b9de..9986f76a992 100644 --- a/sdk/lib/_internal/vm_shared/lib/compact_hash.dart +++ b/sdk/lib/_internal/vm_shared/lib/compact_hash.dart @@ -407,8 +407,9 @@ base class _Map extends _HashVMBase // This is essentially the same class as _Map, but it does // not permit any modification of map entries from Dart code. We use // this class for maps constructed from Dart constant maps. +@pragma("vm:deeply-immutable") @pragma("vm:entry-point") -base class _ConstMap extends _HashVMImmutableBase +final class _ConstMap extends _HashVMImmutableBase with MapMixin, _HashBase, @@ -422,6 +423,12 @@ base class _ConstMap extends _HashVMImmutableBase } } +@pragma("vm:entry-point") +@pragma("vm:external-name", "createConstMapFromMapOfDeeplyImmutables") +external _ConstMap createConstMapFromMapOfDeeplyImmutables( + Map map, +); + mixin _ImmutableLinkedHashMapMixin on _LinkedHashMapMixin, _HashAbstractImmutableBase { bool containsKey(Object? key) {