[vm/bytecode] Add DebugCheck bytecode instruction

DebugCheck bytecode instruction is generated after parameter variables are declared and copied into their locations in the prologue. It helps debugger to stop in the beginning of a function at the point where parameters can be inspected. It is generated only if '--bytecode-options=debugger-stops' is specified. Change-Id: I0f3b1ea8dc45d762a5dcee75b5d3a4ffc0b2a1b1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108371 Reviewed-by: Ryan Macnak <[email protected]> Reviewed-by: Régis Crelier <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
dart-lang · Jul 9, 2019 · 9503969 · 9503969
1 parent fc542be
commit 9503969
Show file tree

Hide file tree

Showing 12 changed files with 62 additions and 26 deletions.
diff --git a/pkg/vm/bin/kernel_service.dart b/pkg/vm/bin/kernel_service.dart
@@ -146,8 +146,8 @@ abstract class Compiler {
 
       if (options.bytecode && errors.isEmpty) {
         await runWithFrontEndCompilerContext(script, options, component, () {
-          // TODO(alexmarkov): disable source positions, local variables info
-          //  and source files in VM PRODUCT mode.
+          // TODO(alexmarkov): disable source positions, local variables info,
+          //  debugger stops and source files in VM PRODUCT mode.
           // TODO(alexmarkov): disable asserts if they are not enabled in VM.
           // TODO(rmacnak): disable annotations if mirrors are not enabled.
           generateBytecode(component,
@@ -156,6 +156,7 @@ abstract class Compiler {
                   environmentDefines: options.environmentDefines,
                   emitSourcePositions: true,
                   emitLocalVarInfo: true,
+                  emitDebuggerStops: true,
                   emitSourceFiles: true,
                   emitAnnotations: true));
         });

diff --git a/pkg/vm/lib/bytecode/assembler.dart b/pkg/vm/lib/bytecode/assembler.dart
@@ -513,6 +513,11 @@ class BytecodeAssembler {
     _emitInstructionA(Opcode.kCheckStack, ra);
   }
 
+  void emitDebugCheck() {
+    emitSourcePosition();
+    _emitInstruction0(Opcode.kDebugCheck);
+  }
+
   void emitCheckFunctionTypeArgs(int ra, int re) {
     emitSourcePosition();
     _emitInstructionAE(Opcode.kCheckFunctionTypeArgs, ra, re);

diff --git a/pkg/vm/lib/bytecode/dbc.dart b/pkg/vm/lib/bytecode/dbc.dart
@@ -10,7 +10,7 @@ library vm.bytecode.dbc;
 /// Before bumping current bytecode version format, make sure that
 /// all users have switched to a VM which is able to consume new
 /// version of bytecode.
-const int currentBytecodeFormatVersion = 12;
+const int currentBytecodeFormatVersion = 13;
 
 enum Opcode {
   kUnusedOpcode000,
@@ -99,8 +99,6 @@ enum Opcode {
   kUnusedOpcode083,
   kUnusedOpcode084,
 
-  // Bytecode instructions since bytecode format v7:
-
   kTrap,
 
   // Prologue and stack management.
@@ -117,7 +115,7 @@ enum Opcode {
   kCheckFunctionTypeArgs,
   kCheckFunctionTypeArgs_Wide,
   kCheckStack,
-  kUnused01,
+  kDebugCheck,
   kUnused02, // Reserved for CheckParameterTypes
   kUnused03, // Reserved for CheckParameterTypes_Wide
 
@@ -370,6 +368,8 @@ const Map<Opcode, Format> BytecodeFormats = const {
       Encoding.kAE, const [Operand.imm, Operand.reg, Operand.none]),
   Opcode.kCheckStack: const Format(
       Encoding.kA, const [Operand.imm, Operand.none, Operand.none]),
+  Opcode.kDebugCheck: const Format(
+      Encoding.k0, const [Operand.none, Operand.none, Operand.none]),
   Opcode.kAllocate: const Format(
       Encoding.kD, const [Operand.lit, Operand.none, Operand.none]),
   Opcode.kAllocateT: const Format(

diff --git a/pkg/vm/lib/bytecode/gen_bytecode.dart b/pkg/vm/lib/bytecode/gen_bytecode.dart
@@ -1534,12 +1534,6 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
     } else {
       asm.emitEntry(locals.frameSize);
     }
-    // TODO(alexmarkov): Introduce a new bytecode triggering a debug check in
-    // the interpreter. Its token position should correspond to the declaration
-    // position of the last parameter, which the debugger can inspect at the
-    // point of the debug check.
-    // TODO(regis): Support the new bytecode in the interpreter and dissociate
-    // the debug check from the CheckStack bytecode.
     asm.emitCheckStack(0);
 
     if (isClosure) {
@@ -1666,6 +1660,23 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
       function.positionalParameters.forEach(_copyParamIfCaptured);
       locals.sortedNamedParameters.forEach(_copyParamIfCaptured);
     }
+
+    if (options.emitDebuggerStops) {
+      // DebugCheck instruction should be emitted after parameter variables
+      // are declared and copied into context.
+      // The debugger expects the source position to correspond to the
+      // declaration position of the last parameter, if any, or of the function.
+      if (options.emitSourcePositions && function != null) {
+        var pos = function.fileOffset;
+        if (function.namedParameters.isNotEmpty) {
+          pos = function.namedParameters.last.fileOffset;
+        } else if (function.positionalParameters.isNotEmpty) {
+          pos = function.positionalParameters.last.fileOffset;
+        }
+        _recordSourcePosition(pos);
+      }
+      asm.emitDebugCheck();
+    }
   }
 
   void _copyParamIfCaptured(VariableDeclaration variable) {

diff --git a/pkg/vm/lib/bytecode/options.dart b/pkg/vm/lib/bytecode/options.dart
@@ -9,6 +9,7 @@ class BytecodeOptions {
   static Map<String, String> commandLineFlags = {
     'annotations': 'Emit Dart annotations',
     'local-var-info': 'Emit debug information about local variables',
+    'debugger-stops': 'Emit bytecode instructions for stopping in the debugger',
     'show-bytecode-size-stat': 'Show bytecode size breakdown',
     'source-positions': 'Emit source positions',
   };
@@ -18,6 +19,7 @@ class BytecodeOptions {
   bool emitSourcePositions;
   bool emitSourceFiles;
   bool emitLocalVarInfo;
+  bool emitDebuggerStops;
   bool emitAnnotations;
   bool omitAssertSourcePositions;
   bool showBytecodeSizeStatistics;
@@ -29,6 +31,7 @@ class BytecodeOptions {
       this.emitSourcePositions = false,
       this.emitSourceFiles = false,
       this.emitLocalVarInfo = false,
+      this.emitDebuggerStops = false,
       this.emitAnnotations = false,
       this.omitAssertSourcePositions = false,
       this.showBytecodeSizeStatistics = false,
@@ -49,6 +52,9 @@ class BytecodeOptions {
         case 'local-var-info':
           emitLocalVarInfo = true;
           break;
+        case 'debugger-stops':
+          emitDebuggerStops = true;
+          break;
         case 'annotations':
           emitAnnotations = true;
           break;

diff --git a/runtime/vm/compiler/frontend/base_flow_graph_builder.cc b/runtime/vm/compiler/frontend/base_flow_graph_builder.cc
@@ -873,6 +873,15 @@ Fragment BaseFlowGraphBuilder::BuildFfiAsFunctionInternalCall(
   return code;
 }
 
+Fragment BaseFlowGraphBuilder::DebugStepCheck(TokenPosition position) {
+#ifdef PRODUCT
+  return Fragment();
+#else
+  return Fragment(new (Z) DebugStepCheckInstr(
+      position, RawPcDescriptors::kRuntimeCall, GetNextDeoptId()));
+#endif
+}
+
 }  // namespace kernel
 }  // namespace dart
 

diff --git a/runtime/vm/compiler/frontend/base_flow_graph_builder.h b/runtime/vm/compiler/frontend/base_flow_graph_builder.h
@@ -301,6 +301,8 @@ class BaseFlowGraphBuilder {
                           const Class& klass,
                           intptr_t argument_count);
 
+  Fragment DebugStepCheck(TokenPosition position);
+
  protected:
   intptr_t AllocateBlockId() { return ++last_used_block_id_; }
 

diff --git a/runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc b/runtime/vm/compiler/frontend/bytecode_flow_graph_builder.cc
@@ -730,6 +730,13 @@ void BytecodeFlowGraphBuilder::BuildCheckStack() {
   }
 }
 
+void BytecodeFlowGraphBuilder::BuildDebugCheck() {
+  if (is_generating_interpreter()) {
+    UNIMPLEMENTED();  // TODO(alexmarkov): interpreter
+  }
+  code_ += B->DebugStepCheck(position_);
+}
+
 void BytecodeFlowGraphBuilder::BuildPushConstant() {
   PushConstant(ConstantAt(DecodeOperandD()));
 }

diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc
@@ -1220,15 +1220,6 @@ bool FlowGraphBuilder::NeedsDebugStepCheck(Value* value,
   return definition->IsLoadLocal();
 }
 
-Fragment FlowGraphBuilder::DebugStepCheck(TokenPosition position) {
-#ifdef PRODUCT
-  return Fragment();
-#else
-  return Fragment(new (Z) DebugStepCheckInstr(
-      position, RawPcDescriptors::kRuntimeCall, GetNextDeoptId()));
-#endif
-}
-
 Fragment FlowGraphBuilder::EvaluateAssertion() {
   const Class& klass =
       Class::ZoneHandle(Z, Library::LookupCoreClass(Symbols::AssertionError()));

diff --git a/runtime/vm/compiler/frontend/kernel_to_il.h b/runtime/vm/compiler/frontend/kernel_to_il.h
@@ -212,7 +212,6 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
 
   bool NeedsDebugStepCheck(const Function& function, TokenPosition position);
   bool NeedsDebugStepCheck(Value* value, TokenPosition position);
-  Fragment DebugStepCheck(TokenPosition position);
 
   // Truncates (instead of deoptimizing) if the origin does not fit into the
   // target representation.

diff --git a/runtime/vm/constants_kbc.h b/runtime/vm/constants_kbc.h
@@ -570,7 +570,7 @@ namespace dart {
   V(CheckFunctionTypeArgs,               A_E, ORDN, num, reg, ___)             \
   V(CheckFunctionTypeArgs_Wide,          A_E, WIDE, num, reg, ___)             \
   V(CheckStack,                            A, ORDN, num, ___, ___)             \
-  V(Unused01,                              0, RESV, ___, ___, ___)             \
+  V(DebugCheck,                            0, ORDN, ___, ___, ___)             \
   V(Unused02,                              0, RESV, ___, ___, ___)             \
   V(Unused03,                              0, RESV, ___, ___, ___)             \
   V(Allocate,                              D, ORDN, lit, ___, ___)             \
@@ -749,7 +749,7 @@ class KernelBytecode {
   // Maximum bytecode format version supported by VM.
   // The range of supported versions should include version produced by bytecode
   // generator (currentBytecodeFormatVersion in pkg/vm/lib/bytecode/dbc.dart).
-  static const intptr_t kMaxSupportedBytecodeFormatVersion = 12;
+  static const intptr_t kMaxSupportedBytecodeFormatVersion = 13;
 
   enum Opcode {
 #define DECLARE_BYTECODE(name, encoding, kind, op1, op2, op3) k##name,
@@ -969,7 +969,7 @@ class KernelBytecode {
       case KernelBytecode::kStoreLocal_Wide:
       case KernelBytecode::kStoreStaticTOS:
       case KernelBytecode::kStoreStaticTOS_Wide:
-      case KernelBytecode::kCheckStack:
+      case KernelBytecode::kDebugCheck:
       case KernelBytecode::kDirectCall:
       case KernelBytecode::kDirectCall_Wide:
       case KernelBytecode::kInterfaceCall:

diff --git a/runtime/vm/interpreter.cc b/runtime/vm/interpreter.cc
@@ -1750,7 +1750,6 @@ RawObject* Interpreter::Call(RawFunction* function,
 
   {
     BYTECODE(CheckStack, A);
-    DEBUG_CHECK;
     {
       // Check the interpreter's own stack limit for actual interpreter's stack
       // overflows, and also the thread's stack limit for scheduled interrupts.
@@ -1775,6 +1774,12 @@ RawObject* Interpreter::Call(RawFunction* function,
     DISPATCH();
   }
 
+  {
+    BYTECODE(DebugCheck, 0);
+    DEBUG_CHECK;
+    DISPATCH();
+  }
+
   {
     BYTECODE(CheckFunctionTypeArgs, A_E);
     const intptr_t declared_type_args_len = rA;