[vm/bytecode] Ignore source positions not denoting a debug point in debugger.

crelier · commit-bot@chromium.org · commit 55b67e9eae9a · 2019-07-12T00:18:21.000Z
The CheckStack bytecode does not denote a debug point anymore after the introduction of the DebugCheck bytecode. Fix caller/callee determination in mixed mode. Factorize some code. Change-Id: Ide1e0bbad022a83e6113243dc996396f9f5d2f3c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108565 Commit-Queue: Régis Crelier <regis@google.com> Reviewed-by: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/vm/constants_kbc.h b/runtime/vm/constants_kbc.h
@@ -959,8 +959,7 @@ class KernelBytecode {
   // The interpreter checks for a debug break at each instruction with listed
   // opcode and the bytecode generator emits a source position at each
   // instruction with listed opcode.
-  DART_FORCE_INLINE static bool IsDebugBreakCheckedOpcode(
-      const KBCInstr* instr) {
+  DART_FORCE_INLINE static bool IsDebugCheckedOpcode(const KBCInstr* instr) {
     switch (DecodeOpcode(instr)) {
       case KernelBytecode::kAllocate:
       case KernelBytecode::kPopLocal:
diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc
diff --git a/runtime/vm/debugger.h b/runtime/vm/debugger.h
@@ -309,6 +309,14 @@ class ActivationFrame : public ZoneAllocated {
   }
   bool IsInterpreted() const { return !bytecode_.IsNull(); }
 
+  enum Relation {
+    kCallee,
+    kSelf,
+    kCaller,
+  };
+
+  Relation CompareTo(uword other_fp, bool other_is_interpreted) const;
+
   RawString* QualifiedFunctionName();
   RawString* SourceUrl();
   RawScript* SourceScript();
@@ -821,8 +829,10 @@ class Debugger {
   // frame corresponds to this fp value, or if the top frame is
   // lower on the stack.
   uword stepping_fp_;
+  bool interpreted_stepping_;
   // Used to track the current async/async* function.
   uword async_stepping_fp_;
+  bool interpreted_async_stepping_;
   RawObject* top_frame_awaiter_;
 
   // If we step while at a breakpoint, we would hit the same pc twice.
diff --git a/runtime/vm/interpreter.cc b/runtime/vm/interpreter.cc
@@ -1146,7 +1146,7 @@ DART_FORCE_INLINE bool Interpreter::InstanceCall2(Thread* thread,
 #define DEBUG_CHECK
 #else
 // The DEBUG_CHECK macro must only be called from bytecodes listed in
-// KernelBytecode::IsDebugBreakCheckedOpcode.
+// KernelBytecode::IsDebugCheckedOpcode.
 #define DEBUG_CHECK                                                            \
   if (is_debugging()) {                                                        \
     /* Check for debug breakpoint or if single stepping. */                    \
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
@@ -15395,6 +15395,26 @@ intptr_t Bytecode::GetTryIndexAtPc(uword return_address) const {
 #endif
 }
 
+uword Bytecode::GetDebugCheckedOpcodePc(uword from_offset,
+                                        uword to_offset) const {
+#if defined(DART_PRECOMPILED_RUNTIME)
+  UNREACHABLE();
+#else
+  uword pc = PayloadStart() + from_offset;
+  uword end_pc = pc + (to_offset - from_offset);
+  while (pc < end_pc) {
+    uword next_pc = KernelBytecode::Next(pc);
+    if (KernelBytecode::IsDebugCheckedOpcode(
+            reinterpret_cast<const KBCInstr*>(pc))) {
+      // Return the pc after the opcode, i.e. its 'return address'.
+      return next_pc;
+    }
+    pc = next_pc;
+  }
+  return 0;
+#endif
+}
+
 const char* Bytecode::ToCString() const {
   return Thread::Current()->zone()->PrintToString("Bytecode(%s)",
                                                   QualifiedName());
@@ -15440,6 +15460,16 @@ const char* Bytecode::QualifiedName() const {
   return zone->PrintToString("[Bytecode] %s", function_name);
 }
 
+const char* Bytecode::FullyQualifiedName() const {
+  Zone* zone = Thread::Current()->zone();
+  const Function& fun = Function::Handle(zone, function());
+  if (fun.IsNull()) {
+    return BytecodeStubName(*this);
+  }
+  const char* function_name = fun.ToFullyQualifiedCString();
+  return zone->PrintToString("[Bytecode] %s", function_name);
+}
+
 bool Bytecode::SlowFindRawBytecodeVisitor::FindObject(
     RawObject* raw_obj) const {
   return RawBytecode::ContainsPC(raw_obj, pc_);
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
@@ -5729,6 +5729,9 @@ class Bytecode : public Object {
   TokenPosition GetTokenIndexOfPC(uword return_address) const;
   intptr_t GetTryIndexAtPc(uword return_address) const;
 
+  // Return the pc after the first 'debug checked' opcode in the range.
+  uword GetDebugCheckedOpcodePc(uword from_offset, uword to_offset) const;
+
   intptr_t instructions_binary_offset() const {
     return raw_ptr()->instructions_binary_offset_;
   }
@@ -5780,6 +5783,7 @@ class Bytecode : public Object {
 
   const char* Name() const;
   const char* QualifiedName() const;
+  const char* FullyQualifiedName() const;
 
   class SlowFindRawBytecodeVisitor : public FindObjectVisitor {
    public:
diff --git a/runtime/vm/source_report.cc b/runtime/vm/source_report.cc
@@ -340,13 +340,29 @@ void SourceReport::PrintPossibleBreakpointsData(JSONObject* jsobj,
     const Bytecode& bytecode = Bytecode::Handle(func.bytecode());
     ASSERT(!bytecode.IsNull());
     kernel::BytecodeSourcePositionsIterator iter(zone(), bytecode);
+    intptr_t token_offset = -1;
+    uword pc_offset = kUwordMax;
+    // TODO(regis): We should ignore all possible breakpoint positions until
+    // the first DebugCheck opcode of the function.
     while (iter.MoveNext()) {
+      if (pc_offset != kUwordMax) {
+        // Check that there is at least one 'debug checked' opcode in the last
+        // source position range.
+        if (bytecode.GetDebugCheckedOpcodePc(pc_offset, iter.PcOffset()) != 0) {
+          possible[token_offset] = true;
+        }
+        pc_offset = kUwordMax;
+      }
       const TokenPosition token_pos = iter.TokenPos();
       if ((token_pos < begin_pos) || (token_pos > end_pos)) {
         // Does not correspond to a valid source position.
         continue;
       }
-      intptr_t token_offset = token_pos.Pos() - begin_pos.Pos();
+      pc_offset = iter.PcOffset();
+      token_offset = token_pos.Pos() - begin_pos.Pos();
+    }
+    if (pc_offset != kUwordMax &&
+        bytecode.GetDebugCheckedOpcodePc(pc_offset, bytecode.Size()) != 0) {
       possible[token_offset] = true;
     }
   } else {
diff --git a/runtime/vm/stack_frame.cc b/runtime/vm/stack_frame.cc
@@ -184,7 +184,7 @@ const char* StackFrame::ToCString() const {
                                  " offset:0x%" Px ") %s ]",
                                  GetName(), sp(), fp(), pc(),
                                  pc() - bytecode.PayloadStart(),
-                                 bytecode.Name());
+                                 bytecode.FullyQualifiedName());
     }
     const Code& code = Code::Handle(zone, LookupDartCode());
     ASSERT(!code.IsNull());
diff --git a/runtime/vm/stack_frame.h b/runtime/vm/stack_frame.h
@@ -456,10 +456,18 @@ DART_FORCE_INLINE static uword ParamAddress(uword fp, intptr_t reverse_index) {
   return fp + (kParamEndSlotFromFp * kWordSize) + (reverse_index * kWordSize);
 }
 
+// Both fp and other_fp are compiled code frame pointers.
+// See stack_frame_dbc.h for the DBC version.
 DART_FORCE_INLINE static bool IsCalleeFrameOf(uword fp, uword other_fp) {
   return other_fp < fp;
 }
 
+// Both fp and other_fp are bytecode frame pointers.
+DART_FORCE_INLINE static bool IsBytecodeCalleeFrameOf(uword fp,
+                                                      uword other_fp) {
+  return other_fp > fp;
+}
+
 // Value for stack limit that is used to cause an interrupt.
 // Note that on DBC stack is growing upwards so interrupt limit is 0 unlike
 // on all other architectures.
diff --git a/runtime/vm/stack_frame_dbc.h b/runtime/vm/stack_frame_dbc.h
@@ -74,6 +74,12 @@ DART_FORCE_INLINE static bool IsCalleeFrameOf(uword fp, uword other_fp) {
   return other_fp > fp;
 }
 
+DART_FORCE_INLINE static bool IsBytecodeCalleeFrameOf(uword fp,
+                                                      uword other_fp) {
+  UNREACHABLE();
+  return false;
+}
+
 static const int kExitLinkSlotFromEntryFp = 0;
 
 // Value for stack limit that is used to cause an interrupt.

Original file line number	Diff line number	Diff line change
`@@ -184,7 +184,7 @@ const char* StackFrame::ToCString() const {`
`184`	`184`	`" offset:0x%" Px ") %s ]",`
`185`	`185`	`GetName(), sp(), fp(), pc(),`
`186`	`186`	`pc() - bytecode.PayloadStart(),`
`187`		`- bytecode.Name());`
	`187`	`+ bytecode.FullyQualifiedName());`
`188`	`188`	`}`
`189`	`189`	`const Code& code = Code::Handle(zone, LookupDartCode());`
`190`	`190`	`ASSERT(!code.IsNull());`