[vm,dyn_modules] Initial handling of breakpoints in the interpreter.

Adds a set of new VMInternal_Breakpoint instructions, one for each
possible instruction size, and adds a bytecode_ and saved_opcode_
field to code breakpoints.

When enabling a breakpoint, the original opcode of the instruction is
replaced with the same-sized VmInternal_Breakpoint opcode and stored in
the saved_opcode_ field of the CodeBreakpoint. When disabling it, the
original opcode is replaced.

New labels are added to the dispatch loop for single stepping
purposes. Both the computed goto and switch dispatch versions
of the dispatch loop are appropriately altered to dispatch to
the single stepping labels instead of the original ones if single
stepping is currently enabled.

Fix up more parts of the debugger that assumed functions had Code
objects to handle functions with Bytecode objects as well. In
particular, instead of using the PcDescriptors to find safepoint
source locations in Bytecode objects, the source positions information
is used instead (since the PcDescriptors for Bytecode objects only
stores information about the start and end of try blocks at the moment).

Todo (from looking at the remaining failing tests):
* Handle async jumps.
* Handle coverage information.

TEST=now-passing tests from pkg/vm_service like
     pkg/vm_service/test/break_on_function_test

Change-Id: Icbd4b818e00508d9a4e74c81520aad2363b26d41
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
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444880
Reviewed-by: Alexander Markov <[email protected]>
Commit-Queue: Tess Strickland <[email protected]>

Author: sstrickl

runtime/vm/code_patcher.cc

@@ -3,6 +3,9 @@
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/code_patcher.h"
+#if defined(DART_DYNAMIC_MODULES)
+#include "vm/constants_kbc.h"
+#endif
 #include "vm/cpu.h"
 #include "vm/instructions.h"
 #include "vm/object.h"
@@ -61,4 +64,63 @@ bool MatchesPattern(uword end, const int16_t* pattern, intptr_t size) {
   return true;
 }
 
+#if !defined(PRODUCT) && defined(DART_DYNAMIC_MODULES)
+
+uint32_t BytecodePatcher::AddBreakpointAt(uword return_address,
+                                          const Bytecode& bytecode) {
+  auto thread = Thread::Current();
+  uint32_t old_opcode;
+  thread->isolate_group()->RunWithStoppedMutators([&]() {
+    old_opcode =
+        AddBreakpointAtWithMutatorsStopped(thread, return_address, bytecode);
+  });
+  return old_opcode;
+}
+
+void BytecodePatcher::RemoveBreakpointAt(uword return_address,
+                                         const Bytecode& bytecode,
+                                         uint32_t opcode) {
+  auto thread = Thread::Current();
+  thread->isolate_group()->RunWithStoppedMutators([&]() {
+    RemoveBreakpointAtWithMutatorsStopped(thread, return_address, bytecode,
+                                          opcode);
+  });
+}
+
+KBCInstr* GetInstructionBefore(const Bytecode& bytecode, uword return_address) {
+  ASSERT(bytecode.ContainsInstructionAt(return_address));
+  ASSERT(return_address != bytecode.PayloadStart());
+  uword prev = bytecode.PayloadStart();
+  uword current = KernelBytecode::Next(prev);
+  while (current < return_address) {
+    prev = current;
+    current = KernelBytecode::Next(prev);
+  }
+  ASSERT_EQUAL(current, return_address);
+  return reinterpret_cast<KBCInstr*>(prev);
+}
+
+uint32_t BytecodePatcher::AddBreakpointAtWithMutatorsStopped(
+    Thread* thread,
+    uword return_address,
+    const Bytecode& bytecode) {
+  auto* const instr = GetInstructionBefore(bytecode, return_address);
+  uint32_t old_opcode = *instr;
+  *instr = KernelBytecode::BreakpointOpcode(instr);
+  return old_opcode;
+}
+
+void BytecodePatcher::RemoveBreakpointAtWithMutatorsStopped(
+    Thread* thread,
+    uword return_address,
+    const Bytecode& bytecode,
+    uint32_t opcode) {
+  auto* const instr = GetInstructionBefore(bytecode, return_address);
+  // Must be previously enabled and not yet removed.
+  ASSERT(*instr == KernelBytecode::BreakpointOpcode(
+                       static_cast<KernelBytecode::Opcode>(opcode)));
+  *instr = opcode;
+}
+#endif  // !defined(PRODUCT) && defined(DART_DYNAMIC_MODULES)
+
 }  // namespace dart

runtime/vm/code_patcher.h

@@ -101,6 +101,32 @@ class CodePatcher : public AllStatic {
   static intptr_t GetSubtypeTestCachePoolIndex(uword return_address);
 };
 
+#if !defined(PRODUCT) && defined(DART_DYNAMIC_MODULES)
+class BytecodePatcher : public AllStatic {
+ public:
+  // Patch call instruction prior to return_address to add a breakpoint.
+  // Returns the original opcode of the patched instruction.
+  static uint32_t AddBreakpointAt(uword return_address,
+                                  const Bytecode& bytecode);
+
+  // Patch call instruction prior to return_address to remove a breakpoint.
+  // Replaces the breakpoint instruction opcode with the provided opcode.
+  static void RemoveBreakpointAt(uword return_address,
+                                 const Bytecode& code,
+                                 uint32_t opcode);
+
+ private:
+  static uint32_t AddBreakpointAtWithMutatorsStopped(Thread* thread,
+                                                     uword return_address,
+                                                     const Bytecode& bytecode);
+
+  static void RemoveBreakpointAtWithMutatorsStopped(Thread* thread,
+                                                    uword return_address,
+                                                    const Bytecode& bytecode,
+                                                    uint32_t opcode);
+};
+#endif  // !defined(PRODUCT) && defined(DART_DYNAMIC_MODULES)
+
 // Beginning from [end - size] we compare [size] bytes with [pattern]. All
 // [0..255] values in [pattern] have to match, negative values are skipped.
 //

runtime/vm/constants_kbc.h

@@ -225,6 +225,13 @@ namespace dart {
   V(VMInternal_ForwardDynamicInvocation,          0, ORDN, ___, ___, ___)      \
   V(VMInternal_ImplicitStaticClosure,             0, ORDN, ___, ___, ___)      \
   V(VMInternal_NoSuchMethodDispatcher,            0, ORDN, ___, ___, ___)      \
+  /* One breakpoint opcode for each instruction size. */                       \
+  V(VMInternal_Breakpoint_0,                      0, ORDN, ___, ___, ___)      \
+  V(VMInternal_Breakpoint_A_B_C,              A_B_C, ORDN, num, num, num)      \
+  V(VMInternal_Breakpoint_D,                      D, ORDN, num, ___, ___)      \
+  V(VMInternal_Breakpoint_D_Wide,                 D, WIDE, num, ___, ___)      \
+  V(VMInternal_Breakpoint_A_E,                  A_E, ORDN, num, num, ___)      \
+  V(VMInternal_Breakpoint_A_E_Wide,             A_E, WIDE, num, num, ___)      \
 
 #define INTERNAL_KERNEL_BYTECODES_LIST(V)                                      \
   INTERNAL_KERNEL_BYTECODES_WITH_CUSTOM_CODE(V)                                \
@@ -250,6 +257,7 @@ class KernelBytecode {
 #define DECLARE_BYTECODE(name, encoding, kind, op1, op2, op3) k##name,
     KERNEL_BYTECODES_LIST(DECLARE_BYTECODE)
 #undef DECLARE_BYTECODE
+        kNumOpcodes,
   };
 
   static const char* NameOf(Opcode op) {
@@ -274,7 +282,12 @@ class KernelBytecode {
 
   // Should be used only on instructions with wide variants.
   DART_FORCE_INLINE static bool IsWide(const KBCInstr* instr) {
-    return ((DecodeOpcode(instr) & kWideModifier) != 0);
+    return IsWide(DecodeOpcode(instr));
+  }
+
+  // Should be used only on instructions with wide variants.
+  DART_FORCE_INLINE static constexpr bool IsWide(Opcode opcode) {
+    return ((opcode & kWideModifier) != 0);
   }
 
  public:
@@ -423,7 +436,11 @@ class KernelBytecode {
   // - The bytecode generator emits a source position.
   // - The bytecode compiler may emit a DebugStepCheck call.
   DART_FORCE_INLINE static bool IsDebugCheckedOpcode(const KBCInstr* instr) {
-    switch (DecodeOpcode(instr)) {
+    return IsDebugCheckedOpcode(DecodeOpcode(instr));
+  }
+
+  DART_FORCE_INLINE static bool IsDebugCheckedOpcode(Opcode op) {
+    switch (op) {
       case KernelBytecode::kDebugCheck:
       case KernelBytecode::kDirectCall:
       case KernelBytecode::kDirectCall_Wide:
@@ -494,7 +511,42 @@ class KernelBytecode {
                                                 const KBCInstr** instructions,
                                                 intptr_t* instructions_size);
 
+  static Opcode BreakpointOpcode(Opcode opcode) {
+    Opcode replacement;
+    switch (kInstructionSize[opcode]) {
+      case 1:
+        replacement = kVMInternal_Breakpoint_0;
+        break;
+      case 2:
+        replacement = kVMInternal_Breakpoint_D;
+        break;
+      case 3:
+        replacement = kVMInternal_Breakpoint_A_E;
+        break;
+      case 4:
+        replacement = kVMInternal_Breakpoint_A_B_C;
+        break;
+      case 5:
+        replacement = kVMInternal_Breakpoint_D_Wide;
+        break;
+      case 6:
+        replacement = kVMInternal_Breakpoint_A_E_Wide;
+        break;
+      default:
+        UNREACHABLE();
+        return kTrap;
+    }
+    ASSERT_EQUAL(kInstructionSize[replacement], kInstructionSize[opcode]);
+    return replacement;
+  }
+
+  static Opcode BreakpointOpcode(const KBCInstr* instr) {
+    return BreakpointOpcode(DecodeOpcode(instr));
+  }
+
  private:
+  friend class Interpreter;  // for IsWide(Opcode) in static_asserts.
+
   DISALLOW_ALLOCATION();
   DISALLOW_IMPLICIT_CONSTRUCTORS(KernelBytecode);
 };