Skip to content

Commit b32e5e5

Browse files
aamCommit Queue
authored andcommitted
[vm/shared] Throw AccessError when isolategroup mutator accesses static fields.
Sample snapshot size comparison before/after: === dart2js_aot.dart.snapshot before: 19946368 after: 19998800 (with flag turned on) delta: 52432 0.26% === Bug: #54530 Bug: #56841 CoreLibraryReviewExempt: internal library change only Change-Id: I34b1945c040bbad22cb3ec6fdb6e6776df31a82f TEST=run_isolate_group_run_test.dart Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422360 Reviewed-by: Alexander Markov <[email protected]> Commit-Queue: Alexander Aprelev <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
1 parent 200597c commit b32e5e5

37 files changed

+3246
-2792
lines changed

runtime/vm/compiler/backend/flow_graph_compiler.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3273,6 +3273,11 @@ void LateInitializationErrorSlowPath::EmitSharedStubCall(
32733273
#endif
32743274
}
32753275

3276+
void FieldAccessErrorSlowPath::PushArgumentsForRuntimeCall(
3277+
FlowGraphCompiler* compiler) {
3278+
__ PushObject(Field::ZoneHandle(OriginalField()));
3279+
}
3280+
32763281
void FlowGraphCompiler::EmitNativeMove(
32773282
const compiler::ffi::NativeLocation& destination,
32783283
const compiler::ffi::NativeLocation& source,

runtime/vm/compiler/backend/flow_graph_compiler.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,28 @@ class LateInitializationErrorSlowPath : public ThrowErrorSlowPathCode {
334334
}
335335
};
336336

337+
class FieldAccessErrorSlowPath : public ThrowErrorSlowPathCode {
338+
public:
339+
explicit FieldAccessErrorSlowPath(Instruction* instruction)
340+
: ThrowErrorSlowPathCode(
341+
instruction,
342+
kStaticFieldAccessedWithoutIsolateErrorRuntimeEntry) {
343+
ASSERT(instruction->IsStoreStaticField());
344+
}
345+
virtual const char* name() { return "field access error"; }
346+
347+
virtual intptr_t GetNumberOfArgumentsForRuntimeCall() {
348+
return 1; // field
349+
}
350+
351+
virtual void PushArgumentsForRuntimeCall(FlowGraphCompiler* compiler);
352+
353+
private:
354+
FieldPtr OriginalField() const {
355+
return instruction()->AsStoreStaticField()->field().Original();
356+
}
357+
};
358+
337359
class FlowGraphCompiler : public ValueObject {
338360
private:
339361
class BlockInfo : public ZoneAllocated {

runtime/vm/compiler/backend/il.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4553,8 +4553,8 @@ void LoadStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
45534553
__ BranchIf(EQUAL, slow_path->entry_label());
45544554
return;
45554555
}
4556-
ASSERT(field().has_initializer());
4557-
ASSERT(field().is_late());
4556+
ASSERT((FLAG_experimental_shared_data && !field().is_shared()) ||
4557+
(field().has_initializer() && field().is_late()));
45584558
auto object_store = compiler->isolate_group()->object_store();
45594559
const Field& original_field = Field::ZoneHandle(field().Original());
45604560

@@ -4568,7 +4568,9 @@ void LoadStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
45684568
} else {
45694569
// The stubs below call the initializer function directly, so make sure
45704570
// one is created.
4571-
original_field.EnsureInitializerFunction();
4571+
if (original_field.has_nontrivial_initializer()) {
4572+
original_field.EnsureInitializerFunction();
4573+
}
45724574
stub =
45734575
field().is_shared()
45744576
? (field().is_final()

runtime/vm/compiler/backend/il.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6710,12 +6710,13 @@ class LoadStaticFieldInstr : public TemplateLoadField<0> {
67106710
DISALLOW_COPY_AND_ASSIGN(LoadStaticFieldInstr);
67116711
};
67126712

6713-
class StoreStaticFieldInstr : public TemplateDefinition<1, NoThrow> {
6713+
class StoreStaticFieldInstr : public TemplateDefinition<1, Throws> {
67146714
public:
67156715
StoreStaticFieldInstr(const Field& field,
67166716
Value* value,
6717-
const InstructionSource& source)
6718-
: TemplateDefinition(source),
6717+
const InstructionSource& source,
6718+
intptr_t deopt_id)
6719+
: TemplateDefinition(source, deopt_id),
67196720
field_(field),
67206721
token_pos_(source.token_pos) {
67216722
DEBUG_ASSERT(field.IsNotTemporaryScopedHandle());
@@ -6731,6 +6732,9 @@ class StoreStaticFieldInstr : public TemplateDefinition<1, NoThrow> {
67316732
Value* value() const { return inputs_[kValuePos]; }
67326733

67336734
virtual bool ComputeCanDeoptimize() const { return false; }
6735+
virtual bool ComputeCanDeoptimizeAfterCall() const {
6736+
return FLAG_experimental_shared_data;
6737+
}
67346738

67356739
// Currently CSE/LICM don't operate on any instructions that can be affected
67366740
// by stores/loads. LoadOptimizer handles loads separately. Hence stores
@@ -6739,6 +6743,9 @@ class StoreStaticFieldInstr : public TemplateDefinition<1, NoThrow> {
67396743

67406744
virtual bool MayHaveVisibleEffect() const { return true; }
67416745

6746+
virtual bool CanTriggerGC() const { return FLAG_experimental_shared_data; }
6747+
virtual bool MayThrow() const { return FLAG_experimental_shared_data; }
6748+
67426749
virtual TokenPosition token_pos() const { return token_pos_; }
67436750

67446751
PRINT_OPERANDS_TO_SUPPORT

runtime/vm/compiler/backend/il_arm.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2901,8 +2901,12 @@ 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();
29042906
LocationSummary* locs = new (zone)
2905-
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
2907+
LocationSummary(zone, kNumInputs, kNumTemps,
2908+
can_call_to_throw ? LocationSummary::kCallOnSlowPath
2909+
: LocationSummary::kNoCall);
29062910
locs->set_in(0, Location::RequiresRegister());
29072911
locs->set_temp(0, Location::RequiresRegister());
29082912
return locs;
@@ -2914,6 +2918,14 @@ void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
29142918

29152919
compiler->used_static_fields().Add(&field());
29162920

2921+
if (FLAG_experimental_shared_data && !field().is_shared()) {
2922+
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2923+
compiler->AddSlowPathCode(slow_path);
2924+
2925+
__ LoadIsolate(temp);
2926+
__ BranchIfZero(temp, slow_path->entry_label());
2927+
}
2928+
29172929
__ LoadFromOffset(
29182930
temp, THR,
29192931
field().is_shared()

runtime/vm/compiler/backend/il_arm64.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2465,8 +2465,12 @@ LocationSummary* StoreStaticFieldInstr::MakeLocationSummary(Zone* zone,
24652465
bool opt) const {
24662466
const intptr_t kNumInputs = 1;
24672467
const intptr_t kNumTemps = 1;
2468+
const bool can_call_to_throw =
2469+
FLAG_experimental_shared_data && !field().is_shared();
24682470
LocationSummary* locs = new (zone)
2469-
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
2471+
LocationSummary(zone, kNumInputs, kNumTemps,
2472+
can_call_to_throw ? LocationSummary::kCallOnSlowPath
2473+
: LocationSummary::kNoCall);
24702474
locs->set_in(0, Location::RequiresRegister());
24712475
locs->set_temp(0, Location::RequiresRegister());
24722476
return locs;
@@ -2478,6 +2482,14 @@ void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
24782482

24792483
compiler->used_static_fields().Add(&field());
24802484

2485+
if (FLAG_experimental_shared_data && !field().is_shared()) {
2486+
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2487+
compiler->AddSlowPathCode(slow_path);
2488+
2489+
__ LoadIsolate(temp);
2490+
__ BranchIfZero(temp, slow_path->entry_label());
2491+
}
2492+
24812493
__ LoadFromOffset(
24822494
temp, THR,
24832495
field().is_shared()

runtime/vm/compiler/backend/il_ia32.cc

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,20 +2104,38 @@ void GuardFieldLengthInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
21042104

21052105
LocationSummary* StoreStaticFieldInstr::MakeLocationSummary(Zone* zone,
21062106
bool opt) const {
2107-
LocationSummary* locs =
2108-
new (zone) LocationSummary(zone, 1, 1, LocationSummary::kNoCall);
2107+
const bool can_call_to_throw =
2108+
FLAG_experimental_shared_data && !field().is_shared();
2109+
LocationSummary* locs = new (zone)
2110+
LocationSummary(zone, 1, 1,
2111+
can_call_to_throw ? LocationSummary::kCallOnSlowPath
2112+
: LocationSummary::kNoCall);
21092113
locs->set_in(0, value()->NeedsWriteBarrier() ? Location::WritableRegister()
21102114
: Location::RequiresRegister());
21112115
locs->set_temp(0, Location::RequiresRegister());
21122116
return locs;
21132117
}
21142118

21152119
void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
2116-
Register value = locs()->in(0).reg();
2120+
Register in = locs()->in(0).reg();
21172121
Register temp = locs()->temp(0).reg();
21182122

21192123
compiler->used_static_fields().Add(&field());
21202124

2125+
if (FLAG_experimental_shared_data && !field().is_shared()) {
2126+
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2127+
if (value()->NeedsWriteBarrier()) {
2128+
// Value input is a writable register and should be manually preserved
2129+
// across allocation slow-path. Add it to live_registers set which
2130+
// determines which registers to preserve.
2131+
locs()->live_registers()->Add(Location::RegisterLocation(in));
2132+
}
2133+
compiler->AddSlowPathCode(slow_path);
2134+
2135+
__ LoadIsolate(temp);
2136+
__ BranchIfZero(temp, slow_path->entry_label());
2137+
}
2138+
21212139
__ movl(temp,
21222140
compiler::Address(
21232141
THR,
@@ -2127,7 +2145,7 @@ void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
21272145
// Note: static fields ids won't be changed by hot-reload.
21282146
__ movl(
21292147
compiler::Address(temp, compiler::target::FieldTable::OffsetOf(field())),
2130-
value);
2148+
in);
21312149
}
21322150

21332151
LocationSummary* InstanceOfInstr::MakeLocationSummary(Zone* zone,

runtime/vm/compiler/backend/il_riscv.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2664,8 +2664,12 @@ LocationSummary* StoreStaticFieldInstr::MakeLocationSummary(Zone* zone,
26642664
bool opt) const {
26652665
const intptr_t kNumInputs = 1;
26662666
const intptr_t kNumTemps = 0;
2667+
const bool can_call_to_throw =
2668+
FLAG_experimental_shared_data && !field().is_shared();
26672669
LocationSummary* locs = new (zone)
2668-
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
2670+
LocationSummary(zone, kNumInputs, kNumTemps,
2671+
can_call_to_throw ? LocationSummary::kCallOnSlowPath
2672+
: LocationSummary::kNoCall);
26692673
locs->set_in(0, Location::RequiresRegister());
26702674
return locs;
26712675
}
@@ -2675,6 +2679,14 @@ void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
26752679

26762680
compiler->used_static_fields().Add(&field());
26772681

2682+
if (FLAG_experimental_shared_data && !field().is_shared()) {
2683+
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2684+
compiler->AddSlowPathCode(slow_path);
2685+
2686+
__ LoadIsolate(TMP);
2687+
__ BranchIfZero(TMP, slow_path->entry_label());
2688+
}
2689+
26782690
__ LoadFromOffset(
26792691
TMP, THR,
26802692
field().is_shared()

runtime/vm/compiler/backend/il_x64.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2442,8 +2442,12 @@ LocationSummary* StoreStaticFieldInstr::MakeLocationSummary(Zone* zone,
24422442
bool opt) const {
24432443
const intptr_t kNumInputs = 1;
24442444
const intptr_t kNumTemps = 1;
2445+
const bool can_call_to_throw =
2446+
FLAG_experimental_shared_data && !field().is_shared();
24452447
LocationSummary* locs = new (zone)
2446-
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
2448+
LocationSummary(zone, kNumInputs, kNumTemps,
2449+
can_call_to_throw ? LocationSummary::kCallOnSlowPath
2450+
: LocationSummary::kNoCall);
24472451
locs->set_in(0, Location::RegisterLocation(kWriteBarrierValueReg));
24482452
locs->set_temp(0, Location::RequiresRegister());
24492453
return locs;
@@ -2455,6 +2459,14 @@ void StoreStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
24552459

24562460
compiler->used_static_fields().Add(&field());
24572461

2462+
if (FLAG_experimental_shared_data && !field().is_shared()) {
2463+
ThrowErrorSlowPathCode* slow_path = new FieldAccessErrorSlowPath(this);
2464+
compiler->AddSlowPathCode(slow_path);
2465+
2466+
__ LoadIsolate(temp);
2467+
__ BranchIfZero(temp, slow_path->entry_label());
2468+
}
2469+
24582470
__ movq(temp,
24592471
compiler::Address(
24602472
THR,

runtime/vm/compiler/frontend/base_flow_graph_builder.cc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -634,9 +634,14 @@ Fragment BaseFlowGraphBuilder::StoreFieldGuarded(
634634

635635
Fragment BaseFlowGraphBuilder::LoadStaticField(const Field& field,
636636
bool calls_initializer) {
637+
// "Inititalizer" code is in charge of checking non-shared field access from
638+
// stateless isolates(which exist when experimental_shared_data is enabled).
639+
const bool do_call_initializer =
640+
calls_initializer ||
641+
(dart::FLAG_experimental_shared_data && !field.is_shared());
637642
LoadStaticFieldInstr* load = new (Z) LoadStaticFieldInstr(
638-
field, InstructionSource(), calls_initializer,
639-
calls_initializer ? GetNextDeoptId() : DeoptId::kNone);
643+
field, InstructionSource(), do_call_initializer,
644+
do_call_initializer ? GetNextDeoptId() : DeoptId::kNone);
640645
Push(load);
641646
return Fragment(load);
642647
}
@@ -674,7 +679,8 @@ Fragment BaseFlowGraphBuilder::Utf8Scan() {
674679
Fragment BaseFlowGraphBuilder::StoreStaticField(TokenPosition position,
675680
const Field& field) {
676681
return Fragment(new (Z) StoreStaticFieldInstr(MayCloneField(Z, field), Pop(),
677-
InstructionSource(position)));
682+
InstructionSource(position),
683+
GetNextDeoptId()));
678684
}
679685

680686
Fragment BaseFlowGraphBuilder::StoreIndexed(classid_t class_id) {

0 commit comments

Comments
 (0)