[vm,dyn_modules] Add flags to bytecode source positions.

There are two possible flags for each source position currently:
a flag that marks the source position as synthetic and a flag that
marks the source position as within a yield point.

Synthetic source positions in bytecode are treated the same as synthetic
source positions in compiled code. That is, they encode the source
position in the text that caused them to be synthesized, but denote that
the covered instructions are internal and not to be used for debugger
pause points or for call site/branch coverage information.

Adding these flags allow us to mark appropriate parts of the async
machinery as synthetic, and also allow us to mark all the bytecode
involved in yield points as having the same token position.

The latter fixes tests where the code would step over a previous
expression, thus being paused at the start of the await bytecode,
and would record the fp and token position there as the ones to
ignore. However, since a new source position wasn't emitted until the
direct call to the await method, the recorded token position would
be the token position prior to the await call, and so the change
in token position at the await call would trigger an early pause.

TEST=pkg/vm_service/test/async_single_step_exception_test
     pkg/vm_service/test/async_single_step_into_test
     pkg/vm_service/test/async_single_step_out_test
     pkg/vm_service/test/async_star_single_step_into_test
     pkg/vm_service/test/async_step_out_test
     pkg/vm_service/test/positive_token_pos_test
     pkg/vm_service/test/step_into_async_no_await_test

Cq-Include-Trybots: luci.dart.try:vm-dyn-linux-debug-x64-try,vm-aot-dyn-linux-debug-x64-try,vm-aot-dyn-linux-product-x64-try,vm-dyn-mac-debug-arm64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-product-x64-try
Change-Id: Ic7642a74fb76227a473f461f360e84dd3d5a45a1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/453322
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>

pkg/dart2bytecode/docs/bytecode.md

@@ -850,9 +850,27 @@ the 'sourcePositions' section of BytecodeFile.
 ```
 type SourcePositions {
   UInt numEntries;
-  // Encoded list of pairs (PC, FileOffset), ordered by PC (offset of bytecode instruction).
+  // Encoded list of pairs (PC, encodedFileOffset), ordered by PC
+  // (offset of bytecode instruction).
+  //
+  // An encodedFileOffset encodes a fileOffset and a set of flags.
+  //
+  // If the encodedFileOffset is non-negative, then the encodedFileOffset
+  // is the same numeric value as the fileOffset.
+  //
+  // If the encodedFileOffset is -1, then there is no source position
+  // associated with the given PC.
+  //
+  // Otherwise, encodedFileOffset = -((fileOffset << n) | flags) -1,
+  // where n is the number of flags below:
+  // - synthetic (bit 0): set if the source position corresponds to
+  //   synthetic code (e.g., a source position that should not be used
+  //   for debugger breakpoints or pausing or for code coverage).
+  // - yieldPoint (bit 1): set if the source position corresponds to
+  //   bytecode that implements a yield point.
+  //
   // For two consecutive entries (PC1, FileOffset1), (PC2, FileOffset2) all
-  // bytecode instructions in range [PC1,PC2) correspond to a source
+  // bytecode instructions in range [PC1,PC2) correspond to an encoded source
   // position FileOffset1.
   SourcePositionEntry[numEntries] entries
 }

pkg/dart2bytecode/lib/assembler.dart

@@ -63,6 +63,7 @@ class BytecodeAssembler {
   final bool _emitSourcePositions;
   bool isUnreachable = false;
   int currentSourcePosition = TreeNode.noOffset;
+  int currentSourcePositionFlags = 0;
 
   BytecodeAssembler(BytecodeOptions options)
       : _emitSourcePositions = options.emitSourcePositions;
@@ -81,12 +82,22 @@ class BytecodeAssembler {
     }
   }
 
+  @pragma('vm:prefer-inline')
+  void _emitSourcePosition() {
+    if (_emitSourcePositions && !isUnreachable) {
+      sourcePositions.add(
+          offset,
+          currentSourcePosition == TreeNode.noOffset
+              ? SourcePositions.noSourcePosition
+              : currentSourcePosition,
+          currentSourcePositionFlags);
+    }
+  }
+
   @pragma('vm:prefer-inline')
   void emitSourcePosition() {
-    if (_emitSourcePositions &&
-        !isUnreachable &&
-        currentSourcePosition != TreeNode.noOffset) {
-      sourcePositions.add(offset, currentSourcePosition);
+    if (currentSourcePosition != TreeNode.noOffset) {
+      _emitSourcePosition();
     }
   }
 
@@ -95,15 +106,7 @@ class BytecodeAssembler {
   // source position to distinguish these calls and avoid stopping at them
   // while single stepping.
   @pragma('vm:prefer-inline')
-  void emitSourcePositionForCall() {
-    if (_emitSourcePositions && !isUnreachable) {
-      sourcePositions.add(
-          offset,
-          currentSourcePosition == TreeNode.noOffset
-              ? SourcePositions.syntheticCodeMarker
-              : currentSourcePosition);
-    }
-  }
+  void emitSourcePositionForCall() => _emitSourcePosition();
 
   void _grow() {
     final newSize = _buffer.length << 1;

pkg/dart2bytecode/lib/bytecode_generator.dart

@@ -1021,8 +1021,11 @@ class BytecodeGenerator extends RecursiveVisitor {
   // variable 'foo' has type 'dynamic'.
   late final implicitCallName = Name('implicit:call');
 
-  void _recordSourcePosition(int fileOffset) {
+  void _recordSourcePosition(int fileOffset, [int? flags]) {
     asm.currentSourcePosition = fileOffset;
+    if (flags != null) {
+      asm.currentSourcePositionFlags = flags;
+    }
     maxSourcePosition = math.max(maxSourcePosition, fileOffset);
   }
 
@@ -1031,9 +1034,11 @@ class BytecodeGenerator extends RecursiveVisitor {
       return;
     }
     final savedSourcePosition = asm.currentSourcePosition;
-    _recordSourcePosition(node.fileOffset);
+    final savedFlags = asm.currentSourcePositionFlags;
+    _recordSourcePosition(node.fileOffset, 0);
     node.accept(this);
     asm.currentSourcePosition = savedSourcePosition;
+    asm.currentSourcePositionFlags = savedFlags;
   }
 
   void _generateNodeList(List<TreeNode> nodes) {
@@ -1216,6 +1221,9 @@ class BytecodeGenerator extends RecursiveVisitor {
           break;
       }
       if (returnMethod != null) {
+        // Unlike other async machinery, this can't be marked synthetic
+        // as the method may return directly from the direct call and so
+        // the debugger needs to pause at it, not the following return.
         asm.emitPopLocal(locals.returnVarIndexInFrame);
         asm.emitPush(locals.suspendStateVarIndexInFrame);
         asm.emitPush(locals.returnVarIndexInFrame);
@@ -1633,17 +1641,19 @@ class BytecodeGenerator extends RecursiveVisitor {
     savedAssemblers = null;
     currentLoopDepth = 0;
     savedMaxSourcePositions = <int>[];
-    if (node is Procedure) {
-      maxSourcePosition = node.fileStartOffset;
-    } else if (node is Constructor) {
-      maxSourcePosition = node.startFileOffset;
-    } else {
-      maxSourcePosition = node.fileOffset;
-    }
 
     locals = new LocalVariables(node, options, staticTypeContext);
     locals.enterScope(node);
 
+    final int startPosition;
+    if (node is Procedure) {
+      startPosition = node.fileStartOffset;
+    } else if (node is Constructor) {
+      startPosition = node.startFileOffset;
+    } else {
+      startPosition = node.fileOffset;
+    }
+    _recordSourcePosition(startPosition, SourcePositions.syntheticFlag);
     _genPrologue(node, node.function);
     _setupInitialContext(node.function);
     _emitFirstDebugCheck(node, node.function);
@@ -1677,6 +1687,9 @@ class BytecodeGenerator extends RecursiveVisitor {
     if (!locals.isSuspendableFunction) {
       return;
     }
+
+    final savedFlags = asm.currentSourcePositionFlags;
+    asm.currentSourcePositionFlags |= SourcePositions.syntheticFlag;
     Procedure initMethod;
     switch (function!.dartAsyncMarker) {
       case AsyncMarker.Async:
@@ -1696,6 +1709,10 @@ class BytecodeGenerator extends RecursiveVisitor {
     asm.emitPopLocal(locals.suspendStateVarIndexInFrame);
 
     if (function.dartAsyncMarker != AsyncMarker.Async) {
+      final savedFlags = asm.currentSourcePositionFlags;
+      // Mark all of the code in this block as within the yield point.
+      asm.currentSourcePositionFlags |= SourcePositions.yieldPointFlag;
+      asm.emitSourcePositionForCall();
       // Suspend async* and sync* functions after prologue is finished.
       Label done = Label();
       asm.emitSuspend(done);
@@ -1710,6 +1727,7 @@ class BytecodeGenerator extends RecursiveVisitor {
 
       asm.bind(done);
       asm.emitDrop1(); // Discard result of Suspend.
+      asm.currentSourcePositionFlags = savedFlags;
     }
 
     if (function.dartAsyncMarker == AsyncMarker.SyncStar &&
@@ -1729,6 +1747,7 @@ class BytecodeGenerator extends RecursiveVisitor {
       asyncTryBlock.needsStackTrace = true;
       asyncTryBlock.types.add(cp.addType(const DynamicType()));
     }
+    asm.currentSourcePositionFlags = savedFlags;
   }
 
   void _endSuspendableFunction(FunctionNode? function) {
@@ -1744,6 +1763,8 @@ class BytecodeGenerator extends RecursiveVisitor {
       // Exception handlers are reachable although there are no labels or jumps.
       asm.isUnreachable = false;
 
+      final savedFlags = asm.currentSourcePositionFlags;
+      asm.currentSourcePositionFlags |= SourcePositions.syntheticFlag;
       asm.emitSetFrame(locals.frameSize);
 
       final rethrowException = Label();
@@ -1765,6 +1786,7 @@ class BytecodeGenerator extends RecursiveVisitor {
       asm.emitMoveSpecial(SpecialIndex.stackTrace, temp);
       asm.emitPush(temp);
       asm.emitThrow(1);
+      asm.currentSourcePositionFlags = savedFlags;
     }
   }
 
@@ -1954,9 +1976,11 @@ class BytecodeGenerator extends RecursiveVisitor {
       }
     }
 
+    // The CheckStack below is the instruction which should be used for function
+    // entry breakpoints.
+    _recordInitialSourcePositionForFunction(node, function);
     // CheckStack must see a properly initialized context when stress-testing
     // stack trace collection.
-    _recordInitialSourcePositionForFunction(node, function);
     asm.emitCheckStack(0);
 
     if (locals.hasFunctionTypeArgsVar && isClosure) {
@@ -2063,7 +2087,8 @@ class BytecodeGenerator extends RecursiveVisitor {
 
   void _recordInitialSourcePositionForFunction(
       TreeNode node, FunctionNode? function) {
-    _recordSourcePosition(_initialSourcePositionForFunction(node, function));
+    final position = _initialSourcePositionForFunction(node, function);
+    _recordSourcePosition(position, 0);
   }
 
   void _emitFirstDebugCheck(TreeNode node, FunctionNode? function) {
@@ -2417,7 +2442,7 @@ class BytecodeGenerator extends RecursiveVisitor {
 
     final int closureFunctionIndex = cp.addClosureFunction(closureIndex);
 
-    maxSourcePosition = math.max(maxSourcePosition, function.fileOffset);
+    _recordSourcePosition(function.fileOffset, SourcePositions.syntheticFlag);
     _genPrologue(node, function);
     _setupInitialContext(function);
     _emitFirstDebugCheck(node, function);
@@ -2695,7 +2720,8 @@ class BytecodeGenerator extends RecursiveVisitor {
     if (options.emitSourcePositions) {
       asm.emitSourcePosition();
     }
-    if (options.emitDebuggerStops) {
+    if (options.emitDebuggerStops &&
+        (asm.currentSourcePositionFlags & SourcePositions.syntheticFlag) == 0) {
       asm.emitDebugCheck();
     }
   }
@@ -3413,8 +3439,7 @@ class BytecodeGenerator extends RecursiveVisitor {
     tryCatches![tryCatch]!.needsStackTrace = true;
 
     // Allow breakpoint on explicit rethrow statement.
-    _emitSourcePosition();
-    _genRethrow(tryCatch);
+    _genRethrow(tryCatch, isSynthetic: false);
   }
 
   bool _hasNonTrivialInitializer(Field field) {
@@ -4147,7 +4172,13 @@ class BytecodeGenerator extends RecursiveVisitor {
     }
   }
 
-  void _genRethrow(TreeNode node) {
+  void _genRethrow(TreeNode node, {required bool isSynthetic}) {
+    final savedFlags = asm.currentSourcePositionFlags;
+    if (isSynthetic) {
+      asm.currentSourcePositionFlags |= SourcePositions.syntheticFlag;
+    } else {
+      asm.currentSourcePositionFlags &= ~SourcePositions.syntheticFlag;
+    }
     final capturedExceptionVar = locals.capturedExceptionVar(node);
     if (capturedExceptionVar != null) {
       assert(locals.isCaptured(capturedExceptionVar));
@@ -4165,6 +4196,7 @@ class BytecodeGenerator extends RecursiveVisitor {
     }
 
     asm.emitThrow(1);
+    asm.currentSourcePositionFlags = savedFlags;
   }
 
   @override
@@ -4241,7 +4273,7 @@ class BytecodeGenerator extends RecursiveVisitor {
 
     if (!hasCatchAll) {
       tryBlock.needsStackTrace = true;
-      _genRethrow(node);
+      _genRethrow(node, isSynthetic: true);
     }
 
     asm.bind(done);
@@ -4277,7 +4309,7 @@ class BytecodeGenerator extends RecursiveVisitor {
     _generateNode(node.finalizer);
 
     tryBlock.needsStackTrace = true; // For rethrowing.
-    _genRethrow(node);
+    _genRethrow(node, isSynthetic: true);
 
     for (var finallyBlock in finallyBlocks[node]!) {
       asm.bind(finallyBlock.entry);
@@ -4457,6 +4489,14 @@ class BytecodeGenerator extends RecursiveVisitor {
   void visitAwaitExpression(AwaitExpression node) {
     _generateNode(node.operand);
 
+    // The rest of the await expression bytecode is the yield point.
+    // Emit a source position at the start to ensure that if the debugger
+    // is paused anywhere in this bytecode, a request to step over the await
+    // doesn't pause at either the direct call or the return, which have the
+    // same source position information.
+    final savedFlags = asm.currentSourcePositionFlags;
+    asm.currentSourcePositionFlags |= SourcePositions.yieldPointFlag;
+    asm.emitSourcePositionForCall();
     final int temp = locals.tempIndexInFrame(node);
     asm.emitPopLocal(temp);
 
@@ -4478,6 +4518,7 @@ class BytecodeGenerator extends RecursiveVisitor {
       _genDirectCall(_await, objectTable.getArgDescHandle(2), 2);
     }
     asm.emitReturnTOS();
+    asm.currentSourcePositionFlags = savedFlags;
 
     asm.bind(done);
   }

pkg/dart2bytecode/lib/source_positions.dart

@@ -14,57 +14,94 @@ import 'bytecode_serialization.dart'
 
 /// Maintains mapping between bytecode instructions and source positions.
 class SourcePositions extends BytecodeDeclaration {
-  // Special value of fileOffset which marks synthetic code without source
+  // Special value of fileOffset which marks synthetic code without a source
   // position.
-  static const syntheticCodeMarker = -1;
+  static const noSourcePosition = -1;
+  // The flags encoded into the low bits of the source position.
+  static const syntheticFlag = 1 << 0;
+  static const yieldPointFlag = 1 << 1;
+  static const _numFlags = 2;
+  static const _flagMask = (1 << _numFlags) - 1;
 
-  final List<int> _positions = <int>[]; // Pairs (PC, fileOffset).
-  int _lastPc = 0;
-  int _lastOffset = 0;
+  final _positions = <int>[]; // Pairs (PC, fileOffset).
+  // Stored separately just to make sure no call to add uses a smaller
+  // PC offset than the previous call, even if the previous call didn't
+  // add an entry to the list because the last entry covers it.
+  int _lastPcAdded = 0;
 
   SourcePositions();
 
-  // Maps the given PC to the given file offset.
-  void add(int pc, int fileOffset) {
-    assert((fileOffset >= 0) || (fileOffset == syntheticCodeMarker));
-    if (fileOffset != _lastOffset) {
-      if (pc <= _lastPc) {
-        throw ArgumentError(
-            '$pc <= $_lastPc for change in source position $fileOffset != $_lastOffset',
-            'pc');
-      }
-      _positions.add(pc);
-      _positions.add(fileOffset);
-      _lastPc = pc;
-      _lastOffset = fileOffset;
+  int _encode(int fileOffset, int flags) =>
+      (flags == 0 || fileOffset == noSourcePosition)
+          ? fileOffset
+          : -((fileOffset << _numFlags) | flags) - 1;
+
+  (int, int) _decode(int encoded) {
+    if (encoded >= 0 || encoded == noSourcePosition) {
+      return (encoded, 0);
     }
+    final value = -encoded - 1;
+    return (value >> _numFlags, value & _flagMask);
+  }
+
+  // Maps the given PC to the given file offset. If there is already an entry
+  // for the given PC, then the new information overwrites the old information.
+  //
+  // Marks the source position as synthetic (not to be used by the debugger
+  // or coverage calculations) if [(flags & syntheticFlag) != 0].
+  //
+  // Marks the pc as within a yield point if [(flags & yieldPointFlag) != 0].
+  //
+  // Assumes that the pc is greater than or equal to the pc used in the most
+  // recent call to add, if any.
+  void add(int pc, int fileOffset, int flags) {
+    assert(fileOffset >= 0 || fileOffset == noSourcePosition);
+    assert((flags & ~_flagMask) == 0);
+    if (_lastPcAdded > pc) {
+      throw ArgumentError('Attempt to add entry for $pc after $_lastPcAdded');
+    }
+    _lastPcAdded = pc;
+    final encodedFileOffset = _encode(fileOffset, flags);
+    if (_positions.isNotEmpty) {
+      final i = _positions.length - 2;
+      final lastPc = _positions[i];
+      final lastFileOffset = _positions[i + 1];
+      if (lastFileOffset == encodedFileOffset) {
+        // The last entry covers this PC offset as well.
+        return;
+      }
+      if (lastPc == pc) {
+        // The new entry for this PC overwrites the old one.
+        _positions.removeRange(i, i + 2);
+      }
+    }
+    _positions.add(pc);
+    _positions.add(encodedFileOffset);
   }
 
   bool get isEmpty => _positions.isEmpty;
   bool get isNotEmpty => !isEmpty;
 
   void write(BufferedWriter writer) {
-    writer.writePackedUInt30(_positions.length ~/ 2);
+    final pairs = _positions.length ~/ 2;
+    writer.writePackedUInt30(pairs);
     final encodePC = new PackedUInt30DeltaEncoder();
     final encodeOffset = new SLEB128DeltaEncoder();
-    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);
+    for (int i = 0; i < pairs; i++) {
+      encodePC.write(writer, _positions[2 * i]);
+      encodeOffset.write(writer, _positions[2 * i + 1]);
     }
   }
 
   SourcePositions.read(BufferedReader reader) {
-    final int length = reader.readPackedUInt30();
+    final int pairs = reader.readPackedUInt30();
     final decodePC = new PackedUInt30DeltaDecoder();
     final decodeOffset = new SLEB128DeltaDecoder();
-    for (int i = 0; i < length; ++i) {
-      int pc = decodePC.read(reader);
-      int fileOffset = decodeOffset.read(reader);
-      _positions.add(pc);
-      _positions.add(fileOffset);
+    for (int i = 0; i < pairs; i++) {
+      _positions.add(decodePC.read(reader));
+      _positions.add(decodeOffset.read(reader));
     }
+    _lastPcAdded = _positions.isEmpty ? 0 : _positions[_positions.length - 2];
   }
 
   @override
@@ -73,14 +110,19 @@ class SourcePositions extends BytecodeDeclaration {
   Map<int, String> getBytecodeAnnotations() {
     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 = 'source position $fileOffset';
-      if (map[pc] == null) {
-        map[pc] = entry;
-      } else {
-        map[pc] = "${map[pc]}; $entry";
+      final pc = _positions[i];
+      final (fileOffset, flags) = _decode(_positions[i + 1]);
+      String annotation = '';
+      if ((flags & syntheticFlag) != 0) {
+        annotation += 'synthetic ';
       }
+      if ((flags & yieldPointFlag) != 0) {
+        annotation += 'yield point @ ';
+      }
+      annotation += 'source position $fileOffset';
+      // There is at most one entry per PC offset.
+      assert(map[pc] == null);
+      map[pc] = annotation;
     }
     return map;
   }

runtime/vm/bytecode_reader.h

@@ -537,12 +537,15 @@ class BytecodeReader : public AllStatic {
 };
 
 class BytecodeSourcePositionsIterator : ValueObject {
- public:
   // These constants should match corresponding constants in class
   // SourcePositions (pkg/dart2bytecode/lib/source_positions.dart).
-  static const intptr_t kSyntheticCodeMarker = -1;
-  static const intptr_t kYieldPointMarker = -2;
+  static const intptr_t kNoSourcePosition = -1;
+  static const intptr_t kSyntheticFlag = 1 << 0;
+  static const intptr_t kYieldPointFlag = 1 << 1;
+  static const intptr_t kNumFlags = 2;
+  static const intptr_t kFlagMask = (1 << kNumFlags) - 1;
 
+ public:
   BytecodeSourcePositionsIterator(Zone* zone, const Bytecode& bytecode)
       : reader_(TypedDataBase::Handle(zone, bytecode.binary())) {
     ASSERT(bytecode.HasSourcePositions());
@@ -557,12 +560,14 @@ class BytecodeSourcePositionsIterator : ValueObject {
     ASSERT(pairs_remaining_ > 0);
     --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;
+    cur_encoded_ += reader_.ReadSLEB128();
+    if (cur_encoded_ >= 0 || cur_encoded_ == kNoSourcePosition) {
+      cur_position_ = cur_encoded_;
+      flags_ = 0;
+    } else {
+      const intptr_t value = -cur_encoded_ - 1;
+      cur_position_ = value >> kNumFlags;
+      flags_ = value & kFlagMask;
     }
     return true;
   }
@@ -570,19 +575,22 @@ class BytecodeSourcePositionsIterator : ValueObject {
   uword PcOffset() const { return cur_bci_; }
 
   TokenPosition TokenPos() const {
-    return (cur_token_pos_ == kSyntheticCodeMarker)
+    return ((flags_ & kSyntheticFlag) != 0)
+               ? TokenPosition::Synthetic(cur_position_)
+           : cur_position_ == kNoSourcePosition
                ? TokenPosition::kNoSource
-               : TokenPosition::Deserialize(cur_token_pos_);
+               : TokenPosition::Deserialize(cur_position_);
   }
 
-  bool IsYieldPoint() const { return is_yield_point_; }
+  bool IsYieldPoint() const { return (flags_ & kYieldPointFlag) != 0; }
 
  private:
   Reader reader_;
   intptr_t pairs_remaining_ = 0;
   intptr_t cur_bci_ = 0;
-  intptr_t cur_token_pos_ = 0;
-  bool is_yield_point_ = false;
+  intptr_t cur_encoded_ = 0;
+  intptr_t cur_position_ = 0;
+  intptr_t flags_ = 0;
 };
 
 #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)

runtime/vm/compiler/assembler/disassembler_kbc.cc

@@ -373,54 +373,187 @@ void KernelBytecodeDisassembler::Disassemble(const Function& function) {
       ObjectPool::Handle(zone, bytecode.object_pool());
   object_pool.DebugPrint();
 
-  THR_Print("PC Descriptors for function '%s' {\n", function_fullname);
   const PcDescriptors& descriptors =
       PcDescriptors::Handle(zone, bytecode.pc_descriptors());
-  THR_Print("%s}\n", descriptors.ToCString());
+  if (!descriptors.IsNull()) {
+    THR_Print("PC Descriptors for function '%s' {\n", function_fullname);
+    ZoneTextBuffer buffer(zone);
+    descriptors.WriteToBuffer(&buffer, base);
+    THR_Print("%s", buffer.buffer());
+    THR_Print("}\n");
+  }
 
   if (bytecode.HasSourcePositions()) {
-    THR_Print("Source positions for function '%s' {\n", function_fullname);
-    // 4 bits per hex digit + 2 for "0x".
-    const int addr_width = (kBitsPerWord / 4) + 2;
-    // "*" in a printf format specifier tells it to read the field width from
-    // the printf argument list.
-    THR_Print("%-*s\tpos\tline\tcolumn\tyield\n", addr_width, "pc");
     const Script& script = Script::Handle(zone, function.script());
-    bytecode::BytecodeSourcePositionsIterator iter(zone, bytecode);
-    while (iter.MoveNext()) {
-      TokenPosition pos = iter.TokenPos();
-      intptr_t line = -1, column = -1;
-      script.GetTokenLocation(pos, &line, &column);
-      THR_Print("%#-*" Px "\t%s\t%" Pd "\t%" Pd "\t%s\n", addr_width,
-                base + iter.PcOffset(), pos.ToCString(), line, column,
-                iter.IsYieldPoint() ? "yield" : "");
-    }
+    THR_Print("Source positions for function '%s' {\n", function_fullname);
+    ZoneTextBuffer buffer(zone);
+    PrintSourcePositions(zone, &buffer, base, bytecode, script);
+    THR_Print("%s", buffer.buffer());
     THR_Print("}\n");
   }
 
   if (bytecode.HasLocalVariablesInfo()) {
-#if !defined(DART_PRECOMPILED_RUNTIME)
     THR_Print("Local variable information for function '%s' {\n",
               function_fullname);
     ZoneTextBuffer buffer(zone);
-    bytecode.WriteLocalVariablesInfo(zone, &buffer);
+    PrintLocalVariablesInfo(zone, &buffer, bytecode, base);
     THR_Print("%s", buffer.buffer());
     THR_Print("}\n");
-#else
-    UNREACHABLE();
-#endif
   }
 
-  THR_Print("Exception Handlers for function '%s' {\n", function_fullname);
   const ExceptionHandlers& handlers =
       ExceptionHandlers::Handle(zone, bytecode.exception_handlers());
-  THR_Print("%s}\n", handlers.ToCString());
-
+  if (!handlers.IsNull()) {
+    THR_Print("Exception Handlers for function '%s' {\n", function_fullname);
+    ZoneTextBuffer buffer(zone);
+    handlers.WriteToBuffer(&buffer, base);
+    THR_Print("%s", buffer.buffer());
+    THR_Print("}\n");
+  }
 #else
   UNREACHABLE();
 #endif
 }
 
+// 4 bits per hex digit + 2 for "0x".
+static const int kProgramCounterFieldWidth = (kBitsPerWord / 4) + 2;
+static const int kUint32FieldWidth = 7;
+// For bytecode, these are either:
+// * real positions, which are a uint32_t source offset and thus a
+//   max of 7 digits,
+// * synthethic positions, which have a prefix of 'syn:' before a
+//   source offset and thus a max of 11 characters, or
+// * NoSource, which is written as "NoSource" (8).
+static const int kSourcePositionFieldWidth = 11;
+static const int kSourcePositionColumnWidths[] = {
+    kProgramCounterFieldWidth,  // pc
+    kSourcePositionFieldWidth,  // pos
+    kUint32FieldWidth,          // line
+    kUint32FieldWidth,          // col
+};
+
+void KernelBytecodeDisassembler::PrintSourcePositions(Zone* zone,
+                                                      BaseTextBuffer* buffer,
+                                                      uword base,
+                                                      const Bytecode& bytecode,
+                                                      const Script& script) {
+  if (!bytecode.HasSourcePositions()) return;
+
+  // "*" in a printf format specifier tells it to read the field width from
+  // the printf argument list.
+  buffer->Printf(" %-*s %*s %*s %*s yield\n", kSourcePositionColumnWidths[0],
+                 "pc", kSourcePositionColumnWidths[1], "pos",
+                 kSourcePositionColumnWidths[2], "line",
+                 kSourcePositionColumnWidths[3], "col");
+  bytecode::BytecodeSourcePositionsIterator iter(zone, bytecode);
+  while (iter.MoveNext()) {
+    buffer->Printf(" %#-*" Px "", kSourcePositionColumnWidths[0],
+                   base + iter.PcOffset());
+    const TokenPosition pos = iter.TokenPos();
+    buffer->Printf(" %*s", kSourcePositionColumnWidths[1], pos.ToCString());
+    intptr_t line = -1, column = -1;
+    if (!script.IsNull() && script.GetTokenLocation(pos, &line, &column)) {
+      buffer->Printf(" %*" Pd " %*" Pd "", kSourcePositionColumnWidths[2], line,
+                     kSourcePositionColumnWidths[3], column);
+    } else {
+      buffer->Printf(" %*s %*s", kSourcePositionColumnWidths[2], "-",
+                     kSourcePositionColumnWidths[3], "-");
+    }
+    if (iter.IsYieldPoint()) {
+      buffer->AddString(" X");
+    }
+    buffer->AddString("\n");
+  }
+}
+
+#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
+static const int kLocalVariableKindFieldWidth = strlen(
+    bytecode::BytecodeLocalVariablesIterator::kKindNames
+        [bytecode::BytecodeLocalVariablesIterator::kVariableDeclaration]);
+
+static const int kLocalVariableColumnWidths[] = {
+    kLocalVariableKindFieldWidth,  // kind
+    kProgramCounterFieldWidth,     // start pc
+    kProgramCounterFieldWidth,     // end pc
+    kUint32FieldWidth,             // context level
+    kUint32FieldWidth,             // index
+    kSourcePositionFieldWidth,     // start token pos
+    kSourcePositionFieldWidth,     // end token pos
+    kSourcePositionFieldWidth,     // decl token pos
+};
+#endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
+
+void KernelBytecodeDisassembler::PrintLocalVariablesInfo(
+    Zone* zone,
+    BaseTextBuffer* buffer,
+    const Bytecode& bytecode,
+    uword base) {
+  if (!bytecode.HasLocalVariablesInfo()) return;
+
+#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
+  // "*" in a printf format specifier tells it to read the field width from
+  // the printf argument list.
+  buffer->Printf(
+      " %-*s %*s %*s %*s %*s %*s %*s %*s   name\n",
+      kLocalVariableColumnWidths[0], "kind", kLocalVariableColumnWidths[1],
+      "start pc", kLocalVariableColumnWidths[2], "end pc",
+      kLocalVariableColumnWidths[3], "ctx", kLocalVariableColumnWidths[4],
+      "index", kLocalVariableColumnWidths[5], "start",
+      kLocalVariableColumnWidths[6], "end", kLocalVariableColumnWidths[7],
+      "decl");
+  auto& name = String::Handle(zone);
+  auto& type = AbstractType::Handle(zone);
+  bytecode::BytecodeLocalVariablesIterator iter(zone, bytecode);
+  while (iter.MoveNext()) {
+    buffer->Printf(" %-*s %#*" Px "", kLocalVariableColumnWidths[0],
+                   iter.KindName(), kLocalVariableColumnWidths[1],
+                   base + iter.StartPC());
+    if (iter.IsVariableDeclaration() || iter.IsScope()) {
+      buffer->Printf(" %#*" Px "", kLocalVariableColumnWidths[2],
+                     base + iter.EndPC());
+    } else {
+      buffer->Printf(" %*s", kLocalVariableColumnWidths[2], "-");
+    }
+    if (iter.IsScope()) {
+      buffer->Printf(" %*" Pd "", kLocalVariableColumnWidths[3],
+                     iter.ContextLevel());
+    } else {
+      buffer->Printf(" %*s", kLocalVariableColumnWidths[3], "-");
+    }
+    if (iter.IsContextVariable() || iter.IsVariableDeclaration()) {
+      buffer->Printf(" %*" Pd "", kLocalVariableColumnWidths[4], iter.Index());
+    } else {
+      buffer->Printf(" %*s", kLocalVariableColumnWidths[4], "-");
+    }
+    if (iter.IsVariableDeclaration() || iter.IsScope()) {
+      buffer->Printf(" %*s %*s", kLocalVariableColumnWidths[5],
+                     iter.StartTokenPos().ToCString(),
+                     kLocalVariableColumnWidths[6],
+                     iter.EndTokenPos().ToCString());
+
+    } else {
+      buffer->Printf(" %*s %*s", kLocalVariableColumnWidths[5], "-",
+                     kLocalVariableColumnWidths[6], "-");
+    }
+    if (iter.IsVariableDeclaration()) {
+      name = iter.Name();
+      type = iter.Type();
+      buffer->Printf(" %*s   %s: ", kLocalVariableColumnWidths[7],
+                     iter.DeclarationTokenPos().ToCString(), name.ToCString());
+      type.PrintName(Object::kInternalName, buffer);
+      if (iter.IsCaptured()) {
+        buffer->AddString(" (captured)");
+      }
+    } else {
+      buffer->Printf(" %*s   %s", kLocalVariableColumnWidths[7], "-", "-");
+    }
+    buffer->AddString("\n");
+  }
+#else
+  UNREACHABLE();
+#endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
+}
+
 }  // namespace dart
 
 #endif  // defined(DART_DYNAMIC_MODULES)

runtime/vm/compiler/assembler/disassembler_kbc.h

@@ -77,6 +77,17 @@ class KernelBytecodeDisassembler : public AllStatic {
 
   static void Disassemble(const Function& function);
 
+  static void PrintSourcePositions(Zone* zone,
+                                   BaseTextBuffer* buffer,
+                                   uword base,
+                                   const Bytecode& bytecode,
+                                   const Script& script);
+
+  static void PrintLocalVariablesInfo(Zone* zone,
+                                      BaseTextBuffer* buffer,
+                                      const Bytecode& bytecode,
+                                      uword base);
+
  private:
   static const int kHexadecimalBufferSize = 32;
   static const int kUserReadableBufferSize = 256;

runtime/vm/debugger.cc

@@ -6,14 +6,13 @@
 
 #include "include/dart_api.h"
 
-#if defined(DART_DYNAMIC_MODULES)
 #include "vm/bytecode_reader.h"
-#endif
 #include "vm/closure_functions_cache.h"
 #include "vm/code_descriptors.h"
 #include "vm/code_patcher.h"
 #include "vm/compiler/api/deopt_id.h"
 #include "vm/compiler/assembler/disassembler.h"
+#include "vm/compiler/assembler/disassembler_kbc.h"
 #include "vm/compiler/jit/compiler.h"
 #include "vm/dart_entry.h"
 #include "vm/flags.h"
@@ -637,16 +636,13 @@ void ActivationFrame::PrintContextLevelError(const char* message) {
   OS::PrintErr("deopt_id_ %" Px "\n", deopt_id_);
   OS::PrintErr("context_level_ %" Px "\n", context_level_);
   OS::PrintErr("token_pos_ %s\n", token_pos_.ToCString());
-  if (IsInterpreted()) {
+  if (IsInterpreted() && bytecode().HasLocalVariablesInfo()) {
 #if defined(DART_DYNAMIC_MODULES)
-#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
     Zone* const zone = Thread::Current()->zone();
     ZoneTextBuffer buffer(zone);
-    bytecode().WriteLocalVariablesInfo(zone, &buffer);
+    KernelBytecodeDisassembler::PrintLocalVariablesInfo(
+        zone, &buffer, bytecode(), PayloadStart());
     OS::PrintErr("%s\n", buffer.buffer());
-#endif
-#else
-    UNREACHABLE();
 #endif
   } else {
     DisassembleToStdout formatter;
@@ -3904,34 +3900,19 @@ static bool IsAtAsyncJump(ActivationFrame* top_frame) {
   if (top_frame->IsInterpreted()) {
 #if defined(DART_DYNAMIC_MODULES)
     const auto& bytecode = top_frame->bytecode();
-    const uword prev = bytecode.GetInstructionBefore(top_frame->pc());
-    if (prev == 0) {
-      // Async awaiter frames have an PC offset of 0 or 1.
-      ASSERT(top_frame->pc() == bytecode.PayloadStart() ||
-             top_frame->pc() == bytecode.PayloadStart() +
-                                    StackTraceUtils::kFutureListenerPcOffset);
-      return false;
+    ASSERT(bytecode.HasSourcePositions());
+    const uword pc_offset = top_frame->pc() - bytecode.PayloadStart();
+    // pc_offset could equal to bytecode size if the last instruction is Throw.
+    ASSERT(pc_offset <= static_cast<uword>(bytecode.Size()));
+    bytecode::BytecodeSourcePositionsIterator iter(zone, bytecode);
+    bool is_yield_point = false;
+    while (iter.MoveNext()) {
+      if (pc_offset <= iter.PcOffset()) {
+        break;
+      }
+      is_yield_point = iter.IsYieldPoint();
     }
-    auto* const instr = reinterpret_cast<const KBCInstr*>(prev);
-    // Async jumps in bytecode are implemented via direct calls to the
-    // appropriate Dart method.
-    if (!KernelBytecode::IsDirectCallOpcode(instr)) {
-      return false;
-    }
-    const auto& object_pool = ObjectPool::Handle(zone, bytecode.object_pool());
-    auto const index = KernelBytecode::DecodeD(instr);
-    const auto& obj = Object::Handle(zone, object_pool.ObjectAt(index));
-    if (obj.IsNull() || !obj.IsFunction()) {
-      return false;
-    }
-    const auto& target = Function::Cast(obj);
-    auto* const object_store = thread->isolate_group()->object_store();
-    return target.ptr() == object_store->suspend_state_await() ||
-           target.ptr() ==
-               object_store->suspend_state_await_with_type_check() ||
-           target.ptr() == object_store->suspend_state_yield_async_star() ||
-           target.ptr() ==
-               object_store->suspend_state_suspend_sync_star_at_start();
+    return is_yield_point;
 #else
     UNREACHABLE();
 #endif
@@ -3955,33 +3936,28 @@ static bool IsAtAsyncJump(ActivationFrame* top_frame) {
 static bool IsAtBytecodeAsyncReturn(ActivationFrame* top_frame) {
 #if defined(DART_DYNAMIC_MODULES)
   if (!top_frame->IsInterpreted()) return false;
-
-  Thread* const thread = Thread::Current();
-  Zone* const zone = thread->zone();
   const auto& bytecode = top_frame->bytecode();
-  uword prev = bytecode.GetInstructionBefore(top_frame->pc());
+  const uword return_address = top_frame->pc();
+  const uword prev = bytecode.GetInstructionBefore(return_address);
   if (prev == 0) {
+    const uword start = bytecode.PayloadStart();
     // Async awaiter frames have an PC offset of 0 or 1.
-    ASSERT(top_frame->pc() == bytecode.PayloadStart() ||
-           top_frame->pc() == bytecode.PayloadStart() +
-                                  StackTraceUtils::kFutureListenerPcOffset);
+    ASSERT(return_address == start ||
+           return_address == start + StackTraceUtils::kFutureListenerPcOffset);
     return false;
   }
-  auto* instr = reinterpret_cast<const KBCInstr*>(prev);
+  auto* const instr = reinterpret_cast<const KBCInstr*>(prev);
   // Async returns in bytecode are implemented via direct calls to
-  // the appropriate Dart async return method followed by a return
-  // instruction.
-  if (KernelBytecode::IsReturnOpcode(instr)) {
-    prev = bytecode.GetInstructionBefore(prev);
-    ASSERT(prev != 0);
-    instr = reinterpret_cast<const KBCInstr*>(prev);
-  }
+  // the appropriate Dart async return method. Note that unlike other async
+  // machinery, the direct call to _returnAsync is not marked synthetic.
   if (!KernelBytecode::IsDirectCallOpcode(instr)) {
     return false;
   }
-  const auto& object_pool = ObjectPool::Handle(zone, bytecode.object_pool());
   auto const index = KernelBytecode::DecodeD(instr);
-  const auto& obj = Object::Handle(zone, object_pool.ObjectAt(index));
+  auto* const thread = Thread::Current();
+  Zone* const zone = thread->zone();
+  const auto& pool = ObjectPool::Handle(zone, bytecode.object_pool());
+  const auto& obj = Object::Handle(zone, pool.ObjectAt(index));
   if (obj.IsNull() || !obj.IsFunction()) {
     return false;
   }

runtime/vm/object.cc

@@ -19426,86 +19426,6 @@ LocalVarDescriptorsPtr Bytecode::GetLocalVarDescriptors() const {
   UNREACHABLE();
 #endif
 }
-
-#if defined(DART_DYNAMIC_MODULES)
-static const int kLocalVariableKindMaxWidth = strlen(
-    bytecode::BytecodeLocalVariablesIterator::kKindNames
-        [bytecode::BytecodeLocalVariablesIterator::kVariableDeclaration]);
-
-static const int kLocalVariableColumnWidths[] = {
-    kLocalVariableKindMaxWidth,  // kind
-    14,                          // start pc
-    14,                          // end pc
-    7,                           // context level
-    7,                           // index
-    7,                           // start token pos
-    7,                           // end token pos
-    7,                           // decl token pos
-};
-#endif
-
-void Bytecode::WriteLocalVariablesInfo(Zone* zone,
-                                       BaseTextBuffer* buffer) const {
-#if defined(DART_DYNAMIC_MODULES)
-  if (!HasLocalVariablesInfo()) return;
-
-  // "*" in a printf format specifier tells it to read the field width from
-  // the printf argument list.
-  buffer->Printf(
-      " %*s %*s %*s %*s %*s %*s %*s %*s name\n", kLocalVariableColumnWidths[0],
-      "kind", kLocalVariableColumnWidths[1], "start pc",
-      kLocalVariableColumnWidths[2], "end pc", kLocalVariableColumnWidths[3],
-      "ctx", kLocalVariableColumnWidths[4], "index",
-      kLocalVariableColumnWidths[5], "start", kLocalVariableColumnWidths[6],
-      "end", kLocalVariableColumnWidths[7], "decl");
-  auto& name = String::Handle(zone);
-  auto& type = AbstractType::Handle(zone);
-  const uword base = PayloadStart();
-  bytecode::BytecodeLocalVariablesIterator iter(zone, *this);
-  while (iter.MoveNext()) {
-    buffer->Printf(" %*s %-#*" Px "", kLocalVariableColumnWidths[0],
-                   iter.KindName(), kLocalVariableColumnWidths[1],
-                   base + iter.StartPC());
-    if (iter.IsVariableDeclaration() || iter.IsScope()) {
-      buffer->Printf(" %-#*" Px "", kLocalVariableColumnWidths[2],
-                     base + iter.EndPC());
-    } else {
-      buffer->Printf(" %*s", kLocalVariableColumnWidths[2], "");
-    }
-    if (iter.IsScope()) {
-      buffer->Printf(" %*" Pd "", kLocalVariableColumnWidths[3],
-                     iter.ContextLevel());
-    } else {
-      buffer->Printf(" %*s", kLocalVariableColumnWidths[3], "");
-    }
-    if (iter.IsContextVariable() || iter.IsVariableDeclaration()) {
-      buffer->Printf(" %*" Pd "", kLocalVariableColumnWidths[4], iter.Index());
-    } else {
-      buffer->Printf(" %*s", kLocalVariableColumnWidths[4], "");
-    }
-    if (iter.IsVariableDeclaration() || iter.IsScope()) {
-      buffer->Printf(" %*s %*s", kLocalVariableColumnWidths[5],
-                     iter.StartTokenPos().ToCString(),
-                     kLocalVariableColumnWidths[6],
-                     iter.EndTokenPos().ToCString());
-
-    } else {
-      buffer->Printf(" %*s %*s", kLocalVariableColumnWidths[5], "",
-                     kLocalVariableColumnWidths[6], "");
-    }
-    if (iter.IsVariableDeclaration()) {
-      name = iter.Name();
-      type = iter.Type();
-      buffer->Printf(" %*s %s: %s%s", kLocalVariableColumnWidths[7],
-                     iter.DeclarationTokenPos().ToCString(), name.ToCString(),
-                     type.ToCString(), iter.IsCaptured() ? " (captured)" : "");
-    }
-    buffer->AddString("\n");
-  }
-#else
-  UNREACHABLE();
-#endif
-}
 #endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
 
 const char* Bytecode::ToCString() const {

runtime/vm/object.h

@@ -7628,8 +7628,6 @@ class Bytecode : public Object {
     untag()->set_var_descriptors<std::memory_order_release>(value.ptr());
   }
 
-  void WriteLocalVariablesInfo(Zone* zone, BaseTextBuffer* buffer) const;
-
   // Will compute local var descriptors if necessary.
   LocalVarDescriptorsPtr GetLocalVarDescriptors() const;
 #endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)