Skip to content

Commit ee2e81e

Browse files
aamCommit Queue
authored andcommitted
[vm/shared] Enforce trivially-shareable constraint.
Ensure only trivially-shareable and typed data values can be placed in shared static fields. BUG=#60825 TEST=ci Change-Id: Ief75cbd3fbb5fa0ebe94aab9a32e1f1fa090bcc5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/438940 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Aprelev <[email protected]>
1 parent 14de152 commit ee2e81e

File tree

16 files changed

+1350
-1132
lines changed

16 files changed

+1350
-1132
lines changed

pkg/vm/lib/modular/target/vm.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ class VmTarget extends Target {
174174
ChangedStructureNotifier? changedStructureNotifier,
175175
}) {
176176
deeply_immutable.validateLibraries(
177+
component,
177178
libraries,
178179
coreTypes,
179180
diagnosticReporter,

pkg/vm/lib/modular/transformations/deeply_immutable.dart

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,23 @@ import 'package:front_end/src/codes/cfe_codes.dart'
1313
messageFfiDeeplyImmutableSupertypeMustBeDeeplyImmutable;
1414
import 'package:kernel/ast.dart';
1515
import 'package:kernel/core_types.dart';
16+
import 'package:kernel/library_index.dart' show LibraryIndex;
1617
import 'package:kernel/target/targets.dart' show DiagnosticReporter;
1718

1819
void validateLibraries(
20+
Component component,
1921
List<Library> libraries,
2022
CoreTypes coreTypes,
2123
DiagnosticReporter diagnosticReporter,
2224
) {
23-
final validator = DeeplyImmutableValidator(coreTypes, diagnosticReporter);
25+
final LibraryIndex index = LibraryIndex(component, const [
26+
'dart:nativewrappers',
27+
]);
28+
final validator = DeeplyImmutableValidator(
29+
index,
30+
coreTypes,
31+
diagnosticReporter,
32+
);
2433
for (final library in libraries) {
2534
validator.visitLibrary(library);
2635
}
@@ -34,10 +43,19 @@ class DeeplyImmutableValidator {
3443
final DiagnosticReporter diagnosticReporter;
3544
final Class pragmaClass;
3645
final Field pragmaName;
46+
// Can be null if nativewrappers library is not available.
47+
final Class? nativeFieldWrapperClass1Class;
3748

38-
DeeplyImmutableValidator(this.coreTypes, this.diagnosticReporter)
39-
: pragmaClass = coreTypes.pragmaClass,
40-
pragmaName = coreTypes.pragmaName;
49+
DeeplyImmutableValidator(
50+
LibraryIndex index,
51+
this.coreTypes,
52+
this.diagnosticReporter,
53+
) : pragmaClass = coreTypes.pragmaClass,
54+
pragmaName = coreTypes.pragmaName,
55+
nativeFieldWrapperClass1Class = index.tryGetClass(
56+
'dart:nativewrappers',
57+
'NativeFieldWrapperClass1',
58+
);
4159

4260
void visitLibrary(Library library) {
4361
for (final cls in library.classes) {
@@ -49,6 +67,13 @@ class DeeplyImmutableValidator {
4967
_validateDeeplyImmutable(node);
5068
}
5169

70+
bool _isOrExtendsNativeFieldWrapper1Class(Class? node) {
71+
while (node != null && node != nativeFieldWrapperClass1Class) {
72+
node = node.superclass;
73+
}
74+
return node != null;
75+
}
76+
5277
void _validateDeeplyImmutable(Class node) {
5378
if (!_isDeeplyImmutableClass(node)) {
5479
// If class is not marked deeply immutable, check that none of the super
@@ -72,7 +97,9 @@ class DeeplyImmutableValidator {
7297
}
7398

7499
final superClass = node.superclass;
75-
if (superClass != null && superClass != coreTypes.objectClass) {
100+
if (superClass != null &&
101+
superClass != coreTypes.objectClass &&
102+
!_isOrExtendsNativeFieldWrapper1Class(superClass)) {
76103
if (!_isDeeplyImmutableClass(superClass)) {
77104
diagnosticReporter.report(
78105
messageFfiDeeplyImmutableSupertypeMustBeDeeplyImmutable,

runtime/tests/vm/dart/isolates/shared_test_body.dart

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,34 +9,29 @@ import 'dart:concurrent';
99
import 'dart:ffi';
1010
import 'dart:io';
1111
import 'dart:isolate';
12+
import 'dart:typed_data';
1213

1314
import 'package:expect/async_helper.dart';
1415
import 'package:expect/expect.dart';
1516
import 'package:ffi/ffi.dart';
1617

17-
class WorkItem {
18-
int i;
18+
void doWork(int i, SendPort results) {
1919
int result = 0;
20-
21-
WorkItem(this.i);
22-
23-
doWork(SendPort results) {
24-
// Calculate fibonacci number i.
25-
if (i < 3) {
26-
result = 1;
27-
} else {
28-
int pp = 1;
29-
int p = 1;
30-
int j = 3;
31-
while (j <= i) {
32-
result = pp + p;
33-
pp = p;
34-
p = result;
35-
j++;
36-
}
20+
// Calculate fibonacci number i.
21+
if (i < 3) {
22+
result = 1;
23+
} else {
24+
int pp = 1;
25+
int p = 1;
26+
int j = 3;
27+
while (j <= i) {
28+
result = pp + p;
29+
pp = p;
30+
p = result;
31+
j++;
3732
}
38-
results.send(<int>[i, result]);
3933
}
34+
results.send(<int>[i, result]);
4035
}
4136

4237
class SharedState {
@@ -48,7 +43,7 @@ int totalWorkItems = 10000;
4843
int numberOfWorkers = 8;
4944

5045
@pragma('vm:shared')
51-
late List<WorkItem> workItems;
46+
late Uint16List workItems;
5247
@pragma('vm:shared')
5348
late int lastProcessed;
5449
@pragma('vm:shared')
@@ -61,7 +56,9 @@ late var results = <int, int>{};
6156
void init() {
6257
SharedState.totalProcessed = 0;
6358
lastProcessed = 0;
64-
workItems = List<WorkItem>.generate(totalWorkItems, (i) => WorkItem(i + 1));
59+
workItems = Uint16List.fromList(
60+
List<int>.generate(totalWorkItems, (i) => i + 1),
61+
);
6562
mutex = Mutex();
6663
}
6764

@@ -92,7 +89,7 @@ void main(List<String> args) async {
9289
if (mine >= workItems.length) {
9390
break;
9491
}
95-
workItems[mine].doWork(sendPort);
92+
doWork(workItems[mine], sendPort);
9693
countProcessed++;
9794
mutex.runLocked(() => SharedState.totalProcessed++);
9895
await Future.delayed(Duration(seconds: 0));

runtime/vm/compiler/backend/flow_graph_compiler.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3284,6 +3284,17 @@ void FieldAccessErrorSlowPath::PushArgumentsForRuntimeCall(
32843284
__ PushObject(Field::ZoneHandle(OriginalField()));
32853285
}
32863286

3287+
void ThrowIfValueCantBeSharedSlowPath::PushArgumentsForRuntimeCall(
3288+
FlowGraphCompiler* compiler) {
3289+
__ PushObject(Field::ZoneHandle(OriginalField()));
3290+
__ PushRegister(value());
3291+
}
3292+
3293+
void ThrowIfValueCantBeSharedSlowPath::EmitNativeCode(
3294+
FlowGraphCompiler* compiler) {
3295+
ThrowErrorSlowPathCode::EmitNativeCode(compiler);
3296+
}
3297+
32873298
void FlowGraphCompiler::EmitNativeMove(
32883299
const compiler::ffi::NativeLocation& destination,
32893300
const compiler::ffi::NativeLocation& source,

runtime/vm/compiler/backend/flow_graph_compiler.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,35 @@ class FieldAccessErrorSlowPath : public ThrowErrorSlowPathCode {
356356
}
357357
};
358358

359+
class ThrowIfValueCantBeSharedSlowPath : public ThrowErrorSlowPathCode {
360+
public:
361+
explicit ThrowIfValueCantBeSharedSlowPath(Instruction* instruction,
362+
Register value)
363+
: ThrowErrorSlowPathCode(instruction,
364+
kThrowIfValueCantBeSharedRuntimeEntry),
365+
value_(value) {
366+
ASSERT(instruction->IsStoreStaticField());
367+
}
368+
virtual const char* name() { return "shared value error"; }
369+
370+
virtual intptr_t GetNumberOfArgumentsForRuntimeCall() {
371+
return 2; // field, value
372+
}
373+
374+
virtual void PushArgumentsForRuntimeCall(FlowGraphCompiler* compiler);
375+
376+
virtual void EmitNativeCode(FlowGraphCompiler* compiler);
377+
378+
private:
379+
Register value_;
380+
381+
FieldPtr OriginalField() const {
382+
return instruction()->AsStoreStaticField()->field().Original();
383+
}
384+
385+
Register value() const { return value_; }
386+
};
387+
359388
class FlowGraphCompiler : public ValueObject {
360389
private:
361390
class BlockInfo : public ZoneAllocated {

runtime/vm/compiler/backend/il_arm.cc

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2901,8 +2901,7 @@ LocationSummary* StoreStaticFieldInstr::MakeLocationSummary(Zone* zone,
29012901
bool opt) const {
29022902
const intptr_t kNumInputs = 1;
29032903
const intptr_t kNumTemps = 1;
2904-
const bool can_call_to_throw =
2905-
FLAG_experimental_shared_data && !field().is_shared();
2904+
const bool can_call_to_throw = FLAG_experimental_shared_data;
29062905
LocationSummary* locs = new (zone)
29072906
LocationSummary(zone, kNumInputs, kNumTemps,
29082907
can_call_to_throw ? LocationSummary::kCallOnSlowPath
@@ -2918,12 +2917,37 @@ void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
29182917

29192918
compiler->used_static_fields().Add(&field());
29202919

2921-
if (FLAG_experimental_shared_data && !field().is_shared()) {
2922-
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2923-
compiler->AddSlowPathCode(slow_path);
2920+
if (FLAG_experimental_shared_data) {
2921+
if (!field().is_shared()) {
2922+
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2923+
compiler->AddSlowPathCode(slow_path);
29242924

2925-
__ LoadIsolate(temp);
2926-
__ BranchIfZero(temp, slow_path->entry_label());
2925+
__ LoadIsolate(temp);
2926+
__ BranchIfZero(temp, slow_path->entry_label());
2927+
} else {
2928+
// TODO(dartbug.com/61078): use field static type information to decide
2929+
// whether the following value check is needed or not.
2930+
auto throw_if_cant_be_shared_slow_path =
2931+
new ThrowIfValueCantBeSharedSlowPath(this, value);
2932+
compiler->AddSlowPathCode(throw_if_cant_be_shared_slow_path);
2933+
2934+
compiler::Label allow_store;
2935+
__ BranchIfSmi(value, &allow_store, compiler::Assembler::kNearJump);
2936+
__ ldrb(temp, compiler::FieldAddress(
2937+
value, compiler::target::Object::tags_offset()));
2938+
__ TestImmediate(temp,
2939+
1 << compiler::target::UntaggedObject::kImmutableBit);
2940+
__ b(&allow_store, NOT_ZERO);
2941+
2942+
// Allow TypedData because they contain non-structural mutable state.
2943+
__ LoadClassId(temp, value);
2944+
__ CompareImmediate(temp, kFirstTypedDataCid);
2945+
__ b(throw_if_cant_be_shared_slow_path->entry_label(), LT);
2946+
__ CompareImmediate(temp, kLastTypedDataCid);
2947+
__ b(throw_if_cant_be_shared_slow_path->entry_label(), GT);
2948+
2949+
__ Bind(&allow_store);
2950+
}
29272951
}
29282952

29292953
__ LoadFromOffset(

runtime/vm/compiler/backend/il_arm64.cc

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2467,8 +2467,7 @@ LocationSummary* StoreStaticFieldInstr::MakeLocationSummary(Zone* zone,
24672467
bool opt) const {
24682468
const intptr_t kNumInputs = 1;
24692469
const intptr_t kNumTemps = 1;
2470-
const bool can_call_to_throw =
2471-
FLAG_experimental_shared_data && !field().is_shared();
2470+
const bool can_call_to_throw = FLAG_experimental_shared_data;
24722471
LocationSummary* locs = new (zone)
24732472
LocationSummary(zone, kNumInputs, kNumTemps,
24742473
can_call_to_throw ? LocationSummary::kCallOnSlowPath
@@ -2484,12 +2483,38 @@ void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
24842483

24852484
compiler->used_static_fields().Add(&field());
24862485

2487-
if (FLAG_experimental_shared_data && !field().is_shared()) {
2488-
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2489-
compiler->AddSlowPathCode(slow_path);
2486+
if (FLAG_experimental_shared_data) {
2487+
if (!field().is_shared()) {
2488+
auto slow_path = new FieldAccessErrorSlowPath(this);
2489+
compiler->AddSlowPathCode(slow_path);
24902490

2491-
__ LoadIsolate(temp);
2492-
__ BranchIfZero(temp, slow_path->entry_label());
2491+
__ LoadIsolate(temp);
2492+
__ BranchIfZero(temp, slow_path->entry_label());
2493+
} else {
2494+
// TODO(dartbug.com/61078): use field static type information to decide
2495+
// whether the following value check is needed or not.
2496+
auto throw_if_cant_be_shared_slow_path =
2497+
new ThrowIfValueCantBeSharedSlowPath(this, value);
2498+
compiler->AddSlowPathCode(throw_if_cant_be_shared_slow_path);
2499+
2500+
compiler::Label allow_store;
2501+
__ BranchIfSmi(value, &allow_store, compiler::Assembler::kNearJump);
2502+
__ ldr(temp,
2503+
compiler::FieldAddress(value,
2504+
compiler::target::Object::tags_offset()),
2505+
compiler::kUnsignedByte);
2506+
__ tbnz(&allow_store, temp,
2507+
compiler::target::UntaggedObject::kImmutableBit);
2508+
2509+
// Allow TypedData because they contain non-structural mutable state.
2510+
__ LoadClassId(temp, value);
2511+
__ CompareImmediate(temp, kFirstTypedDataCid);
2512+
__ b(throw_if_cant_be_shared_slow_path->entry_label(), UNSIGNED_LESS);
2513+
__ CompareImmediate(temp, kLastTypedDataCid);
2514+
__ b(throw_if_cant_be_shared_slow_path->entry_label(), UNSIGNED_GREATER);
2515+
2516+
__ Bind(&allow_store);
2517+
}
24932518
}
24942519

24952520
__ LoadFromOffset(

runtime/vm/compiler/backend/il_ia32.cc

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,8 +2104,7 @@ void GuardFieldLengthInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
21042104

21052105
LocationSummary* StoreStaticFieldInstr::MakeLocationSummary(Zone* zone,
21062106
bool opt) const {
2107-
const bool can_call_to_throw =
2108-
FLAG_experimental_shared_data && !field().is_shared();
2107+
const bool can_call_to_throw = FLAG_experimental_shared_data;
21092108
LocationSummary* locs = new (zone)
21102109
LocationSummary(zone, 1, 1,
21112110
can_call_to_throw ? LocationSummary::kCallOnSlowPath
@@ -2122,18 +2121,44 @@ void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
21222121

21232122
compiler->used_static_fields().Add(&field());
21242123

2125-
if (FLAG_experimental_shared_data && !field().is_shared()) {
2126-
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2124+
if (FLAG_experimental_shared_data) {
21272125
if (value()->NeedsWriteBarrier()) {
21282126
// Value input is a writable register and should be manually preserved
21292127
// across allocation slow-path. Add it to live_registers set which
21302128
// determines which registers to preserve.
21312129
locs()->live_registers()->Add(Location::RegisterLocation(in));
21322130
}
2133-
compiler->AddSlowPathCode(slow_path);
2131+
if (!field().is_shared()) {
2132+
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2133+
compiler->AddSlowPathCode(slow_path);
21342134

2135-
__ LoadIsolate(temp);
2136-
__ BranchIfZero(temp, slow_path->entry_label());
2135+
__ LoadIsolate(temp);
2136+
__ BranchIfZero(temp, slow_path->entry_label());
2137+
} else {
2138+
// TODO(dartbug.com/61078): use field static type information to decide
2139+
// whether the following value check is needed or not.
2140+
auto throw_if_cant_be_shared_slow_path =
2141+
new ThrowIfValueCantBeSharedSlowPath(this, in);
2142+
compiler->AddSlowPathCode(throw_if_cant_be_shared_slow_path);
2143+
2144+
compiler::Label allow_store;
2145+
__ BranchIfSmi(in, &allow_store, compiler::Assembler::kNearJump);
2146+
2147+
__ movl(temp, compiler::FieldAddress(
2148+
in, compiler::target::Object::tags_offset()));
2149+
__ testl(temp, compiler::Immediate(
2150+
1 << compiler::target::UntaggedObject::kImmutableBit));
2151+
__ j(NOT_ZERO, &allow_store);
2152+
2153+
// Allow TypedData because they contain non-structural mutable state.
2154+
__ LoadClassId(temp, in);
2155+
__ CompareImmediate(temp, kFirstTypedDataCid);
2156+
__ BranchIf(LESS, throw_if_cant_be_shared_slow_path->entry_label());
2157+
__ CompareImmediate(temp, kLastTypedDataCid);
2158+
__ BranchIf(GREATER, throw_if_cant_be_shared_slow_path->entry_label());
2159+
2160+
__ Bind(&allow_store);
2161+
}
21372162
}
21382163

21392164
__ movl(temp,

0 commit comments

Comments
 (0)