[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 <vegorov@google.com>
This commit is contained in:
committed by
Commit Queue
parent
3875050f81
commit
cdf7bf9c78
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<String, Object> 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);
|
||||
}
|
||||
@@ -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);
|
||||
|
||||
@@ -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) \
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 = <int, int>{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 <int, int>{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
|
||||
|
||||
+13
-2
@@ -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<ObjectPtr>(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
|
||||
|
||||
@@ -407,8 +407,9 @@ base class _Map<K, V> 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<K, V> extends _HashVMImmutableBase
|
||||
final class _ConstMap<K, V> extends _HashVMImmutableBase
|
||||
with
|
||||
MapMixin<K, V>,
|
||||
_HashBase,
|
||||
@@ -422,6 +423,12 @@ base class _ConstMap<K, V> extends _HashVMImmutableBase
|
||||
}
|
||||
}
|
||||
|
||||
@pragma("vm:entry-point")
|
||||
@pragma("vm:external-name", "createConstMapFromMapOfDeeplyImmutables")
|
||||
external _ConstMap<K, V> createConstMapFromMapOfDeeplyImmutables<K, V>(
|
||||
Map<K, V> map,
|
||||
);
|
||||
|
||||
mixin _ImmutableLinkedHashMapMixin<K, V>
|
||||
on _LinkedHashMapMixin<K, V>, _HashAbstractImmutableBase {
|
||||
bool containsKey(Object? key) {
|
||||
|
||||
Reference in New Issue
Block a user