Return past breakpoints on a call

Summary:
Imported from static_h
Original Author: [email protected]
Original Git: b623a8d
Original Reviewed By: dannysu
Original Revision: D63920292

The interpreter inspects the IP up the stack to determine where to
return to, so check whether there's a breakpoint there before doing
`nextInstCall`. The check is now faster because we maintain a count of
how many installed breakpoints each `CodeBlock` has when the debugger is
enabled and check that before doing the more expensive map lookup on
every Ret.

Reviewed By: avp

Differential Revision: D64125238

fbshipit-source-id: 3bf400146c76d6d6cb715b8973ec6dea289f317f

include/hermes/VM/CodeBlock.h

@@ -51,6 +51,11 @@ class CodeBlock final
   /// ID of this function in the module's function list.
   uint32_t functionID_;
 
+#ifdef HERMES_ENABLE_DEBUGGER
+  /// The number of breakpoints currently installed in this function.
+  uint32_t numInstalledBreakpoints_ = 0;
+#endif
+
   /// Total size of the property cache.
   const uint32_t propertyCacheSize_;
 
@@ -302,6 +307,11 @@ class CodeBlock final
   /// Requires that a breakpoint has been set at \p offset.
   /// Decrements the user count of the associated runtime module.
   void uninstallBreakpointAtOffset(uint32_t offset, uint8_t opCode);
+
+  /// \return the number of breakpoints installed in this code block.
+  uint32_t getNumInstalledBreakpoints() const {
+    return numInstalledBreakpoints_;
+  }
 #endif
 };
 

include/hermes/VM/Debugger/Debugger.h

@@ -408,6 +408,10 @@ class Debugger {
   ScriptID resolveScriptId(RuntimeModule *runtimeModule, uint32_t filenameId)
       const;
 
+  /// Get the actual OpCode produced from the source without being affected by
+  /// any user installed breakpoint "Debugger" OpCode overrides.
+  inst::OpCode getRealOpCode(CodeBlock *block, uint32_t offset) const;
+
  private:
   /// The primary debugger command loop.
   ExecutionStatus debuggerLoop(
@@ -557,10 +561,6 @@ class Debugger {
 
   /// Set breakpoints at all possible next instructions after the current one.
   void breakAtPossibleNextInstructions(const InterpreterState &state);
-
-  /// Get the actual OpCode produced from the source without being affected by
-  /// any user installed breakpoint "Debugger" OpCode overrides.
-  inst::OpCode getRealOpCode(CodeBlock *block, uint32_t offset) const;
 };
 
 } // namespace vm

lib/VM/CodeBlock.cpp

@@ -424,6 +424,7 @@ void CodeBlock::installBreakpointAtOffset(uint32_t offset) {
 
   makeWritable(address, sizeof(inst::DebuggerInst));
   *address = debuggerOpcode;
+  ++numInstalledBreakpoints_;
 }
 
 void CodeBlock::uninstallBreakpointAtOffset(
@@ -440,6 +441,7 @@ void CodeBlock::uninstallBreakpointAtOffset(
   // This is valid because we can only uninstall breakpoints that we installed.
   // Therefore, the page here must be writable.
   *address = opCode;
+  --numInstalledBreakpoints_;
 }
 
 #endif

lib/VM/Interpreter.cpp

@@ -1818,7 +1818,23 @@ CallResult<HermesValue> Interpreter::interpretFunction(
 
         INIT_STATE_FOR_CODEBLOCK(curCodeBlock);
         O1REG(Call) = res.getValue();
+
+#ifdef HERMES_ENABLE_DEBUGGER
+        // Only do the more expensive check for breakpoint location (in
+        // getRealOpCode) if there are breakpoints installed in the function
+        // we're returning into.
+        if (LLVM_UNLIKELY(curCodeBlock->getNumInstalledBreakpoints() > 0)) {
+          ip = IPADD(inst::getInstSize(
+              runtime.debugger_.getRealOpCode(curCodeBlock, CUROFFSET)));
+        } else {
+          // No breakpoints in the function being returned to, just use
+          // nextInstCall().
+          ip = nextInstCall(ip);
+        }
+#else
         ip = nextInstCall(ip);
+#endif
+
         DISPATCH;
       }
 

test/debugger/ret-breakpoint.js

@@ -0,0 +1,29 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hdb %s < %s.debug | %FileCheckOrRegen --match-full-lines %s
+// REQUIRES: debugger
+
+function foo() {
+  debugger;
+  // Ret has to check the actual opcode at the callsite,
+  // which means it has to account for breakpoints being installed there.
+  return 1;
+}
+
+function main(f) {
+  return f();
+}
+
+print(main(foo));
+
+// Auto-generated content below. Please do not modify manually.
+
+// CHECK:Break on 'debugger' statement in foo: {{.*}}ret-breakpoint.js[2]:12:3
+// CHECK-NEXT:Set breakpoint 1 at {{.*}}ret-breakpoint.js[2]:19:11
+// CHECK-NEXT:Continuing execution
+// CHECK-NEXT:1

test/debugger/ret-breakpoint.js.debug

@@ -0,0 +1,2 @@
+break 19 11
+continue