[vm/bytecode] Add yield point markers to source positions

Issue: https://github.com/dart-lang/sdk/issues/36427
Change-Id: I384161fd27b977e05796666c9a1a1e336d4d6440
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107572
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>

pkg/vm/lib/bytecode/assembler.dart

@@ -79,6 +79,12 @@ class BytecodeAssembler {
     }
   }
 
+  void emitYieldPointSourcePosition() {
+    if (!isUnreachable) {
+      sourcePositions.addYieldPoint(offset, currentSourcePosition);
+    }
+  }
+
   void _emitByte(int abyte) {
     assert(_isUint8(abyte));
     bytecode.add(abyte);

pkg/vm/lib/bytecode/declarations.dart

@@ -760,7 +760,7 @@ class Code {
 
   bool get hasExceptionsTable => exceptionsTable.blocks.isNotEmpty;
   bool get hasSourcePositions =>
-      sourcePositions != null && sourcePositions.mapping.isNotEmpty;
+      sourcePositions != null && sourcePositions.isNotEmpty;
   bool get hasLocalVariables =>
       localVariables != null && localVariables.isNotEmpty;
   bool get hasNullableFields => nullableFields.isNotEmpty;
@@ -1056,7 +1056,7 @@ class ClosureCode {
 
   bool get hasExceptionsTable => exceptionsTable.blocks.isNotEmpty;
   bool get hasSourcePositions =>
-      sourcePositions != null && sourcePositions.mapping.isNotEmpty;
+      sourcePositions != null && sourcePositions.isNotEmpty;
   bool get hasLocalVariables =>
       localVariables != null && localVariables.isNotEmpty;
 

pkg/vm/lib/bytecode/gen_bytecode.dart

@@ -1485,7 +1485,7 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
   }
 
   SourcePositions finalizeSourcePositions() {
-    if (asm.sourcePositions.mapping.isEmpty) {
+    if (asm.sourcePositions.isEmpty) {
       return null;
     }
     bytecodeComponent.sourcePositions.add(asm.sourcePositions);
@@ -3692,6 +3692,10 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
       return;
     }
 
+    if (options.emitSourcePositions) {
+      asm.emitYieldPointSourcePosition();
+    }
+
     // 0 is reserved for normal entry, yield points are counted from 1.
     final int yieldIndex = yieldPoints.length + 1;
     final Label continuationLabel = new Label(allowsBackwardJumps: true);

pkg/vm/lib/bytecode/source_positions.dart

@@ -15,7 +15,10 @@ import 'bytecode_serialization.dart'
 
 /// Maintains mapping between bytecode instructions and source positions.
 class SourcePositions {
-  final Map<int, int> mapping = <int, int>{}; // PC -> fileOffset
+  // Special value of fileOffset which marks yield point.
+  static const yieldPointMarker = -2;
+
+  final List<int> _positions = <int>[]; // Pairs (PC, fileOffset).
   int _lastPc = 0;
   int _lastOffset = 0;
 
@@ -25,20 +28,37 @@ class SourcePositions {
     assert(pc > _lastPc);
     assert(fileOffset >= 0);
     if (fileOffset != _lastOffset) {
-      mapping[pc] = fileOffset;
+      _positions.add(pc);
+      _positions.add(fileOffset);
       _lastPc = pc;
       _lastOffset = fileOffset;
     }
   }
 
+  void addYieldPoint(int pc, int fileOffset) {
+    assert(pc > _lastPc);
+    assert(fileOffset >= 0);
+    _positions.add(pc);
+    _positions.add(yieldPointMarker);
+    _positions.add(pc);
+    _positions.add(fileOffset);
+    _lastPc = pc;
+    _lastOffset = fileOffset;
+  }
+
+  bool get isEmpty => _positions.isEmpty;
+  bool get isNotEmpty => !isEmpty;
+
   void write(BufferedWriter writer) {
-    writer.writePackedUInt30(mapping.length);
+    writer.writePackedUInt30(_positions.length ~/ 2);
     final encodePC = new PackedUInt30DeltaEncoder();
     final encodeOffset = new SLEB128DeltaEncoder();
-    mapping.forEach((int pc, int fileOffset) {
+    for (int i = 0; i < _positions.length; i += 2) {
+      final int pc = _positions[i];
+      final int fileOffset = _positions[i + 1];
       encodePC.write(writer, pc);
       encodeOffset.write(writer, fileOffset);
-    });
+    }
   }
 
   SourcePositions.read(BufferedReader reader) {
@@ -48,16 +68,29 @@ class SourcePositions {
     for (int i = 0; i < length; ++i) {
       int pc = decodePC.read(reader);
       int fileOffset = decodeOffset.read(reader);
-      add(pc, fileOffset);
+      _positions.add(pc);
+      _positions.add(fileOffset);
     }
   }
 
   @override
-  String toString() => mapping.toString();
+  String toString() => _positions.toString();
 
   Map<int, String> getBytecodeAnnotations() {
-    return mapping.map((int pc, int fileOffset) =>
-        new MapEntry(pc, 'source position $fileOffset'));
+    final map = <int, String>{};
+    for (int i = 0; i < _positions.length; i += 2) {
+      final int pc = _positions[i];
+      final int fileOffset = _positions[i + 1];
+      final entry = (fileOffset == yieldPointMarker)
+          ? 'yield point'
+          : 'source position $fileOffset';
+      if (map[pc] == null) {
+        map[pc] = entry;
+      } else {
+        map[pc] = "${map[pc]}; $entry";
+      }
+    }
+    return map;
   }
 }
 

runtime/vm/compiler/frontend/bytecode_reader.h

@@ -331,6 +331,10 @@ class BytecodeReader : public AllStatic {
 
 class BytecodeSourcePositionsIterator : ValueObject {
  public:
+  // This constant should match corresponding constant in class SourcePositions
+  // (pkg/vm/lib/bytecode/source_positions.dart).
+  static const intptr_t kYieldPointMarker = -2;
+
   BytecodeSourcePositionsIterator(Zone* zone, const Bytecode& bytecode)
       : reader_(ExternalTypedData::Handle(zone, bytecode.GetBinary(zone))) {
     if (bytecode.HasSourcePositions()) {
@@ -347,6 +351,12 @@ class BytecodeSourcePositionsIterator : ValueObject {
     --pairs_remaining_;
     cur_bci_ += reader_.ReadUInt();
     cur_token_pos_ += reader_.ReadSLEB128();
+    is_yield_point_ = false;
+    if (cur_token_pos_ == kYieldPointMarker) {
+      const bool result = MoveNext();
+      is_yield_point_ = true;
+      return result;
+    }
     return true;
   }
 
@@ -354,11 +364,14 @@ class BytecodeSourcePositionsIterator : ValueObject {
 
   TokenPosition TokenPos() const { return TokenPosition(cur_token_pos_); }
 
+  bool IsYieldPoint() const { return is_yield_point_; }
+
  private:
   Reader reader_;
   intptr_t pairs_remaining_ = 0;
   intptr_t cur_bci_ = 0;
   intptr_t cur_token_pos_ = 0;
+  bool is_yield_point_ = false;
 };
 
 #if !defined(PRODUCT)

runtime/vm/debugger.cc

@@ -948,23 +948,7 @@ bool ActivationFrame::HandlesException(const Instance& exc_obj) {
   return false;
 }
 
-void ActivationFrame::ExtractTokenPositionFromAsyncClosure() {
-  // Attempt to determine the token pos and try index from the async closure.
-  Thread* thread = Thread::Current();
-  Zone* zone = thread->zone();
-  const Script& script = Script::Handle(zone, function().script());
-
-  ASSERT(function_.IsAsyncGenClosure() || function_.IsAsyncClosure());
-  // This should only be called on frames that aren't active on the stack.
-  ASSERT(fp() == 0);
-
-  ASSERT(script.kind() == RawScript::kKernelTag);
-  const Array& await_to_token_map =
-      Array::Handle(zone, script.yield_positions());
-  if (await_to_token_map.IsNull()) {
-    // No mapping.
-    return;
-  }
+intptr_t ActivationFrame::GetAwaitJumpVariable() {
   GetVarDescriptors();
   intptr_t var_desc_len = var_descriptors_.Length();
   intptr_t await_jump_var = -1;
@@ -980,6 +964,59 @@ void ActivationFrame::ExtractTokenPositionFromAsyncClosure() {
       await_jump_var = Smi::Cast(await_jump_index).Value();
     }
   }
+  return await_jump_var;
+}
+
+void ActivationFrame::ExtractTokenPositionFromAsyncClosure() {
+  // Attempt to determine the token pos and try index from the async closure.
+  Thread* thread = Thread::Current();
+  Zone* zone = thread->zone();
+  const Script& script = Script::Handle(zone, function().script());
+
+  ASSERT(function_.IsAsyncGenClosure() || function_.IsAsyncClosure());
+  // This should only be called on frames that aren't active on the stack.
+  ASSERT(fp() == 0);
+
+  if (function_.is_declared_in_bytecode()) {
+#if !defined(DART_PRECOMPILED_RUNTIME)
+    const auto& bytecode = Bytecode::Handle(zone, function_.bytecode());
+    if (!bytecode.HasSourcePositions()) {
+      return;
+    }
+    const intptr_t await_jump_var = GetAwaitJumpVariable();
+    if (await_jump_var < 0) {
+      return;
+    }
+    // Yield points are counted from 1 (0 is reserved for normal entry).
+    intptr_t yield_point_index = 1;
+    kernel::BytecodeSourcePositionsIterator iter(zone, bytecode);
+    while (iter.MoveNext()) {
+      if (iter.IsYieldPoint()) {
+        if (yield_point_index == await_jump_var) {
+          token_pos_ = iter.TokenPos();
+          token_pos_initialized_ = true;
+          try_index_ = bytecode.GetTryIndexAtPc(bytecode.PayloadStart() +
+                                                iter.PcOffset());
+          return;
+        }
+        ++yield_point_index;
+      }
+    }
+    return;
+#else
+    UNREACHABLE();
+#endif  // !defined(DART_PRECOMPILED_RUNTIME)
+  }
+
+  ASSERT(!IsInterpreted());
+  ASSERT(script.kind() == RawScript::kKernelTag);
+  const Array& await_to_token_map =
+      Array::Handle(zone, script.yield_positions());
+  if (await_to_token_map.IsNull()) {
+    // No mapping.
+    return;
+  }
+  const intptr_t await_jump_var = GetAwaitJumpVariable();
   if (await_jump_var < 0) {
     return;
   }
@@ -1011,29 +1048,6 @@ void ActivationFrame::ExtractTokenPositionFromAsyncClosure() {
   ASSERT(token_pos.IsSmi());
   token_pos_ = TokenPosition(Smi::Cast(token_pos).Value());
   token_pos_initialized_ = true;
-  if (IsInterpreted()) {
-#if !defined(DART_PRECOMPILED_RUNTIME)
-    // In order to determine the try index, we need to map the token position
-    // to a pc offset, and then a pc offset to the try index.
-    // TODO(regis): Should we set the token position fields in pc descriptors?
-    uword pc_offset = kUwordMax;
-    kernel::BytecodeSourcePositionsIterator iter(zone, bytecode());
-    while (iter.MoveNext()) {
-      // PcOffsets are monotonic in source positions, so we get the lowest one.
-      if (iter.TokenPos() == token_pos_) {
-        pc_offset = iter.PcOffset();
-        break;
-      }
-    }
-    if (pc_offset < kUwordMax) {
-      try_index_ =
-          bytecode().GetTryIndexAtPc(bytecode().PayloadStart() + pc_offset);
-    }
-#else
-    UNREACHABLE();
-#endif  // !defined(DART_PRECOMPILED_RUNTIME)
-    return;
-  }
   GetPcDescriptors();
   PcDescriptors::Iterator iter(pc_desc_, RawPcDescriptors::kAnyKind);
   while (iter.MoveNext()) {
@@ -4170,6 +4184,23 @@ bool Debugger::IsAtAsyncJump(ActivationFrame* top_frame) {
   if (!closure_or_null.IsNull()) {
     ASSERT(closure_or_null.IsInstance());
     ASSERT(Instance::Cast(closure_or_null).IsClosure());
+    if (top_frame->function().is_declared_in_bytecode()) {
+#if !defined(DART_PRECOMPILED_RUNTIME)
+      const auto& bytecode =
+          Bytecode::Handle(zone, top_frame->function().bytecode());
+      const TokenPosition token_pos = top_frame->TokenPos();
+      kernel::BytecodeSourcePositionsIterator iter(zone, bytecode);
+      while (iter.MoveNext()) {
+        if (iter.IsYieldPoint() && (iter.TokenPos() == token_pos)) {
+          return true;
+        }
+      }
+      return false;
+#else
+      UNREACHABLE();
+#endif  // !defined(DART_PRECOMPILED_RUNTIME)
+    }
+    ASSERT(!top_frame->IsInterpreted());
     const Script& script = Script::Handle(zone, top_frame->SourceScript());
     ASSERT(script.kind() == RawScript::kKernelTag);
     // Are we at a yield point (previous await)?

runtime/vm/debugger.h

@@ -388,6 +388,7 @@ class ActivationFrame : public ZoneAllocated {
   RawObject* GetAsyncStreamControllerStream();
   RawObject* GetAsyncCompleterAwaiter(const Object& completer);
   RawObject* GetAsyncCompleter();
+  intptr_t GetAwaitJumpVariable();
   void ExtractTokenPositionFromAsyncClosure();
 
   bool IsAsyncMachinery() const;