Skip to content

Commit 6a7d4e2

Browse files
askeksacommit-bot@chromium.org
askeksa
authored and
commit-bot@chromium.org
committed
[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>
1 parent 27e64c3 commit 6a7d4e2

20 files changed

Lines changed: 124 additions & 41 deletions

‎pkg/vm/lib/metadata/table_selector.dart‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,23 @@ import 'procedure_attributes.dart';
88

99
// Information associated with a selector, used by the dispatch table generator.
1010
class TableSelectorInfo {
11+
static const int kCalledOnNullBit = 1 << 0;
12+
static const int kCallCountShift = 1;
13+
1114
int callCount;
15+
bool calledOnNull;
1216

13-
TableSelectorInfo() : callCount = 0;
17+
TableSelectorInfo()
18+
: callCount = 0,
19+
calledOnNull = false;
1420

1521
TableSelectorInfo.readFromBinary(BinarySource source)
16-
: callCount = source.readUInt();
22+
: callCount = source.readUInt(),
23+
calledOnNull = source.readByte() != 0;
1724

1825
void writeToBinary(BinarySink sink) {
1926
sink.writeUInt30(callCount);
27+
sink.writeByte(calledOnNull ? 1 : 0);
2028
}
2129
}
2230

‎pkg/vm/lib/transformations/type_flow/table_selector.dart‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,16 @@ class TableSelectorAssigner {
6060
throw "Unexpected member kind '${member.runtimeType}'";
6161
}
6262

63-
void registerCall(int selectorId) {
63+
void registerCall(int selectorId, bool calledOnNull) {
6464
metadata.selectors[selectorId].callCount++;
65+
metadata.selectors[selectorId].calledOnNull |= calledOnNull;
6566
}
6667

67-
void registerMethodOrSetterCall(Member member) {
68-
registerCall(methodOrSetterSelectorId(member));
68+
void registerMethodOrSetterCall(Member member, bool calledOnNull) {
69+
registerCall(methodOrSetterSelectorId(member), calledOnNull);
6970
}
7071

71-
void registerGetterCall(Member member) {
72-
registerCall(getterSelectorId(member));
72+
void registerGetterCall(Member member, bool calledOnNull) {
73+
registerCall(getterSelectorId(member), calledOnNull);
7374
}
7475
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,10 +263,12 @@ class AnnotateKernel extends RecursiveVisitor<Null> {
263263
final Selector selector = callSite.selector;
264264
if (selector is InterfaceSelector && !_callSiteUsesDirectCall(node)) {
265265
if (node is PropertyGet) {
266-
_tableSelectorAssigner.registerGetterCall(selector.member);
266+
_tableSelectorAssigner.registerGetterCall(
267+
selector.member, callSite.isNullableReceiver);
267268
} else {
268269
assertx(node is MethodInvocation || node is PropertySet);
269-
_tableSelectorAssigner.registerMethodOrSetterCall(selector.member);
270+
_tableSelectorAssigner.registerMethodOrSetterCall(
271+
selector.member, callSite.isNullableReceiver);
270272
}
271273
}
272274
}

‎runtime/vm/compiler/aot/aot_call_specializer.cc‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,12 +1303,6 @@ void AotCallSpecializer::TryReplaceWithDispatchTableCall(
13031303
return;
13041304
}
13051305

1306-
if (!selector->on_null_interface) {
1307-
// Selector not implemented by Null. Add null check if receiver is nullable.
1308-
AddCheckNull(receiver->CopyWithType(Z), call->function_name(),
1309-
DeoptId::kNone, call->env(), call);
1310-
}
1311-
13121306
const AbstractType& target_type =
13131307
AbstractType::Handle(Class::Handle(interface_target.Owner()).RareType());
13141308
const bool receiver_can_be_smi =

‎runtime/vm/compiler/aot/dispatch_table_generator.cc‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ class SelectorRow {
109109
void DefineSelectorImplementationForInterval(classid_t cid,
110110
int16_t depth,
111111
const Interval& range,
112-
const Function& function);
112+
const Function* function);
113113
bool Finalize();
114114

115115
int32_t CallCount() const { return selector_->call_count; }
@@ -168,8 +168,8 @@ void SelectorRow::DefineSelectorImplementationForInterval(
168168
classid_t cid,
169169
int16_t depth,
170170
const Interval& range,
171-
const Function& function) {
172-
CidInterval cid_range(cid, depth, range, &function);
171+
const Function* function) {
172+
CidInterval cid_range(cid, depth, range, function);
173173
class_ranges_.Add(cid_range);
174174
}
175175

@@ -251,7 +251,7 @@ void SelectorRow::FillTable(ClassTable* class_table, DispatchTable* table) {
251251
const CidInterval& cid_range = class_ranges_[i];
252252
const Interval& range = cid_range.range();
253253
const Function* function = cid_range.function();
254-
if (function->HasCode()) {
254+
if (function != nullptr && function->HasCode()) {
255255
code = function->CurrentCode();
256256
for (classid_t cid = range.begin(); cid < range.end(); cid++) {
257257
table->SetCodeAt(selector()->offset + cid, code);
@@ -378,9 +378,10 @@ const TableSelector* SelectorMap::GetSelector(
378378
return selector;
379379
}
380380

381-
void SelectorMap::AddSelector(int32_t call_count) {
381+
void SelectorMap::AddSelector(int32_t call_count, bool called_on_null) {
382382
const int32_t added_sid = selectors_.length();
383-
selectors_.Add(TableSelector(added_sid, call_count, kInvalidSelectorOffset));
383+
selectors_.Add(TableSelector(added_sid, call_count, kInvalidSelectorOffset,
384+
called_on_null));
384385
}
385386

386387
void SelectorMap::SetSelectorProperties(int32_t sid,
@@ -417,7 +418,7 @@ void DispatchTableGenerator::ReadTableSelectorInfo() {
417418
RELEASE_ASSERT(metadata != nullptr);
418419
for (intptr_t i = 0; i < metadata->selectors.length(); i++) {
419420
const kernel::TableSelectorInfo* info = &metadata->selectors[i];
420-
selector_map_.AddSelector(info->call_count);
421+
selector_map_.AddSelector(info->call_count, info->called_on_null);
421422
}
422423
}
423424

@@ -533,7 +534,12 @@ void DispatchTableGenerator::SetupSelectorRows() {
533534
// Initialize selector rows.
534535
SelectorRow* selector_rows = Z->Alloc<SelectorRow>(num_selectors_);
535536
for (intptr_t i = 0; i < num_selectors_; i++) {
536-
new (&selector_rows[i]) SelectorRow(Z, &selector_map_.selectors_[i]);
537+
TableSelector* selector = &selector_map_.selectors_[i];
538+
new (&selector_rows[i]) SelectorRow(Z, selector);
539+
if (selector->called_on_null && !selector->on_null_interface) {
540+
selector_rows[i].DefineSelectorImplementationForInterval(
541+
kNullCid, 0, Interval(kNullCid, kNullCid + 1), nullptr);
542+
}
537543
}
538544

539545
// Add implementation intervals to the selector rows for all classes that
@@ -559,7 +565,7 @@ void DispatchTableGenerator::SetupSelectorRows() {
559565
for (intptr_t i = 0; i < subclasss_cid_ranges.length(); i++) {
560566
Interval& subclass_cid_range = subclasss_cid_ranges[i];
561567
selector_rows[sid].DefineSelectorImplementationForInterval(
562-
cid, depth, subclass_cid_range, function_handle);
568+
cid, depth, subclass_cid_range, &function_handle);
563569
}
564570
}
565571
}

‎runtime/vm/compiler/aot/dispatch_table_generator.h‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,29 @@ namespace compiler {
1919
class SelectorRow;
2020

2121
struct TableSelector {
22-
TableSelector(int32_t id, int32_t call_count, int32_t offset)
23-
: id(id), call_count(call_count), offset(offset) {}
22+
TableSelector(int32_t _id,
23+
int32_t _call_count,
24+
int32_t _offset,
25+
bool _called_on_null)
26+
: id(_id),
27+
call_count(_call_count),
28+
offset(_offset),
29+
called_on_null(_called_on_null) {}
2430

2531
bool IsUsed() const { return call_count > 0; }
2632

33+
// ID assigned to the selector.
2734
int32_t id;
35+
// Number of dispatch table call sites with this selector (conservative:
36+
// number may be bigger, but not smaller, than actual number of call sites).
2837
int32_t call_count;
38+
// Table offset assigned to the selector by the dispatch table generator.
2939
int32_t offset;
40+
// Are there any call sites with this selector where the receiver may be null?
41+
bool called_on_null;
42+
// Is the selector part of the interface on Null (same as Object)?
3043
bool on_null_interface = false;
44+
// Do any targets of this selector assume that an args descriptor is passed?
3145
bool requires_args_descriptor = false;
3246
};
3347

@@ -46,7 +60,7 @@ class SelectorMap {
4660

4761
int32_t SelectorId(const Function& interface_target) const;
4862

49-
void AddSelector(int32_t call_count);
63+
void AddSelector(int32_t call_count, bool called_on_null);
5064
void SetSelectorProperties(int32_t sid,
5165
bool on_null_interface,
5266
bool requires_args_descriptor);

‎runtime/vm/compiler/aot/precompiler.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,10 @@ void Precompiler::DoCompileAll() {
292292
StubCode::GetBuildMethodExtractorStub(global_object_pool_builder());
293293
I->object_store()->set_build_method_extractor_code(stub_code);
294294

295+
stub_code = StubCode::BuildIsolateSpecificDispatchTableNullErrorStub(
296+
global_object_pool_builder());
297+
I->object_store()->set_dispatch_table_null_error_stub(stub_code);
298+
295299
stub_code =
296300
StubCode::BuildIsolateSpecificNullErrorSharedWithFPURegsStub(
297301
global_object_pool_builder());

‎runtime/vm/compiler/backend/flow_graph_compiler.cc‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -745,11 +745,18 @@ void FlowGraphCompiler::AddCurrentDescriptor(RawPcDescriptors::Kind kind,
745745
CurrentTryIndex());
746746
}
747747

748-
void FlowGraphCompiler::AddNullCheck(intptr_t pc_offset,
749-
TokenPosition token_pos,
750-
intptr_t null_check_name_idx) {
751-
code_source_map_builder_->NoteNullCheck(pc_offset, token_pos,
752-
null_check_name_idx);
748+
void FlowGraphCompiler::AddNullCheck(TokenPosition token_pos,
749+
const String& name) {
750+
// If we have DWARF stack traces enabled, the AOT runtime is unable to obtain
751+
// the pool index at runtime. There is therefore no reason to put the name
752+
// into the pool in the first place.
753+
// TODO(dartbug.com/40605): Move this info to the pc descriptors.
754+
if (!FLAG_dwarf_stack_traces) {
755+
const intptr_t name_index =
756+
assembler()->object_pool_builder().FindObject(name);
757+
code_source_map_builder_->NoteNullCheck(assembler()->CodeSize(), token_pos,
758+
name_index);
759+
}
753760
}
754761

755762
void FlowGraphCompiler::AddPcRelativeCallTarget(const Function& function,

‎runtime/vm/compiler/backend/flow_graph_compiler.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -819,9 +819,8 @@ class FlowGraphCompiler : public ValueObject {
819819
intptr_t try_index,
820820
intptr_t yield_index = RawPcDescriptors::kInvalidYieldIndex);
821821

822-
void AddNullCheck(intptr_t pc_offset,
823-
TokenPosition token_pos,
824-
intptr_t null_check_name_idx);
822+
// Add NullCheck information for the current PC.
823+
void AddNullCheck(TokenPosition token_pos, const String& name);
825824

826825
void RecordSafepoint(LocationSummary* locs,
827826
intptr_t slow_path_argument_count = 0);

‎runtime/vm/compiler/backend/il.cc‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4457,6 +4457,14 @@ void DispatchTableCallInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
44574457
arguments_descriptor);
44584458
compiler->EmitCallsiteMetadata(token_pos(), DeoptId::kNone,
44594459
RawPcDescriptors::kOther, locs());
4460+
if (selector()->called_on_null && !selector()->on_null_interface) {
4461+
Value* receiver = ArgumentValueAt(FirstArgIndex());
4462+
if (receiver->Type()->is_nullable()) {
4463+
const String& function_name =
4464+
String::ZoneHandle(interface_target().name());
4465+
compiler->AddNullCheck(token_pos(), function_name);
4466+
}
4467+
}
44604468
__ Drop(ArgumentCount());
44614469

44624470
compiler->AddDispatchTableCallTarget(selector());
@@ -4862,11 +4870,7 @@ LocationSummary* CheckNullInstr::MakeLocationSummary(Zone* zone,
48624870

48634871
void CheckNullInstr::AddMetadataForRuntimeCall(CheckNullInstr* check_null,
48644872
FlowGraphCompiler* compiler) {
4865-
const String& function_name = check_null->function_name();
4866-
const intptr_t name_index =
4867-
compiler->assembler()->object_pool_builder().FindObject(function_name);
4868-
compiler->AddNullCheck(compiler->assembler()->CodeSize(),
4869-
check_null->token_pos(), name_index);
4873+
compiler->AddNullCheck(check_null->token_pos(), check_null->function_name());
48704874
}
48714875

48724876
void UnboxInstr::EmitLoadFromBoxWithDeopt(FlowGraphCompiler* compiler) {

0 commit comments

Comments
 (0)