[vm/aot] Include entries for null in the dispatch table to avoid check.

NullCheck pc descriptors are added to all dispatch table calls where the
receiver may be null (and the selector is not one implemented by null).

All null entries in the table go to the NullError runtime entry, which
reads the NullCheck pc descriptor to get the name of the called member
for the error message.

Change-Id: I9d2847d0ccdfdb735b06e879916920ec299f39bc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134294
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

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);
   }
 }
 

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);
   }
 }

pkg/vm/lib/transformations/type_flow/transformer.dart

@@ -263,10 +263,12 @@ class AnnotateKernel extends RecursiveVisitor<Null> {
     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);
       }
     }
   }

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 =

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<SelectorRow>(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);
               }
             }
           }

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);

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());

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,

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);

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) {

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 {

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);

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(

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(

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();

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(

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<uint32_t>();
       if (encoded == 0) {
-        value = 0;
+        value = null_entry;
       } else if (encoded < 0) {
         intptr_t r = ~encoded;
         ASSERT(r < kRecentCount);

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() ==

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)                             \

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)                                             \