[vm] Ensure we copy shallow (but not deeply) immutable lists

The object graph copy code assumed kImmutableCid is used only for
list constants but it turns out our List.immutable([]) implementation
isn't using a wrapper-view, but instead a fixed-length immutable list as
with constants:

  factory List.unmodifiable(Iterable elements) {
    final result = new List<E>.from(elements, growable: false);
    return makeFixedListUnmodifiable(result);
  }

The list constants should already be handled via canonical bit.

Issue https://github.com/dart-lang/sdk/issues/51302

TEST=vm/dart{,_2}/isolates/regress_51302_test

Change-Id: I5924084ff004dc52bf675897455453d728484d1d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/281421
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This commit is contained in:
Martin Kustermann 2023-02-08 08:58:40 +00:00 committed by Commit Queue
parent 035b0c23a9
commit 63b5f5d4b1
3 changed files with 66 additions and 5 deletions

View file

@ -0,0 +1,27 @@
// Copyright (c) 2023, 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:isolate';
import 'package:expect/expect.dart';
main() async {
final original = List<C>.unmodifiable([C()]);
final copy = await sendReceive(original);
Expect.notIdentical(original, copy);
original[0].field = 1;
Expect.equals(1, original[0].field);
Expect.equals(0, copy[0].field);
}
Future<T> sendReceive<T>(T arg) async {
final rp = ReceivePort();
rp.sendPort.send(arg);
return (await rp.first) as T;
}
class C {
int field = 0;
}

View file

@ -0,0 +1,29 @@
// Copyright (c) 2023, 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.
// @dart=2.9
import 'dart:isolate';
import 'package:expect/expect.dart';
main() async {
final original = List<C>.unmodifiable([C()]);
final copy = await sendReceive(original);
Expect.notIdentical(original, copy);
original[0].field = 1;
Expect.equals(1, original[0].field);
Expect.equals(0, copy[0].field);
}
Future<T> sendReceive<T>(T arg) async {
final rp = ReceivePort();
rp.sendPort.send(arg);
return (await rp.first) as T;
}
class C {
int field = 0;
}

View file

@ -156,7 +156,6 @@ static bool CanShareObject(ObjectPtr obj, uword tags) {
if (cid == kExternalOneByteStringCid) return true;
if (cid == kExternalTwoByteStringCid) return true;
if (cid == kMintCid) return true;
if (cid == kImmutableArrayCid) return true;
if (cid == kNeverCid) return true;
if (cid == kSentinelCid) return true;
if (cid == kStackTraceCid) return true;
@ -209,7 +208,6 @@ static bool MightNeedReHashing(ObjectPtr object) {
// These are shared and use identity hash codes. If they are used as a key in
// a map or a value in a set, they will already have the identity hash code
// set.
if (cid == kImmutableArrayCid) return false;
if (cid == kRegExpCid) return false;
if (cid == kInt32x4Cid) return false;
@ -270,11 +268,10 @@ DART_FORCE_INLINE
void UpdateLengthField(intptr_t cid, ObjectPtr from, ObjectPtr to) {
// We share these objects - never copy them.
ASSERT(!IsStringClassId(cid));
ASSERT(cid != kImmutableArrayCid);
// We update any in-heap variable sized object with the length to keep the
// length and the size in the object header in-sync for the GC.
if (cid == kArrayCid) {
if (cid == kArrayCid || cid == kImmutableArrayCid) {
static_cast<UntaggedArray*>(to.untag())->length_ =
static_cast<UntaggedArray*>(from.untag())->length_;
} else if (cid == kContextCid) {
@ -1203,7 +1200,8 @@ class SlowObjectCopyBase : public ObjectCopyBase {
UpdateLengthField(cid, from.ptr(), to_.ptr());
slow_forward_map_.Insert(from, to_, size);
ObjectPtr to = to_.ptr();
if (cid == kArrayCid && !Heap::IsAllocatableInNewSpace(size)) {
if ((cid == kArrayCid || cid == kImmutableArrayCid) &&
!Heap::IsAllocatableInNewSpace(size)) {
to.untag()->SetCardRememberedBitUnsynchronized();
}
if (IsExternalTypedDataClassId(cid)) {
@ -1318,6 +1316,13 @@ class ObjectCopy : public Base {
COPY_TO(Set)
#undef COPY_TO
case ImmutableArray::kClassId: {
typename Types::Array casted_from = Types::CastArray(from);
typename Types::Array casted_to = Types::CastArray(to);
CopyArray(casted_from, casted_to);
return;
}
#define COPY_TO(clazz) case kTypedData##clazz##Cid:
CLASS_LIST_TYPED_DATA(COPY_TO) {