Skip to content

Commit 62325f9

Browse files
creliercommit-bot@chromium.org
crelier
authored and
[email protected]
committed
[vm/bytecode] Collect more bytecode token positions for a given script.
Refactor Kernel::CollectTokenPositionsFor, but do not try to factorize code for fields and functions, as too many handles would need to be passed around. Bytecode reading creates duplicate script objects requiring scripts to be matched by their url rather than by their raw address when collecting token positions in bytecode. However, url comparison is not yet used, because collecting token positions in the duplicated script 'dart:core/map.dart' (in default mode) causes crashes in kernel reading. Will revisit. Fix service test valid_source locations_test.dart and make sure fields are properly reloaded, except fields injected by fasta. Remove unused argument 'record' of KernelReaderHelper::ReadPosition(). Handle bytecode stub frame in IsAsyncMachinery() tester in debugger. Change-Id: Ifbddcaec00e0696f7de13c5cf1e74380b31d2419 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112402 Commit-Queue: Régis Crelier <[email protected]> Reviewed-by: Alexander Markov <[email protected]>
1 parent f7a300f commit 62325f9

File tree

6 files changed

+55
-45
lines changed

6 files changed

+55
-45
lines changed

‎runtime/observatory/tests/service/valid_source_locations_test.dart‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,23 @@ void testFunction() {
1414
debugger();
1515
}
1616

17-
Future validateLocation(Location location) async {
17+
Future validateLocation(Location location, Object object) async {
1818
if (location == null) return;
1919
if (location.tokenPos < 0) return;
20+
if (location.script.uri == 'dart:_internal-patch/class_id_fasta.dart') {
21+
// Injected fields from this script cannot be reloaded.
22+
return;
23+
}
2024

2125
// Ensure the script is loaded.
2226
final Script script = await location.script.load();
2327

2428
// Use the more low-level functions.
25-
script.getLine(script.tokenToLine(location.tokenPos));
29+
final line = script.tokenToLine(location.tokenPos);
30+
if (line == null) {
31+
throw 'missing location for $object in script ${script.uri}';
32+
}
33+
script.getLine(line);
2634
script.tokenToCol(location.tokenPos);
2735

2836
// Use the helper functions.
@@ -31,19 +39,18 @@ Future validateLocation(Location location) async {
3139
}
3240

3341
Future validateFieldLocation(Field field) async {
34-
// TODO(http://dartbug.com/32503): We should `field = await field.load()`
35-
// here, but it causes all kinds of strong-mode errors.
36-
await validateLocation(field.location);
42+
field = await field.load();
43+
await validateLocation(field.location, field);
3744
}
3845

3946
Future validateFunctionLocation(ServiceFunction fun) async {
4047
fun = await fun.load();
41-
await validateLocation(fun.location);
48+
await validateLocation(fun.location, fun);
4249
}
4350

4451
Future validateClassLocation(Class klass) async {
4552
klass = await klass.load();
46-
await validateLocation(klass.location);
53+
await validateLocation(klass.location, klass);
4754

4855
for (Field field in klass.fields) {
4956
await validateFieldLocation(field);

‎runtime/vm/compiler/frontend/kernel_translation_helper.cc‎

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -994,13 +994,11 @@ void FieldHelper::ReadUntilExcluding(Field field) {
994994
if (++next_read_ == field) return;
995995
FALL_THROUGH;
996996
case kPosition:
997-
position_ = helper_->ReadPosition(false); // read position.
998-
helper_->RecordTokenPosition(position_);
997+
position_ = helper_->ReadPosition(); // read position.
999998
if (++next_read_ == field) return;
1000999
FALL_THROUGH;
10011000
case kEndPosition:
1002-
end_position_ = helper_->ReadPosition(false); // read end position.
1003-
helper_->RecordTokenPosition(end_position_);
1001+
end_position_ = helper_->ReadPosition(); // read end position.
10041002
if (++next_read_ == field) return;
10051003
FALL_THROUGH;
10061004
case kFlags:
@@ -1056,18 +1054,15 @@ void ProcedureHelper::ReadUntilExcluding(Field field) {
10561054
if (++next_read_ == field) return;
10571055
FALL_THROUGH;
10581056
case kStartPosition:
1059-
start_position_ = helper_->ReadPosition(false); // read position.
1060-
helper_->RecordTokenPosition(start_position_);
1057+
start_position_ = helper_->ReadPosition(); // read position.
10611058
if (++next_read_ == field) return;
10621059
FALL_THROUGH;
10631060
case kPosition:
1064-
position_ = helper_->ReadPosition(false); // read position.
1065-
helper_->RecordTokenPosition(position_);
1061+
position_ = helper_->ReadPosition(); // read position.
10661062
if (++next_read_ == field) return;
10671063
FALL_THROUGH;
10681064
case kEndPosition:
1069-
end_position_ = helper_->ReadPosition(false); // read end position.
1070-
helper_->RecordTokenPosition(end_position_);
1065+
end_position_ = helper_->ReadPosition(); // read end position.
10711066
if (++next_read_ == field) return;
10721067
FALL_THROUGH;
10731068
case kKind:
@@ -1132,17 +1127,14 @@ void ConstructorHelper::ReadUntilExcluding(Field field) {
11321127
FALL_THROUGH;
11331128
case kStartPosition:
11341129
start_position_ = helper_->ReadPosition(); // read position.
1135-
helper_->RecordTokenPosition(start_position_);
11361130
if (++next_read_ == field) return;
11371131
FALL_THROUGH;
11381132
case kPosition:
11391133
position_ = helper_->ReadPosition(); // read position.
1140-
helper_->RecordTokenPosition(position_);
11411134
if (++next_read_ == field) return;
11421135
FALL_THROUGH;
11431136
case kEndPosition:
11441137
end_position_ = helper_->ReadPosition(); // read end position.
1145-
helper_->RecordTokenPosition(end_position_);
11461138
if (++next_read_ == field) return;
11471139
FALL_THROUGH;
11481140
case kFlags:
@@ -1201,18 +1193,15 @@ void ClassHelper::ReadUntilExcluding(Field field) {
12011193
if (++next_read_ == field) return;
12021194
FALL_THROUGH;
12031195
case kStartPosition:
1204-
start_position_ = helper_->ReadPosition(false); // read position.
1205-
helper_->RecordTokenPosition(start_position_);
1196+
start_position_ = helper_->ReadPosition(); // read position.
12061197
if (++next_read_ == field) return;
12071198
FALL_THROUGH;
12081199
case kPosition:
1209-
position_ = helper_->ReadPosition(false); // read position.
1210-
helper_->RecordTokenPosition(position_);
1200+
position_ = helper_->ReadPosition(); // read position.
12111201
if (++next_read_ == field) return;
12121202
FALL_THROUGH;
12131203
case kEndPosition:
12141204
end_position_ = helper_->ReadPosition(); // read end position.
1215-
helper_->RecordTokenPosition(end_position_);
12161205
if (++next_read_ == field) return;
12171206
FALL_THROUGH;
12181207
case kFlags:
@@ -2560,11 +2549,9 @@ void KernelReaderHelper::SkipLibraryTypedef() {
25602549
SkipListOfVariableDeclarations(); // read named parameters.
25612550
}
25622551

2563-
TokenPosition KernelReaderHelper::ReadPosition(bool record) {
2552+
TokenPosition KernelReaderHelper::ReadPosition() {
25642553
TokenPosition position = reader_.ReadPosition();
2565-
if (record) {
2566-
RecordTokenPosition(position);
2567-
}
2554+
RecordTokenPosition(position);
25682555
return position;
25692556
}
25702557

‎runtime/vm/compiler/frontend/kernel_translation_helper.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,7 @@ class KernelReaderHelper {
10791079
void SkipLibraryDependency();
10801080
void SkipLibraryPart();
10811081
void SkipLibraryTypedef();
1082-
TokenPosition ReadPosition(bool record = true);
1082+
TokenPosition ReadPosition();
10831083
Tag ReadTag(uint8_t* payload = NULL);
10841084
uint8_t ReadFlags() { return reader_.ReadFlags(); }
10851085
Nullability ReadNullability();

‎runtime/vm/debugger.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,10 @@ void ActivationFrame::ExtractTokenPositionFromAsyncClosure() {
10981098
}
10991099

11001100
bool ActivationFrame::IsAsyncMachinery() const {
1101+
if (function_.IsNull()) {
1102+
ASSERT(IsInterpreted()); // This frame is a bytecode stub frame.
1103+
return false;
1104+
}
11011105
Isolate* isolate = Isolate::Current();
11021106
if (function_.raw() == isolate->object_store()->complete_on_async_return()) {
11031107
// We are completing an async function's completer.

‎runtime/vm/kernel.cc‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -260,16 +260,20 @@ static void CollectKernelDataTokenPositions(
260260
token_position_collector.CollectTokenPositions(kernel_offset);
261261
}
262262

263-
static void CollectBytecodeTokenPositions(
263+
static void CollectTokenPosition(TokenPosition position,
264+
GrowableArray<intptr_t>* token_positions) {
265+
if (position.IsReal()) {
266+
token_positions->Add(position.value());
267+
}
268+
}
269+
270+
static void CollectBytecodeSourceTokenPositions(
264271
const Bytecode& bytecode,
265272
Zone* zone,
266273
GrowableArray<intptr_t>* token_positions) {
267274
BytecodeSourcePositionsIterator iter(zone, bytecode);
268275
while (iter.MoveNext()) {
269-
const TokenPosition pos = iter.TokenPos();
270-
if (pos.IsReal()) {
271-
token_positions->Add(pos.value());
272-
}
276+
CollectTokenPosition(iter.TokenPos(), token_positions);
273277
}
274278
}
275279

@@ -279,6 +283,8 @@ static void CollectBytecodeFunctionTokenPositions(
279283
Thread* thread = Thread::Current();
280284
Zone* zone = thread->zone();
281285
ASSERT(function.is_declared_in_bytecode());
286+
CollectTokenPosition(function.token_pos(), token_positions);
287+
CollectTokenPosition(function.end_token_pos(), token_positions);
282288
if (!function.HasBytecode()) {
283289
const Object& result = Object::Handle(
284290
zone, BytecodeReader::ReadFunctionBytecode(thread, function));
@@ -291,7 +297,7 @@ static void CollectBytecodeFunctionTokenPositions(
291297
return;
292298
}
293299
if (bytecode.HasSourcePositions() && !function.IsLocalFunction()) {
294-
CollectBytecodeTokenPositions(bytecode, zone, token_positions);
300+
CollectBytecodeSourceTokenPositions(bytecode, zone, token_positions);
295301
// Find closure functions in the object pool.
296302
const ObjectPool& pool = ObjectPool::Handle(zone, bytecode.object_pool());
297303
Object& object = Object::Handle(zone);
@@ -306,18 +312,21 @@ static void CollectBytecodeFunctionTokenPositions(
306312
closure ^= object.raw();
307313
if (closure.kind() == RawFunction::kClosureFunction &&
308314
closure.IsLocalFunction()) {
315+
CollectTokenPosition(closure.token_pos(), token_positions);
316+
CollectTokenPosition(closure.end_token_pos(), token_positions);
309317
bytecode = closure.bytecode();
310318
ASSERT(!bytecode.IsNull());
311-
if (bytecode.HasSourcePositions()) {
312-
CollectBytecodeTokenPositions(bytecode, zone, token_positions);
313-
}
319+
ASSERT(bytecode.function() != Function::null());
320+
ASSERT(bytecode.HasSourcePositions());
321+
CollectBytecodeSourceTokenPositions(bytecode, zone, token_positions);
314322
}
315323
}
316324
}
317325
}
318326
}
319327

320328
void CollectTokenPositionsFor(const Script& interesting_script) {
329+
ASSERT(interesting_script.url() != String::null());
321330
Thread* thread = Thread::Current();
322331
Zone* zone = thread->zone();
323332
TranslationHelper helper(thread);
@@ -348,6 +357,7 @@ void CollectTokenPositionsFor(const Script& interesting_script) {
348357
const Class& klass = Class::Cast(entry);
349358
if (klass.script() == interesting_script.raw()) {
350359
token_positions.Add(klass.token_pos().value());
360+
token_positions.Add(klass.end_token_pos().value());
351361
}
352362
// If class is declared in bytecode, its members should be loaded
353363
// (via class finalization) before their token positions could be
@@ -363,7 +373,6 @@ void CollectTokenPositionsFor(const Script& interesting_script) {
363373
temp_array = klass.fields();
364374
for (intptr_t i = 0; i < temp_array.Length(); ++i) {
365375
temp_field ^= temp_array.At(i);
366-
// TODO(regis): Factorize field handling code.
367376
if (!temp_field.is_declared_in_bytecode() &&
368377
temp_field.kernel_offset() <= 0) {
369378
// Skip artificially injected fields.
@@ -391,7 +400,6 @@ void CollectTokenPositionsFor(const Script& interesting_script) {
391400
temp_array = klass.functions();
392401
for (intptr_t i = 0; i < temp_array.Length(); ++i) {
393402
temp_function ^= temp_array.At(i);
394-
// TODO(regis): Factorize function handling code.
395403
entry_script = temp_function.script();
396404
if (entry_script.raw() != interesting_script.raw()) {
397405
continue;
@@ -429,7 +437,6 @@ void CollectTokenPositionsFor(const Script& interesting_script) {
429437
}
430438
} else if (entry.IsFunction()) {
431439
temp_function ^= entry.raw();
432-
// TODO(regis): Factorize function handling code.
433440
entry_script = temp_function.script();
434441
if (entry_script.raw() != interesting_script.raw()) {
435442
continue;
@@ -447,7 +454,6 @@ void CollectTokenPositionsFor(const Script& interesting_script) {
447454
}
448455
} else if (entry.IsField()) {
449456
const Field& field = Field::Cast(entry);
450-
// TODO(regis): Factorize field handling code.
451457
if (!field.is_declared_in_bytecode() && field.kernel_offset() <= 0) {
452458
// Skip artificially injected fields.
453459
continue;

‎runtime/vm/source_report.cc‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,12 @@ void SourceReport::VisitFunction(JSONArray* jsarr, const Function& func) {
473473
Code& code = Code::Handle(zone(), func.unoptimized_code());
474474
Bytecode& bytecode = Bytecode::Handle(zone());
475475
#if !defined(DART_PRECOMPILED_RUNTIME)
476-
if (FLAG_enable_interpreter && code.IsNull() && func.HasBytecode()) {
476+
if (FLAG_enable_interpreter && !func.HasCode() && func.HasBytecode()) {
477+
// When the bytecode of a function is loaded, the function code is not null,
478+
// but pointing to the stub to interpret the bytecode. The various Print
479+
// functions below take code as an argument and know to process the bytecode
480+
// if code is null.
481+
code = Code::null(); // Ignore installed stub to interpret bytecode.
477482
bytecode = func.bytecode();
478483
}
479484
#endif // !defined(DART_PRECOMPILED_RUNTIME)
@@ -493,7 +498,8 @@ void SourceReport::VisitFunction(JSONArray* jsarr, const Function& func) {
493498
}
494499
code = func.unoptimized_code();
495500
#if !defined(DART_PRECOMPILED_RUNTIME)
496-
if (FLAG_enable_interpreter && code.IsNull() && func.HasBytecode()) {
501+
if (FLAG_enable_interpreter && !func.HasCode() && func.HasBytecode()) {
502+
code = Code::null(); // Ignore installed stub to interpret bytecode.
497503
bytecode = func.bytecode();
498504
}
499505
#endif // !defined(DART_PRECOMPILED_RUNTIME)

0 commit comments

Comments
 (0)