diff --git a/runtime/vm/class_finalizer.cc b/runtime/vm/class_finalizer.cc index b6ee854eeab..cc14ad3b760 100644 --- a/runtime/vm/class_finalizer.cc +++ b/runtime/vm/class_finalizer.cc @@ -1126,7 +1126,6 @@ ErrorPtr ClassFinalizer::AllocateFinalizeClass(const Class& cls) { ErrorPtr ClassFinalizer::LoadClassMembers(const Class& cls) { ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); - ASSERT(Thread::Current()->IsMutatorThread()); ASSERT(!cls.is_finalized()); LongJumpScope jump; @@ -1663,6 +1662,7 @@ void ClassFinalizer::ClearAllCode(bool including_nonchanging_cids) { #else auto const thread = Thread::Current(); auto const isolate_group = thread->isolate_group(); + SafepointWriteRwLocker ml(thread, isolate_group->program_lock()); StackZone stack_zone(thread); HANDLESCOPE(thread); auto const zone = thread->zone(); diff --git a/runtime/vm/clustered_snapshot.cc b/runtime/vm/clustered_snapshot.cc index 134b26bdab6..12ec76e449c 100644 --- a/runtime/vm/clustered_snapshot.cc +++ b/runtime/vm/clustered_snapshot.cc @@ -741,17 +741,17 @@ class FunctionDeserializationCluster : public DeserializationCluster { func ^= refs.At(i); code = func.CurrentCode(); if (func.HasCode() && !code.IsDisabled()) { - func.SetInstructions(code); // Set entrypoint. + func.SetInstructionsSafe(code); // Set entrypoint. func.SetWasCompiled(true); } else { - func.ClearCode(); // Set code and entrypoint to lazy compile stub. + func.ClearCodeSafe(); // Set code and entrypoint to lazy compile stub } } } else { Function& func = Function::Handle(d->zone()); for (intptr_t i = start_index_; i < stop_index_; i++) { func ^= refs.At(i); - func.ClearCode(); // Set code and entrypoint to lazy compile stub. + func.ClearCodeSafe(); // Set code and entrypoint to lazy compile stub. } } } diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc index dfa42c7d76b..28b5d0fda1f 100644 --- a/runtime/vm/compiler/aot/precompiler.cc +++ b/runtime/vm/compiler/aot/precompiler.cc @@ -300,8 +300,12 @@ void Precompiler::DoCompileAll() { IG->object_store()->set_##member(stub_code); OBJECT_STORE_STUB_CODE_LIST(DO) #undef DO - stub_code = - StubCode::GetBuildMethodExtractorStub(global_object_pool_builder()); + + { + SafepointWriteRwLocker ml(T, T->isolate_group()->program_lock()); + stub_code = StubCode::GetBuildMethodExtractorStub( + global_object_pool_builder()); + } IG->object_store()->set_build_method_extractor_code(stub_code); } @@ -2519,6 +2523,8 @@ void PrecompileParsedFunctionHelper::FinalizeCompilation( const auto pool_attachment = FLAG_use_bare_instructions ? Code::PoolAttachment::kNotAttachPool : Code::PoolAttachment::kAttachPool; + + SafepointWriteRwLocker ml(T, T->isolate_group()->program_lock()); const Code& code = Code::Handle( Code::FinalizeCodeAndNotify(function, graph_compiler, assembler, pool_attachment, optimized(), stats)); diff --git a/runtime/vm/compiler/backend/il_test_helper.cc b/runtime/vm/compiler/backend/il_test_helper.cc index f552aff9b63..ab32cd46f72 100644 --- a/runtime/vm/compiler/backend/il_test_helper.cc +++ b/runtime/vm/compiler/backend/il_test_helper.cc @@ -171,8 +171,13 @@ void TestPipeline::CompileGraphAndAttachFunction() { const auto& deopt_info_array = Array::Handle(zone, graph_compiler.CreateDeoptInfo(&assembler)); const auto pool_attachment = Code::PoolAttachment::kAttachPool; - const auto& code = Code::Handle(Code::FinalizeCode( - &graph_compiler, &assembler, pool_attachment, optimized, nullptr)); + Code& code = Code::Handle(); + { + SafepointWriteRwLocker ml(thread_, + thread_->isolate_group()->program_lock()); + code ^= Code::FinalizeCode(&graph_compiler, &assembler, pool_attachment, + optimized, nullptr); + } code.set_is_optimized(optimized); code.set_owner(function_); @@ -186,11 +191,15 @@ void TestPipeline::CompileGraphAndAttachFunction() { graph_compiler.FinalizeStaticCallTargetsTable(code); graph_compiler.FinalizeCodeSourceMap(code); - if (optimized) { - function_.InstallOptimizedCode(code); - } else { - function_.set_unoptimized_code(code); - function_.AttachCode(code); + { + SafepointWriteRwLocker ml(thread_, + thread_->isolate_group()->program_lock()); + if (optimized) { + function_.InstallOptimizedCode(code); + } else { + function_.set_unoptimized_code(code); + function_.AttachCode(code); + } } // We expect there to be no deoptimizations. diff --git a/runtime/vm/dart.cc b/runtime/vm/dart.cc index ce1f9bce4af..44237ab470d 100644 --- a/runtime/vm/dart.cc +++ b/runtime/vm/dart.cc @@ -925,8 +925,13 @@ ErrorPtr Dart::InitializeIsolate(const uint8_t* snapshot_data, } else { #if !defined(TARGET_ARCH_IA32) if (I != Dart::vm_isolate()) { - IG->object_store()->set_build_method_extractor_code( - Code::Handle(StubCode::GetBuildMethodExtractorStub(nullptr))); + if (IG->object_store()->build_method_extractor_code() != nullptr) { + SafepointWriteRwLocker ml(T, IG->program_lock()); + if (IG->object_store()->build_method_extractor_code() != nullptr) { + IG->object_store()->set_build_method_extractor_code( + Code::Handle(StubCode::GetBuildMethodExtractorStub(nullptr))); + } + } } #endif // !defined(TARGET_ARCH_IA32) } diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index 35356faed9d..36e4687ab58 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -6854,10 +6854,12 @@ static void DropRegExpMatchCode(Zone* zone) { Class::Handle(zone, core_lib.LookupClassAllowPrivate(Symbols::_RegExp())); ASSERT(!reg_exp_class.IsNull()); + auto thread = Thread::Current(); Function& func = Function::Handle( zone, reg_exp_class.LookupFunctionAllowPrivate(execute_match_name)); ASSERT(!func.IsNull()); Code& code = Code::Handle(zone); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); if (func.HasCode()) { code = func.CurrentCode(); ASSERT(!code.IsNull()); diff --git a/runtime/vm/deferred_objects.cc b/runtime/vm/deferred_objects.cc index eddfb5e10cb..6452f3f78a9 100644 --- a/runtime/vm/deferred_objects.cc +++ b/runtime/vm/deferred_objects.cc @@ -152,6 +152,7 @@ void DeferredPcMarker::Materialize(DeoptContext* deopt_context) { Function& function = Function::Handle(zone); function ^= deopt_context->ObjectAt(index_); ASSERT(!function.IsNull()); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Error& error = Error::Handle(zone, Compiler::EnsureUnoptimizedCode(thread, function)); if (!error.IsNull()) { diff --git a/runtime/vm/field_table.cc b/runtime/vm/field_table.cc index 42aef79f6c7..f0458604fc7 100644 --- a/runtime/vm/field_table.cc +++ b/runtime/vm/field_table.cc @@ -54,7 +54,6 @@ intptr_t FieldTable::FieldOffsetFor(intptr_t field_id) { bool FieldTable::Register(const Field& field, intptr_t expected_field_id) { DEBUG_ASSERT( IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); - ASSERT(Thread::Current()->IsMutatorThread()); ASSERT(is_ready_to_use_); if (free_head_ < 0) { diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index 21ffcf2ead7..8fc50122ac9 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -2078,13 +2078,15 @@ void ProgramReloadContext::InvalidateFunctions( Zone* zone, const GrowableArray& functions) { TIMELINE_SCOPE(InvalidateFunctions); - HANDLESCOPE(Thread::Current()); + auto thread = Thread::Current(); + HANDLESCOPE(thread); CallSiteResetter resetter(zone); Class& owning_class = Class::Handle(zone); Library& owning_lib = Library::Handle(zone); Code& code = Code::Handle(zone); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); for (intptr_t i = 0; i < functions.length(); i++) { const Function& func = *functions[i]; diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 49907546560..fae18dfad9e 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -2909,7 +2909,6 @@ void Class::SetFunctions(const Array& value) const { const intptr_t len = value.Length(); #if defined(DEBUG) Thread* thread = Thread::Current(); - ASSERT(thread->IsMutatorThread()); ASSERT(thread->isolate_group()->program_lock()->IsCurrentThreadWriter()); if (is_finalized()) { Function& function = Function::Handle(); @@ -3807,7 +3806,6 @@ ArrayPtr Class::invocation_dispatcher_cache() const { void Class::Finalize() const { auto thread = Thread::Current(); auto isolate_group = thread->isolate_group(); - ASSERT(thread->IsMutatorThread()); ASSERT(!thread->isolate()->all_classes_finalized()); ASSERT(!is_finalized()); // Prefinalized classes have a VM internal representation and no Dart fields. @@ -3898,15 +3896,8 @@ void Class::RegisterCHACode(const Code& code) { } void Class::DisableCHAOptimizedCode(const Class& subclass) { - Thread* thread = Thread::Current(); - ASSERT(thread->IsMutatorThread()); - // TODO(dartbug.com/36097): The program_lock acquisition has to move up the - // call chain to ClassFinalizer::AllocateFinalizeClass() so that: - // - no two threads allocate-finalize a class at the same time(we should - // use the logic similar to what is used in EnsureIsAllocateFinalized()). - // - code is deoptimized before we violate optimization assumptions - // potentially done concurrently (AddDirectSubclass/AddDirectImplementor). - SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); + DEBUG_ASSERT( + IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); CHACodeArray a(*this); if (FLAG_trace_deoptimization && a.HasCodes()) { if (subclass.IsNull()) { @@ -4286,16 +4277,11 @@ ErrorPtr Class::EnsureIsFinalized(Thread* thread) const { if (is_finalized()) { return Error::null(); } - if (Compiler::IsBackgroundCompilation()) { - Compiler::AbortBackgroundCompilation(DeoptId::kNone, - "Class finalization while compiling"); - } SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); if (is_finalized()) { return Error::null(); } LeaveCompilerScope ncs(thread); - ASSERT(thread->IsMutatorThread()); ASSERT(thread != NULL); const Error& error = Error::Handle(thread->zone(), ClassFinalizer::LoadClassMembers(*this)); @@ -4316,15 +4302,10 @@ ErrorPtr Class::EnsureIsAllocateFinalized(Thread* thread) const { if (is_allocate_finalized()) { return Error::null(); } - if (Compiler::IsBackgroundCompilation()) { - Compiler::AbortBackgroundCompilation( - DeoptId::kNone, "Class allocate finalization while compiling"); - } SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); if (is_allocate_finalized()) { return Error::null(); } - ASSERT(thread->IsMutatorThread()); ASSERT(thread != NULL); Error& error = Error::Handle(thread->zone(), EnsureIsFinalized(thread)); if (!error.IsNull()) { @@ -4346,7 +4327,6 @@ void Class::SetFields(const Array& value) const { ASSERT(!value.IsNull()); #if defined(DEBUG) Thread* thread = Thread::Current(); - ASSERT(thread->IsMutatorThread()); ASSERT(thread->isolate_group()->program_lock()->IsCurrentThreadWriter()); // Verify that all the fields in the array have this class as owner. Field& field = Field::Handle(); @@ -4364,7 +4344,6 @@ void Class::SetFields(const Array& value) const { void Class::AddField(const Field& field) const { #if defined(DEBUG) Thread* thread = Thread::Current(); - ASSERT(thread->IsMutatorThread()); ASSERT(thread->isolate_group()->program_lock()->IsCurrentThreadWriter()); #endif const Array& arr = Array::Handle(fields()); @@ -4376,7 +4355,6 @@ void Class::AddField(const Field& field) const { void Class::AddFields(const GrowableArray& new_fields) const { #if defined(DEBUG) Thread* thread = Thread::Current(); - ASSERT(thread->IsMutatorThread()); ASSERT(thread->isolate_group()->program_lock()->IsCurrentThreadWriter()); #endif const intptr_t num_new_fields = new_fields.length(); @@ -5063,6 +5041,14 @@ void Class::set_allocation_stub(const Code& value) const { } void Class::DisableAllocationStub() const { + { + const Code& existing_stub = Code::Handle(allocation_stub()); + if (existing_stub.IsNull()) { + return; + } + } + auto thread = Thread::Current(); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& existing_stub = Code::Handle(allocation_stub()); if (existing_stub.IsNull()) { return; @@ -6695,7 +6681,7 @@ bool Function::HasBreakpoint() const { } void Function::InstallOptimizedCode(const Code& code) const { - DEBUG_ASSERT(IsMutatorOrAtSafepoint()); + ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); // We may not have previous code if FLAG_precompile is set. // Hot-reload may have already disabled the current code. if (HasCode() && !Code::Handle(CurrentCode()).IsDisabled()) { @@ -6705,8 +6691,14 @@ void Function::InstallOptimizedCode(const Code& code) const { } void Function::SetInstructions(const Code& value) const { - DEBUG_ASSERT(IsMutatorOrAtSafepoint()); - SetInstructionsSafe(value); + // Ensure that nobody is executing this function when we install it. + if (untag()->code() != Code::null() && HasCode()) { + SafepointOperationScope safepoint(Thread::Current()); + SetInstructionsSafe(value); + } else { + ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); + SetInstructionsSafe(value); + } } void Function::SetInstructionsSafe(const Code& value) const { @@ -6717,7 +6709,7 @@ void Function::SetInstructionsSafe(const Code& value) const { } void Function::AttachCode(const Code& value) const { - DEBUG_ASSERT(IsMutatorOrAtSafepoint()); + ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); // Finish setting up code before activating it. value.set_owner(*this); SetInstructions(value); @@ -6741,11 +6733,19 @@ void Function::ClearCode() const { #if defined(DART_PRECOMPILED_RUNTIME) UNREACHABLE(); #else - ASSERT(Thread::Current()->IsMutatorThread()); + ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); + untag()->set_unoptimized_code(Code::null()); + SetInstructions(StubCode::LazyCompile()); +#endif // defined(DART_PRECOMPILED_RUNTIME) +} +void Function::ClearCodeSafe() const { +#if defined(DART_PRECOMPILED_RUNTIME) + UNREACHABLE(); +#else untag()->set_unoptimized_code(Code::null()); - SetInstructions(StubCode::LazyCompile()); + SetInstructionsSafe(StubCode::LazyCompile()); #endif // defined(DART_PRECOMPILED_RUNTIME) } @@ -6766,9 +6766,10 @@ void Function::EnsureHasCompiledUnoptimizedCode() const { void Function::SwitchToUnoptimizedCode() const { ASSERT(HasOptimizedCode()); Thread* thread = Thread::Current(); + DEBUG_ASSERT( + thread->isolate_group()->program_lock()->IsCurrentThreadWriter()); Isolate* isolate = thread->isolate(); Zone* zone = thread->zone(); - ASSERT(thread->IsMutatorThread()); // TODO(35224): DEBUG_ASSERT(thread->TopErrorHandlerIsExitFrame()); const Code& current_code = Code::Handle(zone, CurrentCode()); @@ -10065,6 +10066,23 @@ intptr_t Field::guarded_cid() const { &untag()->guarded_cid_); } +bool Field::is_nullable(bool silence_assert) const { +#if defined(DEBUG) + if (!silence_assert) { + // Same assert as guarded_cid(), because is_nullable() also needs to be + // consistent for the background compiler. + Thread* thread = Thread::Current(); + ASSERT( + !thread->IsInsideCompiler() || +#if !defined(DART_PRECOMPILED_RUNTIME) + ((CompilerState::Current().should_clone_fields() == !IsOriginal())) || +#endif + is_static()); + } +#endif + return untag()->is_nullable_ == kNullCid; +} + void Field::SetOriginal(const Field& value) const { ASSERT(value.IsOriginal()); ASSERT(!value.IsNull()); @@ -10210,9 +10228,7 @@ intptr_t Field::KernelDataProgramOffset() const { return PatchClass::Cast(obj).library_kernel_offset(); } -// Called at finalization time -void Field::SetFieldType(const AbstractType& value) const { - ASSERT(Thread::Current()->IsMutatorThread()); +void Field::SetFieldTypeSafe(const AbstractType& value) const { ASSERT(IsOriginal()); ASSERT(!value.IsNull()); if (value.ptr() != type()) { @@ -10220,6 +10236,13 @@ void Field::SetFieldType(const AbstractType& value) const { } } +// Called at finalization time +void Field::SetFieldType(const AbstractType& value) const { + DEBUG_ASSERT( + IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); + SetFieldTypeSafe(value); +} + FieldPtr Field::New() { ASSERT(Object::field_class() != Class::null()); ObjectPtr raw = @@ -10241,7 +10264,7 @@ void Field::InitializeNew(const Field& result, result.set_name(name); result.set_is_static(is_static); if (is_static) { - result.set_field_id(-1); + result.set_field_id_unsafe(-1); } else { result.SetOffset(0, 0); } @@ -10249,12 +10272,12 @@ void Field::InitializeNew(const Field& result, result.set_is_const(is_const); result.set_is_reflectable(is_reflectable); result.set_is_late(is_late); - result.set_is_double_initialized(false); + result.set_is_double_initialized_unsafe(false); result.set_owner(owner); result.set_token_pos(token_pos); result.set_end_token_pos(end_token_pos); - result.set_has_nontrivial_initializer(false); - result.set_has_initializer(false); + result.set_has_nontrivial_initializer_unsafe(false); + result.set_has_initializer_unsafe(false); if (FLAG_precompiled_mode) { // May be updated by KernelLoader::ReadInferredType result.set_is_unboxing_candidate_unsafe(false); @@ -10307,7 +10330,7 @@ FieldPtr Field::New(const String& name, const Field& result = Field::Handle(Field::New()); InitializeNew(result, name, is_static, is_final, is_const, is_reflectable, is_late, owner, token_pos, end_token_pos); - result.SetFieldType(type); + result.SetFieldTypeSafe(type); return result.ptr(); } @@ -10374,7 +10397,6 @@ intptr_t Field::guarded_list_length() const { } void Field::set_guarded_list_length_unsafe(intptr_t list_length) const { - ASSERT(Thread::Current()->IsMutatorThread()); ASSERT(IsOriginal()); untag()->set_guarded_list_length(Smi::New(list_length)); } @@ -10385,7 +10407,6 @@ intptr_t Field::guarded_list_length_in_object_offset() const { void Field::set_guarded_list_length_in_object_offset_unsafe( intptr_t list_length_offset) const { - ASSERT(Thread::Current()->IsMutatorThread()); ASSERT(IsOriginal()); StoreNonPointer(&untag()->guarded_list_length_in_object_offset_, static_cast(list_length_offset - kHeapObjectTag)); @@ -16430,7 +16451,9 @@ CodePtr Code::FinalizeCodeAndNotify(const Function& function, PoolAttachment pool_attachment, bool optimized, CodeStatistics* stats) { - DEBUG_ASSERT(IsMutatorOrAtSafepoint()); + auto thread = Thread::Current(); + ASSERT(thread->isolate_group()->program_lock()->IsCurrentThreadWriter()); + const auto& code = Code::Handle( FinalizeCode(compiler, assembler, pool_attachment, optimized, stats)); NotifyCodeObservers(function, code, optimized); @@ -16443,7 +16466,9 @@ CodePtr Code::FinalizeCodeAndNotify(const char* name, PoolAttachment pool_attachment, bool optimized, CodeStatistics* stats) { - DEBUG_ASSERT(IsMutatorOrAtSafepoint()); + auto thread = Thread::Current(); + ASSERT(thread->isolate_group()->program_lock()->IsCurrentThreadWriter()); + const auto& code = Code::Handle( FinalizeCode(compiler, assembler, pool_attachment, optimized, stats)); NotifyCodeObservers(name, code, optimized); @@ -16460,7 +16485,8 @@ CodePtr Code::FinalizeCode(FlowGraphCompiler* compiler, PoolAttachment pool_attachment, bool optimized, CodeStatistics* stats /* = nullptr */) { - DEBUG_ASSERT(IsMutatorOrAtSafepoint()); + auto thread = Thread::Current(); + ASSERT(thread->isolate_group()->program_lock()->IsCurrentThreadWriter()); ASSERT(assembler != NULL); ObjectPool& object_pool = ObjectPool::Handle(); @@ -16514,7 +16540,6 @@ CodePtr Code::FinalizeCode(FlowGraphCompiler* compiler, // Set pointer offsets list in Code object and resolve all handles in // the instruction stream to raw objects. - Thread* thread = Thread::Current(); for (intptr_t i = 0; i < pointer_offsets.length(); i++) { intptr_t offset_in_instrs = pointer_offsets[i]; code.SetPointerOffsetAt(i, offset_in_instrs); @@ -16797,7 +16822,7 @@ bool Code::IsFunctionCode() const { } void Code::DisableDartCode() const { - DEBUG_ASSERT(IsMutatorOrAtSafepoint()); + SafepointOperationScope safepoint(Thread::Current()); ASSERT(IsFunctionCode()); ASSERT(instructions() == active_instructions()); const Code& new_code = StubCode::FixCallersTarget(); @@ -16806,7 +16831,7 @@ void Code::DisableDartCode() const { } void Code::DisableStubCode() const { - ASSERT(Thread::Current()->IsMutatorThread()); + SafepointOperationScope safepoint(Thread::Current()); ASSERT(IsAllocationStubCode()); ASSERT(instructions() == active_instructions()); const Code& new_code = StubCode::FixAllocationStubTarget(); @@ -16833,7 +16858,16 @@ void Code::SetActiveInstructions(const Instructions& instructions, #if defined(DART_PRECOMPILED_RUNTIME) UNREACHABLE(); #else - DEBUG_ASSERT(IsMutatorOrAtSafepoint() || !is_alive()); + ASSERT(IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); + SetActiveInstructionsSafe(instructions, unchecked_offset); +#endif +} + +void Code::SetActiveInstructionsSafe(const Instructions& instructions, + uint32_t unchecked_offset) const { +#if defined(DART_PRECOMPILED_RUNTIME) + UNREACHABLE(); +#else // RawInstructions are never allocated in New space and hence a // store buffer update is not needed here. untag()->set_active_instructions(instructions.ptr()); diff --git a/runtime/vm/object.h b/runtime/vm/object.h index 1a7e62d565c..3c228c3223d 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -2600,7 +2600,9 @@ class Function : public Object { void InstallOptimizedCode(const Code& code) const; void AttachCode(const Code& value) const; void SetInstructions(const Code& value) const; + void SetInstructionsSafe(const Code& value) const; void ClearCode() const; + void ClearCodeSafe() const; // Disables optimized code and switches to unoptimized code. void SwitchToUnoptimizedCode() const; @@ -3723,7 +3725,6 @@ class Function : public Object { void set_parameter_names(const Array& value) const; void set_parameter_types(const Array& value) const; void set_ic_data_array(const Array& value) const; - void SetInstructionsSafe(const Code& value) const; void set_name(const String& value) const; void set_kind(UntaggedFunction::Kind value) const; void set_parent_function(const Function& value) const; @@ -3888,13 +3889,18 @@ class Field : public Object { } // Called in parser after allocating field, immutable property otherwise. // Marks fields that are initialized with a simple double constant. - void set_is_double_initialized(bool value) const { - ASSERT(Thread::Current()->IsMutatorThread()); + void set_is_double_initialized_unsafe(bool value) const { ASSERT(IsOriginal()); // TODO(36097): Once concurrent access is possible ensure updates are safe. set_kind_bits(DoubleInitializedBit::update(value, untag()->kind_bits_)); } + void set_is_double_initialized(bool value) const { + DEBUG_ASSERT( + IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); + set_is_double_initialized_unsafe(value); + } + bool initializer_changed_after_initialization() const { return InitializerChangedAfterInitializatonBit::decode(kind_bits()); } @@ -3968,6 +3974,7 @@ class Field : public Object { inline intptr_t field_id() const; inline void set_field_id(intptr_t field_id) const; + inline void set_field_id_unsafe(intptr_t field_id) const; ClassPtr Owner() const; ClassPtr Origin() const; // Either mixin class, or same as owner(). @@ -3977,6 +3984,7 @@ class Field : public Object { AbstractTypePtr type() const { return untag()->type(); } // Used by class finalizer, otherwise initialized in constructor. void SetFieldType(const AbstractType& value) const; + void SetFieldTypeSafe(const AbstractType& value) const; DART_WARN_UNUSED_RESULT ErrorPtr VerifyEntryPoint(EntryPointPragma kind) const; @@ -4023,25 +4031,34 @@ class Field : public Object { return HasNontrivialInitializerBit::decode(kind_bits()); } // Called by parser after allocating field. - void set_has_nontrivial_initializer(bool has_nontrivial_initializer) const { + void set_has_nontrivial_initializer_unsafe( + bool has_nontrivial_initializer) const { ASSERT(IsOriginal()); - ASSERT(Thread::Current()->IsMutatorThread()); // TODO(36097): Once concurrent access is possible ensure updates are safe. set_kind_bits(HasNontrivialInitializerBit::update( has_nontrivial_initializer, untag()->kind_bits_)); } + void set_has_nontrivial_initializer(bool has_nontrivial_initializer) const { + DEBUG_ASSERT( + IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); + set_has_nontrivial_initializer_unsafe(has_nontrivial_initializer); + } bool has_initializer() const { return HasInitializerBit::decode(kind_bits()); } // Called by parser after allocating field. - void set_has_initializer(bool has_initializer) const { + void set_has_initializer_unsafe(bool has_initializer) const { ASSERT(IsOriginal()); - ASSERT(Thread::Current()->IsMutatorThread()); // TODO(36097): Once concurrent access is possible ensure updates are safe. set_kind_bits( HasInitializerBit::update(has_initializer, untag()->kind_bits_)); } + void set_has_initializer(bool has_initializer) const { + DEBUG_ASSERT( + IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); + set_has_initializer_unsafe(has_initializer); + } bool has_trivial_initializer() const { return has_initializer() && !has_nontrivial_initializer(); @@ -4167,25 +4184,13 @@ class Field : public Object { // Internally we is_nullable_ field contains either kNullCid (nullable) or // kInvalidCid (non-nullable) instead of boolean. This is done to simplify // guarding sequence in the generated code. - bool is_nullable(bool silence_assert = false) const { -#if defined(DEBUG) - if (!silence_assert) { - // Same assert as guarded_cid(), because is_nullable() also needs to be - // consistent for the background compiler. - Thread* thread = Thread::Current(); - ASSERT(!IsOriginal() || is_static() || thread->IsMutatorThread() || - thread->IsAtSafepoint()); - } -#endif - return untag()->is_nullable_ == kNullCid; - } + bool is_nullable(bool silence_assert = false) const; void set_is_nullable(bool val) const { DEBUG_ASSERT( IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); set_is_nullable_unsafe(val); } void set_is_nullable_unsafe(bool val) const { - ASSERT(Thread::Current()->IsMutatorThread()); StoreNonPointer(&untag()->is_nullable_, val ? kNullCid : kIllegalCid); } static intptr_t is_nullable_offset() { @@ -6386,7 +6391,6 @@ class Code : public Object { void Enable() const { if (!IsDisabled()) return; - ASSERT(Thread::Current()->IsMutatorThread()); ResetActiveInstructions(); } @@ -6463,6 +6467,8 @@ class Code : public Object { // entry point addresses. void SetActiveInstructions(const Instructions& instructions, uint32_t unchecked_offset) const; + void SetActiveInstructionsSafe(const Instructions& instructions, + uint32_t unchecked_offset) const; // Resets [active_instructions_] to its original value of [instructions_] and // updates the cached entry point addresses to match. @@ -11290,8 +11296,13 @@ inline intptr_t Field::field_id() const { } void Field::set_field_id(intptr_t field_id) const { + DEBUG_ASSERT( + IsolateGroup::Current()->program_lock()->IsCurrentThreadWriter()); + set_field_id_unsafe(field_id); +} + +void Field::set_field_id_unsafe(intptr_t field_id) const { ASSERT(is_static()); - ASSERT(Thread::Current()->IsMutatorThread()); untag()->set_host_offset_or_field_id(Smi::New(field_id)); } diff --git a/runtime/vm/object_reload.cc b/runtime/vm/object_reload.cc index 0838b247943..2f9f23859df 100644 --- a/runtime/vm/object_reload.cc +++ b/runtime/vm/object_reload.cc @@ -226,7 +226,7 @@ void Class::CopyStaticFieldValues(ProgramReloadContext* reload_context, reload_context->isolate()->group()->initial_field_table()->Free( field.field_id()); reload_context->isolate()->field_table()->Free(field.field_id()); - field.set_field_id(old_field.field_id()); + field.set_field_id_unsafe(old_field.field_id()); } reload_context->AddStaticFieldMapping(old_field, field); } else { diff --git a/runtime/vm/object_test.cc b/runtime/vm/object_test.cc index 1d0c396bc7b..cad9d625356 100644 --- a/runtime/vm/object_test.cc +++ b/runtime/vm/object_test.cc @@ -2729,6 +2729,8 @@ ISOLATE_UNIT_TEST_CASE(Code) { compiler::Assembler _assembler_(&object_pool_builder); GenerateIncrement(&_assembler_); const Function& function = Function::Handle(CreateFunction("Test_Code")); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); Code& code = Code::Handle(Code::FinalizeCodeAndNotify( function, nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); function.AttachCode(code); @@ -2751,6 +2753,8 @@ ISOLATE_UNIT_TEST_CASE_WITH_EXPECTATION(CodeImmutability, "Crash") { compiler::Assembler _assembler_(&object_pool_builder); GenerateIncrement(&_assembler_); const Function& function = Function::Handle(CreateFunction("Test_Code")); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); Code& code = Code::Handle(Code::FinalizeCodeAndNotify( function, nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); function.AttachCode(code); @@ -2789,6 +2793,8 @@ ISOLATE_UNIT_TEST_CASE_WITH_EXPECTATION(CodeExecutability, "Crash") { compiler::Assembler _assembler_(&object_pool_builder); GenerateIncrement(&_assembler_); const Function& function = Function::Handle(CreateFunction("Test_Code")); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); Code& code = Code::Handle(Code::FinalizeCodeAndNotify( function, nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); function.AttachCode(code); @@ -2830,6 +2836,8 @@ ISOLATE_UNIT_TEST_CASE(EmbedStringInCode) { GenerateEmbedStringInCode(&_assembler_, kHello); const Function& function = Function::Handle(CreateFunction("Test_EmbedStringInCode")); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( function, nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); function.AttachCode(code); @@ -2854,6 +2862,8 @@ ISOLATE_UNIT_TEST_CASE(EmbedSmiInCode) { GenerateEmbedSmiInCode(&_assembler_, kSmiTestValue); const Function& function = Function::Handle(CreateFunction("Test_EmbedSmiInCode")); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( function, nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); function.AttachCode(code); @@ -2873,6 +2883,8 @@ ISOLATE_UNIT_TEST_CASE(EmbedSmiIn64BitCode) { GenerateEmbedSmiInCode(&_assembler_, kSmiTestValue); const Function& function = Function::Handle(CreateFunction("Test_EmbedSmiIn64BitCode")); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( function, nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); function.AttachCode(code); @@ -2898,6 +2910,8 @@ ISOLATE_UNIT_TEST_CASE(ExceptionHandlers) { compiler::ObjectPoolBuilder object_pool_builder; compiler::Assembler _assembler_(&object_pool_builder); GenerateIncrement(&_assembler_); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); Code& code = Code::Handle(Code::FinalizeCodeAndNotify( Function::Handle(CreateFunction("Test_Code")), nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); @@ -2945,6 +2959,8 @@ ISOLATE_UNIT_TEST_CASE(PcDescriptors) { compiler::ObjectPoolBuilder object_pool_builder; compiler::Assembler _assembler_(&object_pool_builder); GenerateIncrement(&_assembler_); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); Code& code = Code::Handle(Code::FinalizeCodeAndNotify( Function::Handle(CreateFunction("Test_Code")), nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); @@ -3015,6 +3031,8 @@ ISOLATE_UNIT_TEST_CASE(PcDescriptorsLargeDeltas) { compiler::ObjectPoolBuilder object_pool_builder; compiler::Assembler _assembler_(&object_pool_builder); GenerateIncrement(&_assembler_); + SafepointWriteRwLocker locker(thread, + thread->isolate_group()->program_lock()); Code& code = Code::Handle(Code::FinalizeCodeAndNotify( Function::Handle(CreateFunction("Test_Code")), nullptr, &_assembler_, Code::PoolAttachment::kAttachPool)); diff --git a/runtime/vm/program_visitor.cc b/runtime/vm/program_visitor.cc index 66c6a5813d3..b8e3de13338 100644 --- a/runtime/vm/program_visitor.cc +++ b/runtime/vm/program_visitor.cc @@ -1220,7 +1220,7 @@ void ProgramVisitor::DedupInstructions(Zone* zone, // ProgramWalker, but as long as the deduplication process is idempotent, // the cached entry points won't change during the second visit. VisitCode(code_); - function.SetInstructions(code_); // Update cached entry point. + function.SetInstructionsSafe(code_); // Update cached entry point. } void VisitCode(const Code& code) { @@ -1231,8 +1231,8 @@ void ProgramVisitor::DedupInstructions(Zone* zone, instructions_ = code.active_instructions(); instructions_ = Dedup(instructions_); } - code.SetActiveInstructions(instructions_, - code.UncheckedEntryPointOffset()); + code.SetActiveInstructionsSafe(instructions_, + code.UncheckedEntryPointOffset()); } private: @@ -1258,15 +1258,15 @@ void ProgramVisitor::DedupInstructions(Zone* zone, // ProgramWalker, but as long as the deduplication process is idempotent, // the cached entry points won't change during the second visit. VisitCode(code_); - function.SetInstructions(code_); // Update cached entry point. + function.SetInstructionsSafe(code_); // Update cached entry point. } void VisitCode(const Code& code) { if (code.IsDisabled()) return; canonical_ = Dedup(code); instructions_ = canonical_.instructions(); - code.SetActiveInstructions(instructions_, - code.UncheckedEntryPointOffset()); + code.SetActiveInstructionsSafe(instructions_, + code.UncheckedEntryPointOffset()); code.set_instructions(instructions_); } diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 20125f3df11..8ad3ca9478d 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -2940,7 +2940,10 @@ void DeoptimizeAt(const Code& optimized_code, StackFrame* frame) { ASSERT(!unoptimized_code.IsNull()); // The switch to unoptimized code may have already occurred. if (function.HasOptimizedCode()) { - function.SwitchToUnoptimizedCode(); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); + if (function.HasOptimizedCode()) { + function.SwitchToUnoptimizedCode(); + } } if (frame->IsMarkedForLazyDeopt()) { diff --git a/runtime/vm/stub_code.cc b/runtime/vm/stub_code.cc index 7f09adc0130..e41c6421a09 100644 --- a/runtime/vm/stub_code.cc +++ b/runtime/vm/stub_code.cc @@ -69,6 +69,9 @@ CodePtr StubCode::Generate( const char* name, compiler::ObjectPoolBuilder* object_pool_builder, void (*GenerateStub)(compiler::Assembler* assembler)) { + auto thread = Thread::Current(); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); + compiler::Assembler assembler(object_pool_builder); GenerateStub(&assembler); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( @@ -188,11 +191,13 @@ CodePtr StubCode::GetAllocationStubForClass(const Class& cls) { Array::Handle(zone, compiler::StubCodeCompiler::BuildStaticCallsTable( zone, &unresolved_calls)); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); + auto mutator_fun = [&]() { stub = Code::FinalizeCode(nullptr, &assembler, pool_attachment, /*optimized=*/false, /*stats=*/nullptr); - // Check if background compilation thread has not already added the stub. + // Check if some other thread has not already added the stub. if (cls.allocation_stub() == Code::null()) { stub.set_owner(cls); if (!static_calls_table.IsNull()) { @@ -201,32 +206,13 @@ CodePtr StubCode::GetAllocationStubForClass(const Class& cls) { cls.set_allocation_stub(stub); } }; - auto bg_compiler_fun = [&]() { - ASSERT(Thread::Current()->IsAtSafepoint()); - stub = cls.allocation_stub(); - // Check if stub was already generated. - if (!stub.IsNull()) { - return; - } - stub = Code::FinalizeCode(nullptr, &assembler, pool_attachment, - /*optimized=*/false, /*stats=*/nullptr); - stub.set_owner(cls); - if (!static_calls_table.IsNull()) { - stub.set_static_calls_target_table(static_calls_table); - } - cls.set_allocation_stub(stub); - }; // We have to ensure no mutators are running, because: // // a) We allocate an instructions object, which might cause us to // temporarily flip page protections from (RX -> RW -> RX). - // - // b) To ensure only one thread succeeds installing an allocation for the - // given class. - // - thread->isolate_group()->RunWithStoppedMutators( - mutator_fun, bg_compiler_fun, /*use_force_growth=*/true); + thread->isolate_group()->RunWithStoppedMutators(mutator_fun, + /*use_force_growth=*/true); // We notify code observers after finalizing the code in order to be // outside a [SafepointOperationScope]. diff --git a/runtime/vm/stub_code_arm64_test.cc b/runtime/vm/stub_code_arm64_test.cc index ee35305511d..9814bba849e 100644 --- a/runtime/vm/stub_code_arm64_test.cc +++ b/runtime/vm/stub_code_arm64_test.cc @@ -60,6 +60,7 @@ ISOLATE_UNIT_TEST_CASE(CallRuntimeStubCode) { compiler::ObjectPoolBuilder object_pool_builder; compiler::Assembler assembler(&object_pool_builder); GenerateCallToCallRuntimeStub(&assembler, length); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( *CreateFunction("Test_CallRuntimeStubCode"), nullptr, &assembler, Code::PoolAttachment::kAttachPool)); @@ -102,6 +103,7 @@ ISOLATE_UNIT_TEST_CASE(CallLeafRuntimeStubCode) { compiler::Assembler assembler(&object_pool_builder); GenerateCallToCallLeafRuntimeStub(&assembler, str_value, lhs_index_value, rhs_index_value, length_value); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( *CreateFunction("Test_CallLeafRuntimeStubCode"), nullptr, &assembler, Code::PoolAttachment::kAttachPool)); diff --git a/runtime/vm/stub_code_arm_test.cc b/runtime/vm/stub_code_arm_test.cc index f5cabff23e7..9fee1fd95d5 100644 --- a/runtime/vm/stub_code_arm_test.cc +++ b/runtime/vm/stub_code_arm_test.cc @@ -59,6 +59,7 @@ ISOLATE_UNIT_TEST_CASE(CallRuntimeStubCode) { compiler::ObjectPoolBuilder object_pool_builder; compiler::Assembler assembler(&object_pool_builder); GenerateCallToCallRuntimeStub(&assembler, length); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( *CreateFunction("Test_CallRuntimeStubCode"), nullptr, &assembler, Code::PoolAttachment::kAttachPool)); @@ -100,6 +101,7 @@ ISOLATE_UNIT_TEST_CASE(CallLeafRuntimeStubCode) { compiler::Assembler assembler(&object_pool_builder); GenerateCallToCallLeafRuntimeStub(&assembler, str_value, lhs_index_value, rhs_index_value, length_value); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( *CreateFunction("Test_CallLeafRuntimeStubCode"), nullptr, &assembler, Code::PoolAttachment::kAttachPool)); diff --git a/runtime/vm/stub_code_ia32_test.cc b/runtime/vm/stub_code_ia32_test.cc index f86f89579de..13473adbabd 100644 --- a/runtime/vm/stub_code_ia32_test.cc +++ b/runtime/vm/stub_code_ia32_test.cc @@ -59,6 +59,7 @@ ISOLATE_UNIT_TEST_CASE(CallRuntimeStubCode) { const char* kName = "Test_CallRuntimeStubCode"; compiler::Assembler assembler(nullptr); GenerateCallToCallRuntimeStub(&assembler, length); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( *CreateFunction("Test_CallRuntimeStubCode"), nullptr, &assembler, Code::PoolAttachment::kAttachPool)); @@ -104,6 +105,7 @@ ISOLATE_UNIT_TEST_CASE(CallLeafRuntimeStubCode) { compiler::Assembler assembler(nullptr); GenerateCallToCallLeafRuntimeStub(&assembler, str_value, lhs_index_value, rhs_index_value, length_value); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( *CreateFunction("Test_CallLeafRuntimeStubCode"), nullptr, &assembler, Code::PoolAttachment::kAttachPool)); diff --git a/runtime/vm/stub_code_x64_test.cc b/runtime/vm/stub_code_x64_test.cc index 909d124ad12..c586af81d09 100644 --- a/runtime/vm/stub_code_x64_test.cc +++ b/runtime/vm/stub_code_x64_test.cc @@ -60,6 +60,7 @@ ISOLATE_UNIT_TEST_CASE(CallRuntimeStubCode) { compiler::ObjectPoolBuilder object_pool_builder; compiler::Assembler assembler(&object_pool_builder); GenerateCallToCallRuntimeStub(&assembler, length); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( *CreateFunction("Test_CallRuntimeStubCode"), nullptr, &assembler, Code::PoolAttachment::kAttachPool)); @@ -102,6 +103,7 @@ ISOLATE_UNIT_TEST_CASE(CallLeafRuntimeStubCode) { compiler::Assembler assembler(&object_pool_builder); GenerateCallToCallLeafRuntimeStub(&assembler, str_value, lhs_index_value, rhs_index_value, length_value); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); const Code& code = Code::Handle(Code::FinalizeCodeAndNotify( *CreateFunction("Test_CallLeafRuntimeStubCode"), nullptr, &assembler, Code::PoolAttachment::kAttachPool)); diff --git a/runtime/vm/type_testing_stubs.cc b/runtime/vm/type_testing_stubs.cc index 5fed5d267fc..686b69f4c8b 100644 --- a/runtime/vm/type_testing_stubs.cc +++ b/runtime/vm/type_testing_stubs.cc @@ -243,6 +243,7 @@ CodePtr TypeTestingStubGenerator::BuildCodeForType(const Type& type) { // a) We allocate an instructions object, which might cause us to // temporarily flip page protections from (RX -> RW -> RX). // + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); thread->isolate_group()->RunWithStoppedMutators(install_code_fun, /*use_force_growth=*/true); diff --git a/runtime/vm/unit_test.cc b/runtime/vm/unit_test.cc index 2a2a861d2be..e1370f8e2f1 100644 --- a/runtime/vm/unit_test.cc +++ b/runtime/vm/unit_test.cc @@ -654,8 +654,8 @@ static bool IsHex(int c) { #endif void AssemblerTest::Assemble() { - const String& function_name = - String::ZoneHandle(Symbols::New(Thread::Current(), name_)); + auto thread = Thread::Current(); + const String& function_name = String::ZoneHandle(Symbols::New(thread, name_)); // We make a dummy script so that exception objects can be composed for // assembler instructions that do runtime calls. @@ -669,6 +669,7 @@ void AssemblerTest::Assemble() { Function& function = Function::ZoneHandle(Function::New( signature, function_name, UntaggedFunction::kRegularFunction, true, false, false, false, false, cls, TokenPosition::kMinSource)); + SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock()); code_ = Code::FinalizeCodeAndNotify(function, nullptr, assembler_, Code::PoolAttachment::kAttachPool); code_.set_owner(function);