diff --git a/runtime/vm/compiler/backend/constant_propagator_test.cc b/runtime/vm/compiler/backend/constant_propagator_test.cc index ca99d04067f..e9fd0450f26 100644 --- a/runtime/vm/compiler/backend/constant_propagator_test.cc +++ b/runtime/vm/compiler/backend/constant_propagator_test.cc @@ -307,8 +307,8 @@ void StrictCompareSentinel(Thread* thread, { BlockBuilder builder(H.flow_graph(), b1); auto v_load = builder.AddDefinition(new LoadStaticFieldInstr( - field_x, {}, - /*calls_initializer=*/true, S.GetNextDeoptId())); + field_x, {}, SlowPathOnSentinelValue::kCallInitializer, + S.GetNextDeoptId())); auto v_sentinel = H.flow_graph()->GetConstant(Object::sentinel()); Value* const left_value = non_sentinel_on_left ? new Value(v_load) : new Value(v_sentinel); diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc index 60bd810eb9f..701dd07cb1c 100644 --- a/runtime/vm/compiler/backend/il.cc +++ b/runtime/vm/compiler/backend/il.cc @@ -4504,27 +4504,28 @@ LocationSummary* LoadStaticFieldInstr::MakeLocationSummary(Zone* zone, bool opt) const { const intptr_t kNumInputs = 0; const bool use_shared_stub = UseSharedSlowPathStub(opt); - const intptr_t kNumTemps = calls_initializer() && + const intptr_t kNumTemps = does_throw_access_error_or_call_initializer() && throw_exception_on_initialization() && use_shared_stub ? 1 : 0; LocationSummary* locs = new (zone) LocationSummary( zone, kNumInputs, kNumTemps, - calls_initializer() + does_throw_access_error_or_call_initializer() ? (throw_exception_on_initialization() ? (use_shared_stub ? LocationSummary::kCallOnSharedSlowPath : LocationSummary::kCallOnSlowPath) : LocationSummary::kCall) : LocationSummary::kNoCall); - if (calls_initializer() && throw_exception_on_initialization() && - use_shared_stub) { + if (does_throw_access_error_or_call_initializer() && + throw_exception_on_initialization() && use_shared_stub) { locs->set_temp( 0, Location::RegisterLocation(LateInitializationErrorABI::kFieldReg)); } - locs->set_out(0, calls_initializer() ? Location::RegisterLocation( - InitStaticFieldABI::kResultReg) - : Location::RequiresRegister()); + locs->set_out(0, + does_throw_access_error_or_call_initializer() + ? Location::RegisterLocation(InitStaticFieldABI::kResultReg) + : Location::RequiresRegister()); return locs; } @@ -4543,8 +4544,8 @@ void LoadStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) { __ LoadMemoryValue(result, THR, static_cast(field_table_offset)); __ LoadMemoryValue(result, result, static_cast(field_offset)); - if (calls_initializer()) { - if (throw_exception_on_initialization()) { + if (does_throw_access_error_or_call_initializer()) { + if (calls_initializer() && throw_exception_on_initialization()) { ThrowErrorSlowPathCode* slow_path = new LateInitializationErrorSlowPath(this); compiler->AddSlowPathCode(slow_path); @@ -4563,22 +4564,27 @@ void LoadStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) { __ BranchIf(NOT_EQUAL, &no_call); auto& stub = Code::ZoneHandle(compiler->zone()); - if (field().needs_load_guard()) { - stub = object_store->init_static_field_stub(); - } else { - // The stubs below call the initializer function directly, so make sure - // one is created. - if (original_field.has_nontrivial_initializer()) { - original_field.EnsureInitializerFunction(); + if (calls_initializer()) { + if (field().needs_load_guard()) { + stub = object_store->init_static_field_stub(); + } else { + // The stubs below call the initializer function directly, so make sure + // one is created. + if (original_field.has_nontrivial_initializer()) { + original_field.EnsureInitializerFunction(); + } + stub = field().is_shared() + ? (field().is_final() + ? object_store + ->init_shared_late_final_static_field_stub() + : object_store->init_shared_late_static_field_stub()) + : (field().is_final() + ? object_store->init_late_final_static_field_stub() + : object_store->init_late_static_field_stub()); } - stub = - field().is_shared() - ? (field().is_final() - ? object_store->init_shared_late_final_static_field_stub() - : object_store->init_shared_late_static_field_stub()) - : (field().is_final() - ? object_store->init_late_final_static_field_stub() - : object_store->init_late_static_field_stub()); + } else { + ASSERT(FLAG_experimental_shared_data && !field().is_shared()); + stub = object_store->check_isolate_field_access_stub(); } __ LoadObject(InitStaticFieldABI::kFieldReg, original_field); diff --git a/runtime/vm/compiler/backend/il.h b/runtime/vm/compiler/backend/il.h index 745c4b2fd3c..516c27c0b2b 100644 --- a/runtime/vm/compiler/backend/il.h +++ b/runtime/vm/compiler/backend/il.h @@ -6602,27 +6602,47 @@ class GuardFieldTypeInstr : public GuardFieldInstr { DISALLOW_COPY_AND_ASSIGN(GuardFieldTypeInstr); }; +enum class SlowPathOnSentinelValue { + kDoNothing, + kThrowAccessError, // This is part of shared field implementation. + kCallInitializer, // This will also do the shared field access check. +}; + template class TemplateLoadField : public TemplateDefinition { using Base = TemplateDefinition; public: - TemplateLoadField(const InstructionSource& source, - bool calls_initializer = false, - intptr_t deopt_id = DeoptId::kNone, - const Field* field = nullptr) + TemplateLoadField( + const InstructionSource& source, + SlowPathOnSentinelValue slow_path = SlowPathOnSentinelValue::kDoNothing, + intptr_t deopt_id = DeoptId::kNone, + const Field* field = nullptr) : Base(source, deopt_id), token_pos_(source.token_pos), throw_exception_on_initialization_( field != nullptr && !field->has_initializer() && field->is_late()), - calls_initializer_(calls_initializer) { - ASSERT(!calls_initializer || field != nullptr); - ASSERT(!calls_initializer || (deopt_id != DeoptId::kNone)); + slow_path_(slow_path) { + ASSERT(slow_path == SlowPathOnSentinelValue::kDoNothing || + field != nullptr); + ASSERT(slow_path == SlowPathOnSentinelValue::kDoNothing || + (deopt_id != DeoptId::kNone)); } virtual TokenPosition token_pos() const { return token_pos_; } - bool calls_initializer() const { return calls_initializer_; } - void set_calls_initializer(bool value) { calls_initializer_ = value; } + + bool does_throw_access_error_or_call_initializer() const { + return slow_path_ > SlowPathOnSentinelValue::kDoNothing; + } + bool throws_access_error() const { + return slow_path_ == SlowPathOnSentinelValue::kThrowAccessError; + } + bool calls_initializer() const { + return slow_path_ == SlowPathOnSentinelValue::kCallInitializer; + } + void clear_calls_initializer() { + slow_path_ = SlowPathOnSentinelValue::kDoNothing; + } bool throw_exception_on_initialization() const { return throw_exception_on_initialization_; @@ -6636,14 +6656,16 @@ class TemplateLoadField : public TemplateDefinition { virtual intptr_t DeoptimizationTarget() const { return Base::GetDeoptId(); } virtual bool ComputeCanDeoptimize() const { return false; } virtual bool ComputeCanDeoptimizeAfterCall() const { - return calls_initializer() && !CompilerState::Current().is_aot(); + return does_throw_access_error_or_call_initializer() && + !CompilerState::Current().is_aot(); } virtual intptr_t NumberOfInputsConsumedBeforeCall() const { return Base::InputCount(); } virtual bool HasUnknownSideEffects() const { - return calls_initializer() && !throw_exception_on_initialization(); + return does_throw_access_error_or_call_initializer() && + !throw_exception_on_initialization(); } virtual bool CanCallDart() const { @@ -6653,13 +6675,17 @@ class TemplateLoadField : public TemplateDefinition { // automatically (see Thread::RestoreWriteBarrierInvariant). return false; } - virtual bool CanTriggerGC() const { return calls_initializer(); } - virtual bool MayThrow() const { return calls_initializer(); } + virtual bool CanTriggerGC() const { + return does_throw_access_error_or_call_initializer(); + } + virtual bool MayThrow() const { + return does_throw_access_error_or_call_initializer(); + } #define FIELD_LIST(F) \ F(const TokenPosition, token_pos_) \ F(const bool, throw_exception_on_initialization_) \ - F(bool, calls_initializer_) + F(SlowPathOnSentinelValue, slow_path_) DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(TemplateLoadField, Base, FIELD_LIST) #undef FIELD_LIST @@ -6670,11 +6696,12 @@ class TemplateLoadField : public TemplateDefinition { class LoadStaticFieldInstr : public TemplateLoadField<0> { public: - LoadStaticFieldInstr(const Field& field, - const InstructionSource& source, - bool calls_initializer = false, - intptr_t deopt_id = DeoptId::kNone) - : TemplateLoadField<0>(source, calls_initializer, deopt_id, &field), + LoadStaticFieldInstr( + const Field& field, + const InstructionSource& source, + SlowPathOnSentinelValue slow_path = SlowPathOnSentinelValue::kDoNothing, + intptr_t deopt_id = DeoptId::kNone) + : TemplateLoadField<0>(source, slow_path, deopt_id, &field), field_(field) {} DECLARE_INSTRUCTION(LoadStaticField) @@ -8101,7 +8128,9 @@ class LoadFieldInstr : public TemplateLoadField<1> { bool calls_initializer = false, intptr_t deopt_id = DeoptId::kNone) : TemplateLoadField(source, - calls_initializer, + calls_initializer + ? SlowPathOnSentinelValue::kCallInitializer + : SlowPathOnSentinelValue::kDoNothing, deopt_id, slot.IsDartField() ? &slot.field() : nullptr), slot_(slot), diff --git a/runtime/vm/compiler/backend/il_printer.cc b/runtime/vm/compiler/backend/il_printer.cc index 6a27c2067b1..5ab60a6d4d4 100644 --- a/runtime/vm/compiler/backend/il_printer.cc +++ b/runtime/vm/compiler/backend/il_printer.cc @@ -976,9 +976,15 @@ void IfThenElseInstr::PrintOperandsTo(BaseTextBuffer* f) const { void LoadStaticFieldInstr::PrintOperandsTo(BaseTextBuffer* f) const { f->Printf("%s", String::Handle(field().name()).ToCString()); + if (throws_access_error()) { + f->AddString(", ThrowsAccessError"); + } if (calls_initializer()) { f->AddString(", CallsInitializer"); } + if (throw_exception_on_initialization()) { + f->AddString(", ThrowExceptionOnInitialization"); + } } void StoreStaticFieldInstr::PrintOperandsTo(BaseTextBuffer* f) const { @@ -1033,6 +1039,9 @@ void MaterializeObjectInstr::PrintOperandsTo(BaseTextBuffer* f) const { void LoadFieldInstr::PrintOperandsTo(BaseTextBuffer* f) const { instance()->PrintTo(f); f->Printf(" . %s%s", slot().Name(), IsImmutableLoad() ? " {final}" : ""); + if (throws_access_error()) { + f->AddString(", ThrowsAccessError"); + } if (calls_initializer()) { f->AddString(", CallsInitializer"); } diff --git a/runtime/vm/compiler/backend/redundancy_elimination.cc b/runtime/vm/compiler/backend/redundancy_elimination.cc index 5c80e41f48a..64232b52e3c 100644 --- a/runtime/vm/compiler/backend/redundancy_elimination.cc +++ b/runtime/vm/compiler/backend/redundancy_elimination.cc @@ -1904,9 +1904,9 @@ class LoadOptimizer : public ValueObject { void ClearCallsInitializer(Instruction* instr) { if (auto* load_field = instr->AsLoadField()) { - load_field->set_calls_initializer(false); + load_field->clear_calls_initializer(); } else if (auto* load_static = instr->AsLoadStaticField()) { - load_static->set_calls_initializer(false); + load_static->clear_calls_initializer(); } else { UNREACHABLE(); } diff --git a/runtime/vm/compiler/backend/type_propagator_test.cc b/runtime/vm/compiler/backend/type_propagator_test.cc index 33d27fa2f67..ffae3b50077 100644 --- a/runtime/vm/compiler/backend/type_propagator_test.cc +++ b/runtime/vm/compiler/backend/type_propagator_test.cc @@ -609,8 +609,7 @@ ISOLATE_UNIT_TEST_CASE(TypePropagator_RedefineCanBeSentinelWithCannotBe) { { BlockBuilder builder(H.flow_graph(), b1); v3 = builder.AddDefinition(new LoadStaticFieldInstr( - field_x, {}, - /*calls_initializer=*/false, S.GetNextDeoptId())); + field_x, {}, SlowPathOnSentinelValue::kDoNothing, S.GetNextDeoptId())); auto v5 = builder.AddDefinition(new ConstantInstr(Object::sentinel())); builder.AddBranch(new StrictCompareInstr( {}, Token::kEQ_STRICT, new Value(v3), new Value(v5), diff --git a/runtime/vm/compiler/frontend/base_flow_graph_builder.cc b/runtime/vm/compiler/frontend/base_flow_graph_builder.cc index 7c60cdbd953..359e072a8fc 100644 --- a/runtime/vm/compiler/frontend/base_flow_graph_builder.cc +++ b/runtime/vm/compiler/frontend/base_flow_graph_builder.cc @@ -634,14 +634,17 @@ Fragment BaseFlowGraphBuilder::StoreFieldGuarded( Fragment BaseFlowGraphBuilder::LoadStaticField(const Field& field, bool calls_initializer) { - // "Inititalizer" code is in charge of checking non-shared field access from - // stateless isolates(which exist when experimental_shared_data is enabled). - const bool do_call_initializer = - calls_initializer || + const bool check_access = (dart::FLAG_experimental_shared_data && !field.is_shared()); + const auto slow_path = + calls_initializer + ? SlowPathOnSentinelValue::kCallInitializer + : (check_access ? SlowPathOnSentinelValue::kThrowAccessError + : SlowPathOnSentinelValue::kDoNothing); LoadStaticFieldInstr* load = new (Z) LoadStaticFieldInstr( - field, InstructionSource(), do_call_initializer, - do_call_initializer ? GetNextDeoptId() : DeoptId::kNone); + field, InstructionSource(), slow_path, + slow_path != SlowPathOnSentinelValue::kDoNothing ? GetNextDeoptId() + : DeoptId::kNone); Push(load); return Fragment(load); } diff --git a/runtime/vm/compiler/stub_code_compiler.cc b/runtime/vm/compiler/stub_code_compiler.cc index e30ef6c76de..dfe9ca4a4a7 100644 --- a/runtime/vm/compiler/stub_code_compiler.cc +++ b/runtime/vm/compiler/stub_code_compiler.cc @@ -50,6 +50,38 @@ void StubCodeCompiler::GenerateInitStaticFieldStub() { __ Ret(); } +void StubCodeCompiler::GenerateCheckIsolateFieldAccessStub() { + const Register kFieldReg = InitStaticFieldABI::kFieldReg; + const Register kScratchReg = InitLateStaticFieldInternalRegs::kScratchReg; + + __ EnterStubFrame(); + + Label throw_since_no_isolate_is_present; + if (!FLAG_experimental_shared_data) { + // Should not be invoked + __ Breakpoint(); + } + // This stub is also called from mutator thread running without an + // isolate and attempts to load value from isolate static field. + __ LoadIsolate(kScratchReg); + __ BranchIfZero(kScratchReg, &throw_since_no_isolate_is_present); + __ LeaveStubFrame(); + __ Ret(); + +#if defined(TARGET_ARCH_ARM) || defined(TARGET_ARCH_ARM64) + // We are jumping over LeaveStubFrame so restore LR state to match one + // at the jump point. + __ set_lr_state(compiler::LRState::OnEntry().EnterFrame()); +#endif // defined(TARGET_ARCH_ARM) || defined(TARGET_ARCH_ARM64) + // Throw FieldAccessError + __ Bind(&throw_since_no_isolate_is_present); + __ PushObject(NullObject()); // Make room for (unused) result. + __ PushRegister(kFieldReg); + __ CallRuntime(kStaticFieldAccessedWithoutIsolateErrorRuntimeEntry, + /*argument_count=*/1); + __ Breakpoint(); +} + void StubCodeCompiler::GenerateInitLateStaticFieldStub(bool is_final, bool is_shared) { const Register kResultReg = InitStaticFieldABI::kResultReg; diff --git a/runtime/vm/object_store.h b/runtime/vm/object_store.h index 09cb76bd987..2370d227229 100644 --- a/runtime/vm/object_store.h +++ b/runtime/vm/object_store.h @@ -254,6 +254,7 @@ class ObjectPointerVisitor; RW(Code, allocate_record3_stub) \ RW(Code, allocate_record3_named_stub) \ RW(Code, allocate_unhandled_exception_stub) \ + RW(Code, check_isolate_field_access_stub) \ RW(Code, clone_context_stub) \ RW(Code, write_barrier_wrappers_stub) \ RW(Code, array_write_barrier_stub) \ @@ -361,8 +362,9 @@ class ObjectPointerVisitor; DO(allocate_record3_stub, AllocateRecord3) \ DO(allocate_record3_named_stub, AllocateRecord3Named) \ DO(allocate_unhandled_exception_stub, AllocateUnhandledException) \ - DO(clone_context_stub, CloneContext) \ DO(call_closure_no_such_method_stub, CallClosureNoSuchMethod) \ + DO(clone_context_stub, CloneContext) \ + DO(check_isolate_field_access_stub, CheckIsolateFieldAccess) \ DO(default_tts_stub, DefaultTypeTest) \ DO(default_nullable_tts_stub, DefaultNullableTypeTest) \ DO(top_type_tts_stub, TopTypeTypeTest) \ diff --git a/runtime/vm/regexp/regexp_assembler_ir.cc b/runtime/vm/regexp/regexp_assembler_ir.cc index 0c644af3322..54a778bc336 100644 --- a/runtime/vm/regexp/regexp_assembler_ir.cc +++ b/runtime/vm/regexp/regexp_assembler_ir.cc @@ -518,8 +518,11 @@ void IRRegExpMacroAssembler::StoreLocal(LocalVariable* local, Value* value) { LoadStaticFieldInstr* IRRegExpMacroAssembler::LoadStaticField( const Field& field, bool calls_initializer) const { - return new (Z) LoadStaticFieldInstr(field, InstructionSource(), - calls_initializer, GetNextDeoptId()); + return new (Z) LoadStaticFieldInstr( + field, InstructionSource(), + calls_initializer ? SlowPathOnSentinelValue::kCallInitializer + : SlowPathOnSentinelValue::kDoNothing, + GetNextDeoptId()); } void IRRegExpMacroAssembler::set_current_instruction(Instruction* instruction) { diff --git a/runtime/vm/service_isolate.cc b/runtime/vm/service_isolate.cc index fba50468b3d..b680bf8f559 100644 --- a/runtime/vm/service_isolate.cc +++ b/runtime/vm/service_isolate.cc @@ -392,6 +392,9 @@ class RunServiceTask : public ThreadPool::Task { // e.g. it could have no port to communicate with it. Declare // initialization failure and shut it down. if (main_error != nullptr) { + if (FLAG_trace_service) { + OS::PrintErr("vm-service: encountered an error: %s\n", main_error); + } ShutdownIsolate(reinterpret_cast(isolate)); ServiceIsolate::InitializingFailed(main_error); return; diff --git a/runtime/vm/stub_code_list.h b/runtime/vm/stub_code_list.h index 8f82de808ce..cd37b042e83 100644 --- a/runtime/vm/stub_code_list.h +++ b/runtime/vm/stub_code_list.h @@ -191,6 +191,7 @@ namespace dart { V(AsyncExceptionHandler) \ V(CloneSuspendState) \ V(FfiAsyncCallbackSend) \ + V(CheckIsolateFieldAccess) \ V(UnknownDartCode) } // namespace dart diff --git a/tests/ffi/run_isolate_group_run_test.dart b/tests/ffi/run_isolate_group_run_test.dart index c495c6134ea..4639985fead 100644 --- a/tests/ffi/run_isolate_group_run_test.dart +++ b/tests/ffi/run_isolate_group_run_test.dart @@ -36,6 +36,15 @@ updateFooNoInitializer() { foo_no_initializer = 78; } +class Baz { + static late final foo; +} + +@pragma('vm:never-inline') +bar() { + Baz.foo = 42; +} + main() { Expect.equals(42, IsolateGroup.runSync(() => 42)); @@ -95,7 +104,7 @@ main() { (e) => e is Error && e.toString().contains("AccessError"), 'Expect error accessing', ); - Expect.equals(foo, 56); + Expect.equals(56, foo); updateFooNoInitializer(); Expect.throws( @@ -117,7 +126,12 @@ main() { (e) => e is Error && e.toString().contains("AccessError"), 'Expect error accessing', ); - Expect.equals(foo_no_initializer, 78); + Expect.equals(78, foo_no_initializer); + + { + bar(); + Expect.equals(42, Baz.foo); + } print("All tests completed :)"); }