diff --git a/pkg/vm/lib/metadata/table_selector.dart b/pkg/vm/lib/metadata/table_selector.dart index d107a9fba1d..938d5c56457 100644 --- a/pkg/vm/lib/metadata/table_selector.dart +++ b/pkg/vm/lib/metadata/table_selector.dart @@ -8,15 +8,23 @@ import 'procedure_attributes.dart'; // Information associated with a selector, used by the dispatch table generator. class TableSelectorInfo { - int callCount; + static const int kCalledOnNullBit = 1 << 0; + static const int kCallCountShift = 1; - TableSelectorInfo() : callCount = 0; + int callCount; + bool calledOnNull; + + TableSelectorInfo() + : callCount = 0, + calledOnNull = false; TableSelectorInfo.readFromBinary(BinarySource source) - : callCount = source.readUInt(); + : callCount = source.readUInt(), + calledOnNull = source.readByte() != 0; void writeToBinary(BinarySink sink) { sink.writeUInt30(callCount); + sink.writeByte(calledOnNull ? 1 : 0); } } diff --git a/pkg/vm/lib/transformations/type_flow/table_selector.dart b/pkg/vm/lib/transformations/type_flow/table_selector.dart index be07bb6b512..2533d4897fd 100644 --- a/pkg/vm/lib/transformations/type_flow/table_selector.dart +++ b/pkg/vm/lib/transformations/type_flow/table_selector.dart @@ -60,15 +60,16 @@ class TableSelectorAssigner { throw "Unexpected member kind '${member.runtimeType}'"; } - void registerCall(int selectorId) { + void registerCall(int selectorId, bool calledOnNull) { metadata.selectors[selectorId].callCount++; + metadata.selectors[selectorId].calledOnNull |= calledOnNull; } - void registerMethodOrSetterCall(Member member) { - registerCall(methodOrSetterSelectorId(member)); + void registerMethodOrSetterCall(Member member, bool calledOnNull) { + registerCall(methodOrSetterSelectorId(member), calledOnNull); } - void registerGetterCall(Member member) { - registerCall(getterSelectorId(member)); + void registerGetterCall(Member member, bool calledOnNull) { + registerCall(getterSelectorId(member), calledOnNull); } } diff --git a/pkg/vm/lib/transformations/type_flow/transformer.dart b/pkg/vm/lib/transformations/type_flow/transformer.dart index 46a7df53b9d..37622d256bc 100644 --- a/pkg/vm/lib/transformations/type_flow/transformer.dart +++ b/pkg/vm/lib/transformations/type_flow/transformer.dart @@ -263,10 +263,12 @@ class AnnotateKernel extends RecursiveVisitor { final Selector selector = callSite.selector; if (selector is InterfaceSelector && !_callSiteUsesDirectCall(node)) { if (node is PropertyGet) { - _tableSelectorAssigner.registerGetterCall(selector.member); + _tableSelectorAssigner.registerGetterCall( + selector.member, callSite.isNullableReceiver); } else { assertx(node is MethodInvocation || node is PropertySet); - _tableSelectorAssigner.registerMethodOrSetterCall(selector.member); + _tableSelectorAssigner.registerMethodOrSetterCall( + selector.member, callSite.isNullableReceiver); } } } diff --git a/runtime/vm/compiler/aot/aot_call_specializer.cc b/runtime/vm/compiler/aot/aot_call_specializer.cc index 9e52a228ade..00e1c476ab7 100644 --- a/runtime/vm/compiler/aot/aot_call_specializer.cc +++ b/runtime/vm/compiler/aot/aot_call_specializer.cc @@ -1303,12 +1303,6 @@ void AotCallSpecializer::TryReplaceWithDispatchTableCall( return; } - if (!selector->on_null_interface) { - // Selector not implemented by Null. Add null check if receiver is nullable. - AddCheckNull(receiver->CopyWithType(Z), call->function_name(), - DeoptId::kNone, call->env(), call); - } - const AbstractType& target_type = AbstractType::Handle(Class::Handle(interface_target.Owner()).RareType()); const bool receiver_can_be_smi = diff --git a/runtime/vm/compiler/aot/dispatch_table_generator.cc b/runtime/vm/compiler/aot/dispatch_table_generator.cc index f89b3017105..7ce4ea6f4ca 100644 --- a/runtime/vm/compiler/aot/dispatch_table_generator.cc +++ b/runtime/vm/compiler/aot/dispatch_table_generator.cc @@ -109,7 +109,7 @@ class SelectorRow { void DefineSelectorImplementationForInterval(classid_t cid, int16_t depth, const Interval& range, - const Function& function); + const Function* function); bool Finalize(); int32_t CallCount() const { return selector_->call_count; } @@ -168,8 +168,8 @@ void SelectorRow::DefineSelectorImplementationForInterval( classid_t cid, int16_t depth, const Interval& range, - const Function& function) { - CidInterval cid_range(cid, depth, range, &function); + const Function* function) { + CidInterval cid_range(cid, depth, range, function); class_ranges_.Add(cid_range); } @@ -251,7 +251,7 @@ void SelectorRow::FillTable(ClassTable* class_table, DispatchTable* table) { const CidInterval& cid_range = class_ranges_[i]; const Interval& range = cid_range.range(); const Function* function = cid_range.function(); - if (function->HasCode()) { + if (function != nullptr && function->HasCode()) { code = function->CurrentCode(); for (classid_t cid = range.begin(); cid < range.end(); cid++) { table->SetCodeAt(selector()->offset + cid, code); @@ -378,9 +378,10 @@ const TableSelector* SelectorMap::GetSelector( return selector; } -void SelectorMap::AddSelector(int32_t call_count) { +void SelectorMap::AddSelector(int32_t call_count, bool called_on_null) { const int32_t added_sid = selectors_.length(); - selectors_.Add(TableSelector(added_sid, call_count, kInvalidSelectorOffset)); + selectors_.Add(TableSelector(added_sid, call_count, kInvalidSelectorOffset, + called_on_null)); } void SelectorMap::SetSelectorProperties(int32_t sid, @@ -417,7 +418,7 @@ void DispatchTableGenerator::ReadTableSelectorInfo() { RELEASE_ASSERT(metadata != nullptr); for (intptr_t i = 0; i < metadata->selectors.length(); i++) { const kernel::TableSelectorInfo* info = &metadata->selectors[i]; - selector_map_.AddSelector(info->call_count); + selector_map_.AddSelector(info->call_count, info->called_on_null); } } @@ -533,7 +534,12 @@ void DispatchTableGenerator::SetupSelectorRows() { // Initialize selector rows. SelectorRow* selector_rows = Z->Alloc(num_selectors_); for (intptr_t i = 0; i < num_selectors_; i++) { - new (&selector_rows[i]) SelectorRow(Z, &selector_map_.selectors_[i]); + TableSelector* selector = &selector_map_.selectors_[i]; + new (&selector_rows[i]) SelectorRow(Z, selector); + if (selector->called_on_null && !selector->on_null_interface) { + selector_rows[i].DefineSelectorImplementationForInterval( + kNullCid, 0, Interval(kNullCid, kNullCid + 1), nullptr); + } } // Add implementation intervals to the selector rows for all classes that @@ -559,7 +565,7 @@ void DispatchTableGenerator::SetupSelectorRows() { for (intptr_t i = 0; i < subclasss_cid_ranges.length(); i++) { Interval& subclass_cid_range = subclasss_cid_ranges[i]; selector_rows[sid].DefineSelectorImplementationForInterval( - cid, depth, subclass_cid_range, function_handle); + cid, depth, subclass_cid_range, &function_handle); } } } diff --git a/runtime/vm/compiler/aot/dispatch_table_generator.h b/runtime/vm/compiler/aot/dispatch_table_generator.h index bbd9ac6c211..6cbf816725e 100644 --- a/runtime/vm/compiler/aot/dispatch_table_generator.h +++ b/runtime/vm/compiler/aot/dispatch_table_generator.h @@ -19,15 +19,29 @@ namespace compiler { class SelectorRow; struct TableSelector { - TableSelector(int32_t id, int32_t call_count, int32_t offset) - : id(id), call_count(call_count), offset(offset) {} + TableSelector(int32_t _id, + int32_t _call_count, + int32_t _offset, + bool _called_on_null) + : id(_id), + call_count(_call_count), + offset(_offset), + called_on_null(_called_on_null) {} bool IsUsed() const { return call_count > 0; } + // ID assigned to the selector. int32_t id; + // Number of dispatch table call sites with this selector (conservative: + // number may be bigger, but not smaller, than actual number of call sites). int32_t call_count; + // Table offset assigned to the selector by the dispatch table generator. int32_t offset; + // Are there any call sites with this selector where the receiver may be null? + bool called_on_null; + // Is the selector part of the interface on Null (same as Object)? bool on_null_interface = false; + // Do any targets of this selector assume that an args descriptor is passed? bool requires_args_descriptor = false; }; @@ -46,7 +60,7 @@ class SelectorMap { int32_t SelectorId(const Function& interface_target) const; - void AddSelector(int32_t call_count); + void AddSelector(int32_t call_count, bool called_on_null); void SetSelectorProperties(int32_t sid, bool on_null_interface, bool requires_args_descriptor); diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc index 7dffac58ba9..9ea3bd70b17 100644 --- a/runtime/vm/compiler/aot/precompiler.cc +++ b/runtime/vm/compiler/aot/precompiler.cc @@ -292,6 +292,10 @@ void Precompiler::DoCompileAll() { StubCode::GetBuildMethodExtractorStub(global_object_pool_builder()); I->object_store()->set_build_method_extractor_code(stub_code); + stub_code = StubCode::BuildIsolateSpecificDispatchTableNullErrorStub( + global_object_pool_builder()); + I->object_store()->set_dispatch_table_null_error_stub(stub_code); + stub_code = StubCode::BuildIsolateSpecificNullErrorSharedWithFPURegsStub( global_object_pool_builder()); diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.cc b/runtime/vm/compiler/backend/flow_graph_compiler.cc index 46cfcb96592..6675ac63a5c 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler.cc @@ -745,11 +745,18 @@ void FlowGraphCompiler::AddCurrentDescriptor(RawPcDescriptors::Kind kind, CurrentTryIndex()); } -void FlowGraphCompiler::AddNullCheck(intptr_t pc_offset, - TokenPosition token_pos, - intptr_t null_check_name_idx) { - code_source_map_builder_->NoteNullCheck(pc_offset, token_pos, - null_check_name_idx); +void FlowGraphCompiler::AddNullCheck(TokenPosition token_pos, + const String& name) { + // If we have DWARF stack traces enabled, the AOT runtime is unable to obtain + // the pool index at runtime. There is therefore no reason to put the name + // into the pool in the first place. + // TODO(dartbug.com/40605): Move this info to the pc descriptors. + if (!FLAG_dwarf_stack_traces) { + const intptr_t name_index = + assembler()->object_pool_builder().FindObject(name); + code_source_map_builder_->NoteNullCheck(assembler()->CodeSize(), token_pos, + name_index); + } } void FlowGraphCompiler::AddPcRelativeCallTarget(const Function& function, diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.h b/runtime/vm/compiler/backend/flow_graph_compiler.h index 5c94d9a8e77..1668630cb1c 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler.h +++ b/runtime/vm/compiler/backend/flow_graph_compiler.h @@ -819,9 +819,8 @@ class FlowGraphCompiler : public ValueObject { intptr_t try_index, intptr_t yield_index = RawPcDescriptors::kInvalidYieldIndex); - void AddNullCheck(intptr_t pc_offset, - TokenPosition token_pos, - intptr_t null_check_name_idx); + // Add NullCheck information for the current PC. + void AddNullCheck(TokenPosition token_pos, const String& name); void RecordSafepoint(LocationSummary* locs, intptr_t slow_path_argument_count = 0); diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc index 3d228e338ca..77c5ca6a309 100644 --- a/runtime/vm/compiler/backend/il.cc +++ b/runtime/vm/compiler/backend/il.cc @@ -4457,6 +4457,14 @@ void DispatchTableCallInstr::EmitNativeCode(FlowGraphCompiler* compiler) { arguments_descriptor); compiler->EmitCallsiteMetadata(token_pos(), DeoptId::kNone, RawPcDescriptors::kOther, locs()); + if (selector()->called_on_null && !selector()->on_null_interface) { + Value* receiver = ArgumentValueAt(FirstArgIndex()); + if (receiver->Type()->is_nullable()) { + const String& function_name = + String::ZoneHandle(interface_target().name()); + compiler->AddNullCheck(token_pos(), function_name); + } + } __ Drop(ArgumentCount()); compiler->AddDispatchTableCallTarget(selector()); @@ -4862,11 +4870,7 @@ LocationSummary* CheckNullInstr::MakeLocationSummary(Zone* zone, void CheckNullInstr::AddMetadataForRuntimeCall(CheckNullInstr* check_null, FlowGraphCompiler* compiler) { - const String& function_name = check_null->function_name(); - const intptr_t name_index = - compiler->assembler()->object_pool_builder().FindObject(function_name); - compiler->AddNullCheck(compiler->assembler()->CodeSize(), - check_null->token_pos(), name_index); + compiler->AddNullCheck(check_null->token_pos(), check_null->function_name()); } void UnboxInstr::EmitLoadFromBoxWithDeopt(FlowGraphCompiler* compiler) { diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.cc b/runtime/vm/compiler/frontend/kernel_translation_helper.cc index 4d81b875f32..c401baf2263 100644 --- a/runtime/vm/compiler/frontend/kernel_translation_helper.cc +++ b/runtime/vm/compiler/frontend/kernel_translation_helper.cc @@ -1851,6 +1851,7 @@ TableSelectorMetadata* TableSelectorMetadataHelper::GetTableSelectorMetadata( void TableSelectorMetadataHelper::ReadTableSelectorInfo( TableSelectorInfo* info) { info->call_count = helper_->ReadUInt(); + info->called_on_null = helper_->ReadByte() != 0; } intptr_t KernelReaderHelper::ReaderOffset() const { diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.h b/runtime/vm/compiler/frontend/kernel_translation_helper.h index d836db9a1d1..e87c35682ea 100644 --- a/runtime/vm/compiler/frontend/kernel_translation_helper.h +++ b/runtime/vm/compiler/frontend/kernel_translation_helper.h @@ -1030,6 +1030,7 @@ class CallSiteAttributesMetadataHelper : public MetadataHelper { // Information about a table selector computed by the TFA. struct TableSelectorInfo { int call_count = 0; + bool called_on_null = true; }; // Collection of table selector information for all selectors in the program. @@ -1055,6 +1056,9 @@ class TableSelectorMetadataHelper : public MetadataHelper { TableSelectorMetadata* GetTableSelectorMetadata(Zone* zone); private: + static const int32_t kCalledOnNullBit = 1 << 0; + static const int32_t kCallCountShift = 1; + void ReadTableSelectorInfo(TableSelectorInfo* info); DISALLOW_COPY_AND_ASSIGN(TableSelectorMetadataHelper); diff --git a/runtime/vm/compiler/stub_code_compiler_arm.cc b/runtime/vm/compiler/stub_code_compiler_arm.cc index 0fdf071ad18..72d69f20913 100644 --- a/runtime/vm/compiler/stub_code_compiler_arm.cc +++ b/runtime/vm/compiler/stub_code_compiler_arm.cc @@ -469,6 +469,14 @@ void StubCodeCompiler::GenerateJITCallbackTrampolines( } #endif // !defined(DART_PRECOMPILER) +void StubCodeCompiler::GenerateDispatchTableNullErrorStub( + Assembler* assembler) { + __ EnterStubFrame(); + __ CallRuntime(kNullErrorRuntimeEntry, /*argument_count=*/0); + // The NullError runtime entry does not return. + __ Breakpoint(); +} + void StubCodeCompiler::GenerateNullErrorSharedWithoutFPURegsStub( Assembler* assembler) { GenerateSharedStub( diff --git a/runtime/vm/compiler/stub_code_compiler_arm64.cc b/runtime/vm/compiler/stub_code_compiler_arm64.cc index bf862d6fd3d..3faa3bbf6a7 100644 --- a/runtime/vm/compiler/stub_code_compiler_arm64.cc +++ b/runtime/vm/compiler/stub_code_compiler_arm64.cc @@ -519,6 +519,14 @@ void StubCodeCompiler::GenerateBuildMethodExtractorStub( __ Ret(); } +void StubCodeCompiler::GenerateDispatchTableNullErrorStub( + Assembler* assembler) { + __ EnterStubFrame(); + __ CallRuntime(kNullErrorRuntimeEntry, /*argument_count=*/0); + // The NullError runtime entry does not return. + __ Breakpoint(); +} + void StubCodeCompiler::GenerateNullErrorSharedWithoutFPURegsStub( Assembler* assembler) { GenerateSharedStub( diff --git a/runtime/vm/compiler/stub_code_compiler_ia32.cc b/runtime/vm/compiler/stub_code_compiler_ia32.cc index 6510cb519eb..f29b2db1890 100644 --- a/runtime/vm/compiler/stub_code_compiler_ia32.cc +++ b/runtime/vm/compiler/stub_code_compiler_ia32.cc @@ -287,6 +287,12 @@ void StubCodeCompiler::GenerateJITCallbackTrampolines( #endif } +void StubCodeCompiler::GenerateDispatchTableNullErrorStub( + Assembler* assembler) { + // Only used in AOT. + __ Breakpoint(); +} + void StubCodeCompiler::GenerateNullErrorSharedWithoutFPURegsStub( Assembler* assembler) { __ Breakpoint(); diff --git a/runtime/vm/compiler/stub_code_compiler_x64.cc b/runtime/vm/compiler/stub_code_compiler_x64.cc index a8cf49ddb9c..4febff1f3ba 100644 --- a/runtime/vm/compiler/stub_code_compiler_x64.cc +++ b/runtime/vm/compiler/stub_code_compiler_x64.cc @@ -453,6 +453,14 @@ void StubCodeCompiler::GenerateBuildMethodExtractorStub( __ Ret(); } +void StubCodeCompiler::GenerateDispatchTableNullErrorStub( + Assembler* assembler) { + __ EnterStubFrame(); + __ CallRuntime(kNullErrorRuntimeEntry, /*argument_count=*/0); + // The NullError runtime entry does not return. + __ Breakpoint(); +} + void StubCodeCompiler::GenerateNullErrorSharedWithoutFPURegsStub( Assembler* assembler) { GenerateSharedStub( diff --git a/runtime/vm/dispatch_table.cc b/runtime/vm/dispatch_table.cc index f766024502b..183ff297b56 100644 --- a/runtime/vm/dispatch_table.cc +++ b/runtime/vm/dispatch_table.cc @@ -7,6 +7,7 @@ #include "vm/clustered_snapshot.h" #include "vm/hash_map.h" #include "vm/object.h" +#include "vm/object_store.h" namespace dart { @@ -122,6 +123,10 @@ DispatchTable* DispatchTable::Deserialize(Deserializer* deserializer, Code& code = Code::Handle(); + code = + deserializer->isolate()->object_store()->dispatch_table_null_error_stub(); + uword null_entry = code.EntryPoint(); + uword value = 0; uword recent[kRecentCount] = {0}; intptr_t recent_index = 0; @@ -132,7 +137,7 @@ DispatchTable* DispatchTable::Deserialize(Deserializer* deserializer, } else { int32_t encoded = deserializer->Read(); if (encoded == 0) { - value = 0; + value = null_entry; } else if (encoded < 0) { intptr_t r = ~encoded; ASSERT(r < kRecentCount); diff --git a/runtime/vm/image_snapshot.cc b/runtime/vm/image_snapshot.cc index 6e04ec35c76..b8afa524147 100644 --- a/runtime/vm/image_snapshot.cc +++ b/runtime/vm/image_snapshot.cc @@ -512,6 +512,8 @@ static const char* NameOfStubIsolateSpecificStub(ObjectStore* object_store, const Code& code) { if (code.raw() == object_store->build_method_extractor_code()) { return "_iso_stub_BuildMethodExtractorStub"; + } else if (code.raw() == object_store->dispatch_table_null_error_stub()) { + return "_iso_stub_DispatchTableNullErrorStub"; } else if (code.raw() == object_store->null_error_stub_with_fpu_regs_stub()) { return "_iso_stub_NullErrorSharedWithFPURegsStub"; } else if (code.raw() == diff --git a/runtime/vm/object_store.h b/runtime/vm/object_store.h index 48ff955737f..bb47f4a65fe 100644 --- a/runtime/vm/object_store.h +++ b/runtime/vm/object_store.h @@ -168,6 +168,7 @@ class ObjectPointerVisitor; RW(Array, unique_dynamic_targets) \ RW(GrowableObjectArray, megamorphic_cache_table) \ RW(Code, build_method_extractor_code) \ + RW(Code, dispatch_table_null_error_stub) \ RW(Code, null_error_stub_with_fpu_regs_stub) \ RW(Code, null_error_stub_without_fpu_regs_stub) \ RW(Code, null_arg_error_stub_with_fpu_regs_stub) \ diff --git a/runtime/vm/stub_code_list.h b/runtime/vm/stub_code_list.h index 739443e6d8c..ce41ba0c743 100644 --- a/runtime/vm/stub_code_list.h +++ b/runtime/vm/stub_code_list.h @@ -71,6 +71,7 @@ namespace dart { V(CallClosureNoSuchMethod) \ V(FrameAwaitingMaterialization) \ V(AsynchronousGapMarker) \ + V(DispatchTableNullError) \ V(NullErrorSharedWithFPURegs) \ V(NullErrorSharedWithoutFPURegs) \ V(NullArgErrorSharedWithFPURegs) \