From ed8fb402c005d9a313c0f2fac6b134db57d5335e Mon Sep 17 00:00:00 2001 From: Mike Magruder Date: Mon, 22 Apr 2013 10:40:42 -0700 Subject: [PATCH] Improve the Next command to not interpret and single-step ever line under calls from the original source line This improves both Next and Out to avoid interpreting and stepping everything between when they start and finish. Out now lets the program run free until a pseudo-breakpoint at the return site it hit. Next continues to single-step the source line being stepped over, but now lets the program run free under calls made from that line. The logic in Next regarding "calls made from that line" is extremely generic. We don't look at, say, call opcodes and decide to do something special. Rather, when we find we're off the original source line and a frame deeper we setup a "step out" operation much like the Out command then let the program run free. When we reach our return point, we continue stepping like normal. This accounts for not just calls, but iterators, and anything else that causes more PHP to run under the original source line. This change moves the flow control logic down in to the respective cmds: Next, Step, Out, Continue. These cmds get a crack at executing at various points in the interrupt/command processing path. These cmds now own setting up the last location filter, whether they need VM interrupts, and whether they're done or not. --- hphp/runtime/base/execution_context.h | 2 + hphp/runtime/eval/debugger/break_point.h | 2 +- .../eval/debugger/cmd/cmd_continue.cpp | 14 ++ hphp/runtime/eval/debugger/cmd/cmd_continue.h | 2 + .../eval/debugger/cmd/cmd_flow_control.cpp | 63 +++++- .../eval/debugger/cmd/cmd_flow_control.h | 64 ++++-- hphp/runtime/eval/debugger/cmd/cmd_next.cpp | 60 ++++++ hphp/runtime/eval/debugger/cmd/cmd_next.h | 2 + hphp/runtime/eval/debugger/cmd/cmd_out.cpp | 45 ++++ hphp/runtime/eval/debugger/cmd/cmd_out.h | 2 + hphp/runtime/eval/debugger/cmd/cmd_step.cpp | 20 ++ hphp/runtime/eval/debugger/cmd/cmd_step.h | 2 + hphp/runtime/eval/debugger/debugger.cpp | 8 +- hphp/runtime/eval/debugger/debugger_base.cpp | 12 +- .../eval/debugger/debugger_command.cpp | 13 +- hphp/runtime/eval/debugger/debugger_proxy.cpp | 194 +++++------------- hphp/runtime/eval/debugger/debugger_proxy.h | 14 +- hphp/runtime/vm/bytecode.cpp | 34 +++ hphp/runtime/vm/debugger_hook.cpp | 60 ++++-- hphp/runtime/vm/debugger_hook.h | 15 +- hphp/test/test_debugger.cpp | 1 + 21 files changed, 437 insertions(+), 192 deletions(-) diff --git a/hphp/runtime/base/execution_context.h b/hphp/runtime/base/execution_context.h index ea4d838e1..6fd773cb2 100644 --- a/hphp/runtime/base/execution_context.h +++ b/hphp/runtime/base/execution_context.h @@ -628,6 +628,7 @@ public: void evalPHPDebugger(TypedValue* retval, StringData *code, int frame); void enterDebuggerDummyEnv(); void exitDebuggerDummyEnv(); + void preventReturnsToTC(); void destructObjects(); int m_lambdaCounter; struct ReentryRecord { @@ -688,6 +689,7 @@ private: bool prepareArrayArgs(ActRec* ar, ArrayData* args, ExtraArgs*& extraArgs); void recordCodeCoverage(PC pc); + bool isReturnHelper(uintptr_t address); int m_coverPrevLine; HPHP::VM::Unit* m_coverPrevUnit; Array m_evaledArgs; diff --git a/hphp/runtime/eval/debugger/break_point.h b/hphp/runtime/eval/debugger/break_point.h index 43de6d0de..552d128fc 100644 --- a/hphp/runtime/eval/debugger/break_point.h +++ b/hphp/runtime/eval/debugger/break_point.h @@ -29,7 +29,7 @@ enum InterruptType { RequestEnded, PSPEnded, HardBreakPoint, // From f_hphpd_break(). - BreakPointReached, + BreakPointReached, // Break from the VM interpreter loop ExceptionThrown, }; diff --git a/hphp/runtime/eval/debugger/cmd/cmd_continue.cpp b/hphp/runtime/eval/debugger/cmd/cmd_continue.cpp index 657e3d6f4..12afae1d8 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_continue.cpp +++ b/hphp/runtime/eval/debugger/cmd/cmd_continue.cpp @@ -32,5 +32,19 @@ bool CmdContinue::help(DebuggerClient *client) { return true; } +void CmdContinue::onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt) { + assert(!m_complete); // Complete cmds should not be asked to do work. + CmdFlowControl::onSetup(proxy, interrupt); + // If there's a remaining count on this cmd then we want it left installed + // in the proxy. + m_complete = (decCount() == 0); +} + +void CmdContinue::onBeginInterrupt(DebuggerProxy *proxy, + CmdInterrupt &interrupt) { + assert(!m_complete); // Complete cmds should not be asked to do work. + m_complete = (decCount() == 0); +} + /////////////////////////////////////////////////////////////////////////////// }} diff --git a/hphp/runtime/eval/debugger/cmd/cmd_continue.h b/hphp/runtime/eval/debugger/cmd/cmd_continue.h index 203eb29f8..1e1f797df 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_continue.h +++ b/hphp/runtime/eval/debugger/cmd/cmd_continue.h @@ -28,6 +28,8 @@ public: CmdContinue() : CmdFlowControl(KindOfContinue) {} virtual bool help(DebuggerClient *client); + virtual void onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt); + virtual void onBeginInterrupt(DebuggerProxy *proxy, CmdInterrupt &interrupt); }; /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/eval/debugger/cmd/cmd_flow_control.cpp b/hphp/runtime/eval/debugger/cmd/cmd_flow_control.cpp index 70bb72fcd..dff62dec9 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_flow_control.cpp +++ b/hphp/runtime/eval/debugger/cmd/cmd_flow_control.cpp @@ -15,10 +15,18 @@ */ #include +#include namespace HPHP { namespace Eval { /////////////////////////////////////////////////////////////////////////////// +TRACE_SET_MOD(debugger); + +CmdFlowControl::~CmdFlowControl() { + // Remove any location filter that may have been setup by this cmd. + removeLocationFilter(); +} + void CmdFlowControl::sendImpl(DebuggerThriftBuffer &thrift) { DebuggerCommand::sendImpl(thrift); thrift.write(m_count); @@ -59,9 +67,62 @@ bool CmdFlowControl::onClient(DebuggerClient *client) { } bool CmdFlowControl::onServer(DebuggerProxy *proxy) { + // Flow control cmds do their work in onSetup() and onBeginInterrupt(), so + // there is no real work to do in here. + return true; +} + +void CmdFlowControl::onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt) { // Should only do setting and nothing else g_context->setDebuggerSmallStep(m_smallStep); - return true; +} + +// Setup the last location filter on the VM context for all offsets covered by +// the current source line. This will short-circuit the work done in +// phpDebuggerOpcodeHook() and ensure we don't interrupt on this source line. +void CmdFlowControl::installLocationFilterForLine(InterruptSite *site) { + if (!site) return; // We may be stopped at a place with not source info. + if (g_vmContext->m_lastLocFilter) { + g_vmContext->m_lastLocFilter->clear(); + } else { + g_vmContext->m_lastLocFilter = new VM::PCFilter(); + } + auto offsets = site->getCurOffsetRange(); + if (Trace::moduleEnabled(Trace::debugger, 5)) { + Trace::trace("prepare source loc filter\n"); + for (auto it = offsets.begin(); + it != offsets.end(); ++it) { + Trace::trace("block source loc in %s:%d: unit %p offset [%d, %d)\n", + site->getFile(), site->getLine0(), + site->getUnit(), it->m_base, it->m_past); + } + } + g_vmContext->m_lastLocFilter->addRanges(site->getUnit(), offsets); +} + +void CmdFlowControl::removeLocationFilter() { + if (g_vmContext->m_lastLocFilter) { + delete g_vmContext->m_lastLocFilter; + g_vmContext->m_lastLocFilter = nullptr; + } +} + +void CmdFlowControl::setupStepOut() { + ActRec *fp = g_vmContext->getFP(); + assert(fp); + ActRec *fpPrev = g_vmContext->getPrevVMState(fp, &m_stepOutOffset); + assert(fpPrev); + TRACE(2, "CmdFlowControl: step out to offset %d\n", m_stepOutOffset); + m_stepOutUnit = fpPrev->m_func->unit(); + phpAddBreakPoint(m_stepOutUnit, m_stepOutOffset); +} + +void CmdFlowControl::cleanupStepOut() { + if (m_stepOutUnit) { + phpRemoveBreakPoint(m_stepOutUnit, m_stepOutOffset); + m_stepOutUnit = nullptr; + m_stepOutOffset = 0; + } } /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/eval/debugger/cmd/cmd_flow_control.h b/hphp/runtime/eval/debugger/cmd/cmd_flow_control.h index eee2f78e7..f788aa19b 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_flow_control.h +++ b/hphp/runtime/eval/debugger/cmd/cmd_flow_control.h @@ -18,20 +18,34 @@ #define incl_HPHP_EVAL_DEBUGGER_CMD_FLOW_CONTROL_H_ #include +#include namespace HPHP { namespace Eval { /////////////////////////////////////////////////////////////////////////////// +// CmdFlowControl is the base class of all "flow control" cmds: Continue, Step, +// Next, and Out. A DebuggerProxy can execute exactly one of these cmds at a +// time. +// +// 1. Proxy receives a flow command (say, Next). +// 2. Proxy calls the cmd's onSetup() with current source position. +// 3. Cmd sets up state in the VM to control stepping, etc. +// 4. If the cmd is complete the proxy deletes the cmd, otherwise it saves it +// in m_flow. +// 5. Proxy lets the thread continue execution. +// 6. Proxy receives an interrupt from the VM, delivers it to the cmd in m_flow. +// 7. Cmd inspects the site and VM state, and either decides it is done, or +// sets up state in the VM to continue the control flow operation. +// 8. If the cmd is complete the proxy stops and waits for more input from the +// client. Otherwise, rinse and repeat at step 5. DECLARE_BOOST_TYPES(CmdFlowControl); class CmdFlowControl : public DebuggerCommand { public: explicit CmdFlowControl(Type type) - : DebuggerCommand(type), m_count(1), m_stackDepth(0), m_vmDepth(0) { } - - int decCount() { assert(m_count > 0); return --m_count;} - int getCount() const { assert(m_count > 0); return m_count;} - void setFileLine(const std::string &loc) { m_loc = loc;} - const std::string &getFileLine() const { return m_loc;} + : DebuggerCommand(type), m_complete(false), m_needsVMInterrupt(false), + m_stackDepth(0), m_vmDepth(0), m_stepOutOffset(0), m_stepOutUnit(nullptr), + m_count(1) { } + virtual ~CmdFlowControl(); virtual void sendImpl(DebuggerThriftBuffer &thrift); virtual void recvImpl(DebuggerThriftBuffer &thrift); @@ -39,18 +53,40 @@ public: virtual bool onClient(DebuggerClient *client); virtual bool onServer(DebuggerProxy *proxy); - void setStackDepth(int depth) { m_stackDepth = depth; } - int getStackDepth() const { return m_stackDepth; } - void setVMDepth(int depth) { m_vmDepth = depth; } - int getVMDepth() const { return m_vmDepth; } + // Work done to setup a new flow command, after receiving it from the client. + virtual void onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt); + + // Work done when a VM thread interrupts the proxy. + virtual void onBeginInterrupt(DebuggerProxy *proxy, + CmdInterrupt &interrupt) = 0; + + // A completed flow cmd has done all its work and can be deleted. + bool complete() { return m_complete; } + + // Does this cmd need to force interrupts in the interpreter loop? + bool needsVMInterrupt() { return m_needsVMInterrupt; } + +protected: + int decCount() { assert(m_count > 0); return --m_count;} + int getCount() const { assert(m_count > 0); return m_count;} + void installLocationFilterForLine(InterruptSite *site); + void removeLocationFilter(); + void setupStepOut(); + void cleanupStepOut(); + + bool m_complete; + bool m_needsVMInterrupt; + + // Support for stepping operations + int m_stackDepth; + int m_vmDepth; + std::string m_loc; // last break's source location + Offset m_stepOutOffset; + HPHP::VM::Unit *m_stepOutUnit; private: int16_t m_count; - std::string m_loc; // last break's source location bool m_smallStep; - - int m_stackDepth; - int m_vmDepth; }; /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/eval/debugger/cmd/cmd_next.cpp b/hphp/runtime/eval/debugger/cmd/cmd_next.cpp index 0085d2c7c..4ce874d76 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_next.cpp +++ b/hphp/runtime/eval/debugger/cmd/cmd_next.cpp @@ -19,6 +19,8 @@ namespace HPHP { namespace Eval { /////////////////////////////////////////////////////////////////////////////// +TRACE_SET_MOD(debugger); + bool CmdNext::help(DebuggerClient *client) { client->helpTitle("Next Command"); client->helpCmds( @@ -32,5 +34,63 @@ bool CmdNext::help(DebuggerClient *client) { return true; } +void CmdNext::onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt) { + TRACE(2, "CmdNext::onSetup\n"); + assert(!m_complete); // Complete cmds should not be asked to do work. + CmdFlowControl::onSetup(proxy, interrupt); + m_stackDepth = proxy->getStackDepth(); + m_vmDepth = g_vmContext->m_nesting; + m_loc = interrupt.getFileLine(); + + // Start by single-stepping the current line. + installLocationFilterForLine(interrupt.getSite()); + m_needsVMInterrupt = true; +} + +void CmdNext::onBeginInterrupt(DebuggerProxy *proxy, CmdInterrupt &interrupt) { + TRACE(2, "CmdNext::onBeginInterrupt\n"); + assert(!m_complete); // Complete cmds should not be asked to do work. + if (interrupt.getInterruptType() == ExceptionThrown) { + // If an exception is thrown we turn interrupts on to ensure we stop when + // control reaches the first catch clause. + removeLocationFilter(); + m_needsVMInterrupt = true; + return; + } + + int currentVMDepth = g_vmContext->m_nesting; + int currentStackDepth = proxy->getStackDepth(); + + if ((currentVMDepth < m_vmDepth) || + ((currentVMDepth == m_vmDepth) && (currentStackDepth <= m_stackDepth))) { + // We're at the same depth as when we started, or perhaps even shallower, so + // there's no need for any step out breakpoint anymore. + cleanupStepOut(); + + if (m_loc != interrupt.getFileLine()) { + TRACE(2, "CmdNext: same depth, off original line.\n"); + m_complete = (decCount() == 0); + } + if (!m_complete) { + TRACE(2, "CmdNext: not complete, filter new line and keep stepping.\n"); + m_loc = interrupt.getFileLine(); + installLocationFilterForLine(interrupt.getSite()); + m_needsVMInterrupt = true; + } + } else { + // Deeper, so let's setup a step out operation and turn interrupts off. + if (!m_stepOutUnit) { + // We can nuke the entire location filter here since we'll re-install it + // when we get back to the old level. Keeping it installed may be more + // efficient if we were on a large line, but there is a penalty for every + // opcode executed while it's installed and that's bad if there's a lot of + // code called from that line. + removeLocationFilter(); + setupStepOut(); + } + m_needsVMInterrupt = false; + } +} + /////////////////////////////////////////////////////////////////////////////// }} diff --git a/hphp/runtime/eval/debugger/cmd/cmd_next.h b/hphp/runtime/eval/debugger/cmd/cmd_next.h index 5612cbc89..2ad3206c1 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_next.h +++ b/hphp/runtime/eval/debugger/cmd/cmd_next.h @@ -28,6 +28,8 @@ public: CmdNext() : CmdFlowControl(KindOfNext) {} virtual bool help(DebuggerClient *client); + virtual void onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt); + virtual void onBeginInterrupt(DebuggerProxy *proxy, CmdInterrupt &interrupt); }; /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/eval/debugger/cmd/cmd_out.cpp b/hphp/runtime/eval/debugger/cmd/cmd_out.cpp index 244ba0f69..b537088fd 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_out.cpp +++ b/hphp/runtime/eval/debugger/cmd/cmd_out.cpp @@ -19,6 +19,8 @@ namespace HPHP { namespace Eval { /////////////////////////////////////////////////////////////////////////////// +TRACE_SET_MOD(debugger); + bool CmdOut::help(DebuggerClient *client) { client->helpTitle("Out Command"); client->helpCmds( @@ -32,5 +34,48 @@ bool CmdOut::help(DebuggerClient *client) { return true; } +void CmdOut::onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt) { + TRACE(2, "CmdOut::onSetup\n"); + assert(!m_complete); // Complete cmds should not be asked to do work. + CmdFlowControl::onSetup(proxy, interrupt); + m_stackDepth = proxy->getStackDepth(); + m_vmDepth = g_vmContext->m_nesting; + + // Simply setup a "step out breakpoint" and let the program run. + setupStepOut(); +} + +void CmdOut::onBeginInterrupt(DebuggerProxy *proxy, CmdInterrupt &interrupt) { + TRACE(2, "CmdNext::onBeginInterrupt\n"); + assert(!m_complete); // Complete cmds should not be asked to do work. + if (interrupt.getInterruptType() == ExceptionThrown) { + // If an exception is thrown we turn interrupts on to ensure we stop when + // control reaches the first catch clause. + removeLocationFilter(); + m_needsVMInterrupt = true; + return; + } + + int currentVMDepth = g_vmContext->m_nesting; + int currentStackDepth = proxy->getStackDepth(); + if (currentVMDepth < m_vmDepth) { + // Cut corner here, just break when cross VM boundary no matter how + // many levels we want to go out of + TRACE(2, "CmdOut: shallower VM depth, done.\n"); + cleanupStepOut(); + m_complete = true; + } else if ((currentVMDepth == m_vmDepth) && + (currentStackDepth < m_stackDepth)) { + TRACE(2, "CmdOut: same VM depth, shallower stack depth, done.\n"); + cleanupStepOut(); + m_complete = (decCount() == 0); + if (!m_complete) { + TRACE(2, "CmdOut: not complete, step out again.\n"); + setupStepOut(); + } + } + m_needsVMInterrupt = false; +} + /////////////////////////////////////////////////////////////////////////////// }} diff --git a/hphp/runtime/eval/debugger/cmd/cmd_out.h b/hphp/runtime/eval/debugger/cmd/cmd_out.h index 79e95e3ac..67a50babf 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_out.h +++ b/hphp/runtime/eval/debugger/cmd/cmd_out.h @@ -28,6 +28,8 @@ public: CmdOut() : CmdFlowControl(KindOfOut) {} virtual bool help(DebuggerClient *client); + virtual void onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt); + virtual void onBeginInterrupt(DebuggerProxy *proxy, CmdInterrupt &interrupt); }; /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/eval/debugger/cmd/cmd_step.cpp b/hphp/runtime/eval/debugger/cmd/cmd_step.cpp index 84ce62eee..76aac12a7 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_step.cpp +++ b/hphp/runtime/eval/debugger/cmd/cmd_step.cpp @@ -32,5 +32,25 @@ bool CmdStep::help(DebuggerClient *client) { return true; } +void CmdStep::onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt) { + assert(!m_complete); // Complete cmds should not be asked to do work. + CmdFlowControl::onSetup(proxy, interrupt); + // Allows a breakpoint on this same line to be hit again when control returns + // from function call. + BreakPointInfoPtr bp = proxy->getBreakPointAtCmd(interrupt); + if (bp) { + bp->setBreakable(proxy->getRealStackDepth()); + } + installLocationFilterForLine(interrupt.getSite()); + m_needsVMInterrupt = true; +} + +void CmdStep::onBeginInterrupt(DebuggerProxy *proxy, CmdInterrupt &interrupt) { + m_complete = (decCount() == 0); + if (!m_complete) { + installLocationFilterForLine(interrupt.getSite()); + } +} + /////////////////////////////////////////////////////////////////////////////// }} diff --git a/hphp/runtime/eval/debugger/cmd/cmd_step.h b/hphp/runtime/eval/debugger/cmd/cmd_step.h index bfff4d120..9ded8638c 100644 --- a/hphp/runtime/eval/debugger/cmd/cmd_step.h +++ b/hphp/runtime/eval/debugger/cmd/cmd_step.h @@ -28,6 +28,8 @@ public: CmdStep() : CmdFlowControl(KindOfStep) {} virtual bool help(DebuggerClient *client); + virtual void onSetup(DebuggerProxy *proxy, CmdInterrupt &interrupt); + virtual void onBeginInterrupt(DebuggerProxy *proxy, CmdInterrupt &interrupt); }; /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/eval/debugger/debugger.cpp b/hphp/runtime/eval/debugger/debugger.cpp index eb878c4db..82dff4401 100644 --- a/hphp/runtime/eval/debugger/debugger.cpp +++ b/hphp/runtime/eval/debugger/debugger.cpp @@ -123,7 +123,7 @@ void Debugger::RetireDummySandboxThread(DummySandbox* toRetire) { } void Debugger::CleanupDummySandboxThreads() { - TRACE(2, "Debugger::CleanupDummySandboxThreads\n"); + TRACE(7, "Debugger::CleanupDummySandboxThreads\n"); s_debugger.cleanupDummySandboxThreads(); } @@ -229,10 +229,10 @@ void Debugger::Interrupt(int type, const char *program, proxy->interrupt(cmd); interrupts.pop(); } - // Some cmds requires us to interpret all instructions until the cmd + // Some cmds require us to interpret all instructions until the cmd // completes. Setting this will ensure we stay out of JIT code and in the // interpreter so phpDebuggerOpcodeHook has a chance to work. - rjdata.debuggerIntr = proxy->needInterruptForNonBreak(); + rjdata.debuggerIntr = proxy->needVMInterrupts(); } else { TRACE(3, "proxy == null\n"); // Debugger clients are disconnected abnormally, or this sandbox is not @@ -437,7 +437,7 @@ void Debugger::retireDummySandboxThread(DummySandbox* toRetire) { } void Debugger::cleanupDummySandboxThreads() { - TRACE(2, "Debugger::cleanupDummySandboxThreads\n"); + TRACE(7, "Debugger::cleanupDummySandboxThreads\n"); DummySandbox* ptr = nullptr; while (m_cleanupDummySandboxQ.try_pop(ptr)) { ptr->notify(); diff --git a/hphp/runtime/eval/debugger/debugger_base.cpp b/hphp/runtime/eval/debugger/debugger_base.cpp index a3448cddd..1c73349b4 100644 --- a/hphp/runtime/eval/debugger/debugger_base.cpp +++ b/hphp/runtime/eval/debugger/debugger_base.cpp @@ -293,7 +293,7 @@ static void get_color(int tokid, int prev, int next, const char **palette = DebuggerClient::DefaultCodeColors) { - TRACE(2, "debugger_base:get_color\n"); + TRACE(7, "debugger_base:get_color\n"); #undef YYTOKENTYPE #ifdef YYTOKEN_MAP #undef YYTOKEN_MAP @@ -386,7 +386,7 @@ static void get_color(int tokid, int prev, int next, static void color_line_no(StringBuffer &sb, int line, int lineFocus0, int lineFocus1, const char *color) { - TRACE(2, "debugger_base:color_line_no\n"); + TRACE(7, "debugger_base:color_line_no\n"); if (((line == lineFocus0 && lineFocus1 == 0) || (line >= lineFocus0 && line <= lineFocus1)) && DebuggerClient::HighlightBgColor) { @@ -402,7 +402,7 @@ static void append_line_no(StringBuffer &sb, const char *text, int lineFocus0, int charFocus0, int lineFocus1, int charFocus1, const char **palette = DebuggerClient::DefaultCodeColors) { - TRACE(2, "debugger_base:append_line_no\n"); + TRACE(7, "debugger_base:append_line_no\n"); const char *colorLineNo = palette[CodeColorLineNo * 2]; const char *endLineNo = palette[CodeColorLineNo * 2 + 1]; @@ -457,7 +457,7 @@ static void append_line_no(StringBuffer &sb, const char *text, String highlight_code(CStrRef source, int line /* = 0 */, int lineFocus0 /* = 0 */, int charFocus0 /* = 0 */, int lineFocus1 /* = 0 */, int charFocus1 /* = 0 */) { - TRACE(2, "debugger_base:highlight_code\n"); + TRACE(7, "debugger_base:highlight_code\n"); String prepended = "= lineFocus0 * 1000 + charFocus0 && @@ -483,7 +483,7 @@ string check_char_highlight(int lineFocus0, int charFocus0, String highlight_php(CStrRef source, int line /* = 0 */, int lineFocus0 /* = 0 */, int charFocus0 /* = 0 */, int lineFocus1 /* = 0 */, int charFocus1 /* = 0 */) { - TRACE(2, "debugger_base:highlight_php\n"); + TRACE(7, "debugger_base:highlight_php\n"); StringBuffer res; Scanner scanner(source.data(), source.size(), Scanner::AllowShortTags | Scanner::ReturnAllTokens); diff --git a/hphp/runtime/eval/debugger/debugger_command.cpp b/hphp/runtime/eval/debugger/debugger_command.cpp index 37709cdbc..a3548779a 100644 --- a/hphp/runtime/eval/debugger/debugger_command.cpp +++ b/hphp/runtime/eval/debugger/debugger_command.cpp @@ -27,7 +27,7 @@ static const Trace::Module TRACEMOD = Trace::debugger; // send/recv bool DebuggerCommand::send(DebuggerThriftBuffer &thrift) { - TRACE(2, "DebuggerCommand::send\n"); + TRACE(5, "DebuggerCommand::send\n"); try { thrift.reset(false); sendImpl(thrift); @@ -41,7 +41,7 @@ bool DebuggerCommand::send(DebuggerThriftBuffer &thrift) { } bool DebuggerCommand::recv(DebuggerThriftBuffer &thrift) { - TRACE(2, "DebuggerCommand::recv\n"); + TRACE(5, "DebuggerCommand::recv\n"); try { recvImpl(thrift); } catch (...) { @@ -53,7 +53,7 @@ bool DebuggerCommand::recv(DebuggerThriftBuffer &thrift) { } void DebuggerCommand::sendImpl(DebuggerThriftBuffer &thrift) { - TRACE(2, "DebuggerCommand::sendImpl\n"); + TRACE(5, "DebuggerCommand::sendImpl\n"); thrift.write((int32_t)m_type); thrift.write(m_class); thrift.write(m_body); @@ -61,14 +61,14 @@ void DebuggerCommand::sendImpl(DebuggerThriftBuffer &thrift) { } void DebuggerCommand::recvImpl(DebuggerThriftBuffer &thrift) { - TRACE(2, "DebuggerCommand::recvImpl\n"); + TRACE(5, "DebuggerCommand::recvImpl\n"); thrift.read(m_body); thrift.read(m_version); } bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift, DebuggerCommandPtr &cmd, const char *caller) { - TRACE(2, "DebuggerCommand::Receive\n"); + TRACE(5, "DebuggerCommand::Receive\n"); cmd.reset(); struct pollfd fds[1]; @@ -93,6 +93,8 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift, return true; } + TRACE(1, "DebuggerCommand::Receive: got cmd of type %d\n", type); + // not all commands are here, as not all commands need to be sent over wire switch (type) { case KindOfBreak : cmd = DebuggerCommandPtr(new CmdBreak ()); break; @@ -137,6 +139,7 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift, return true; } if (!cmd->recv(thrift)) { + Logger::Verbose("%s => DebuggerCommand::Receive(): socket error", caller); cmd.reset(); } return true; diff --git a/hphp/runtime/eval/debugger/debugger_proxy.cpp b/hphp/runtime/eval/debugger/debugger_proxy.cpp index cdf85b03e..d1eaa35a8 100644 --- a/hphp/runtime/eval/debugger/debugger_proxy.cpp +++ b/hphp/runtime/eval/debugger/debugger_proxy.cpp @@ -225,48 +225,37 @@ void DebuggerProxy::getBreakFuncs( } } +// Proxy needs to be interrupted because it has something setup which may need +// processing; breakpoints, flow control commands, a signal. bool DebuggerProxy::needInterrupt() { TRACE(2, "DebuggerProxy::needInterrupt\n"); - return m_hasBreakPoints || m_flow || - m_signum != CmdSignal::SignalNone; + return m_hasBreakPoints || m_flow || m_signum != CmdSignal::SignalNone; } -bool DebuggerProxy::needInterruptForNonBreak() { - TRACE(2, "DebuggerProxy::needInterruptForNonBreak\n"); - return m_flow || m_signum != CmdSignal::SignalNone; +// We need VM interrupts if we're in a state that requires the debugger to be +// interrupted for every opcode. +bool DebuggerProxy::needVMInterrupts() { + TRACE(2, "DebuggerProxy::needVMInterrupts\n"); + bool flowNeedsInterrupt = (m_flow && m_flow->needsVMInterrupt()); + return flowNeedsInterrupt || m_signum != CmdSignal::SignalNone; } // Handle an interrupt from the VM. void DebuggerProxy::interrupt(CmdInterrupt &cmd) { TRACE(2, "DebuggerProxy::interrupt\n"); + // Make any breakpoints that have passed breakable again. changeBreakPointDepth(cmd); + // At this point we have an interrupt, but we don't know if we're on the + // thread the proxy considers "current". + // NB: BreakPointReached really means we've got control of a VM thread from + // the opcode hook. This could be for a breakpoint, stepping, etc. if (cmd.getInterruptType() == BreakPointReached) { - if (!needInterrupt()) return; + // If we have this type of interrupt then the proxy must have outstanding + // work to do. If it doesn't, then we've come too far processing this + // interrupt. + assert(needInterrupt()); - // NB: stepping is represented as a BreakPointReached interrupt. - - // Modify m_lastLocFilter to save current location. This will short-circuit - // the work done up in phpDebuggerOpcodeHook() and ensure we don't break on - // this line until we're completely off of it. - InterruptSite *site = cmd.getSite(); - if (g_vmContext->m_lastLocFilter) { - g_vmContext->m_lastLocFilter->clear(); - } else { - g_vmContext->m_lastLocFilter = new VM::PCFilter(); - } - if (debug && Trace::moduleEnabled(Trace::bcinterp, 5)) { - Trace::trace("prepare source loc filter\n"); - const VM::OffsetRangeVec& offsets = site->getCurOffsetRange(); - for (VM::OffsetRangeVec::const_iterator it = offsets.begin(); - it != offsets.end(); ++it) { - Trace::trace("block source loc in %s:%d: unit %p offset [%d, %d)\n", - site->getFile(), site->getLine0(), - site->getUnit(), it->m_base, it->m_past); - } - } - g_vmContext->m_lastLocFilter->addRanges(site->getUnit(), - site->getCurOffsetRange()); // If the breakpoint is not to be processed, we should continue execution. BreakPointInfoPtr bp = getBreakPointAtCmd(cmd); if (bp) { @@ -278,13 +267,14 @@ void DebuggerProxy::interrupt(CmdInterrupt &cmd) { } } - // Wait until this thread is the thread this proxy wants to debug. - // NB: breakpoints and control flow checks happen here, too, and return false - // if we're not done with the flow, or not at a breakpoint, etc. + // Wait until this thread is the one this proxy wants to debug. if (!blockUntilOwn(cmd, true)) { return; } - if (processFlowBreak(cmd)) { + + // We know we're on the "current" thread, so we can process any active flow + // command, stop if we're at a breakpoint, handle other interrupts, etc. + if (checkFlowBreak(cmd)) { while (true) { try { Lock lock(m_signalMutex); @@ -443,10 +433,10 @@ DThreadInfoPtr DebuggerProxy::createThreadInfo(const std::string &desc) { } // Waits until this thread is the one that the proxy considers the current -// thread. This also check to see if the given cmd has any breakpoints or -// flow control that we should stop for. Note: while stepping, pretty much all -// of the stepping logic is handled below here and this will return false if -// the stepping operation has not completed. +// thread. +// NB: if the thread is not the current thread, and we're asked to check +// breakpoints, then if there are no breakpoints which could effect this +// thread we simply return false and stop processing the current interrupt. bool DebuggerProxy::blockUntilOwn(CmdInterrupt &cmd, bool check) { TRACE(2, "DebuggerProxy::blockUntilOwn\n"); int64_t self = cmd.getThreadId(); @@ -476,10 +466,9 @@ bool DebuggerProxy::blockUntilOwn(CmdInterrupt &cmd, bool check) { // The breakpoint might have been removed while I'm waiting return false; } - } else if (check && !checkFlowBreak(cmd)) { - return false; } + // This thread is now the one the proxy considers the current thread. if (m_thread == 0) { m_thread = self; } @@ -510,38 +499,45 @@ bool DebuggerProxy::checkFlowBreak(CmdInterrupt &cmd) { // Note that even if fcShouldBreak, we still need to test bpShouldBreak // so to correctly report breakpoint reached + evaluating watchpoints; - // vice versa, so to decCount() on m_flow. + // vice versa, so to give a flow cmd a chance to react. + // Note: a Continue cmd is a bit special. We need to process it _only_ if + // we decide to break. bool fcShouldBreak = false; // should I break according to flow control? bool bpShouldBreak = false; // should I break according to breakpoints? if ((cmd.getInterruptType() == BreakPointReached || - cmd.getInterruptType() == HardBreakPoint) && m_flow) { - fcShouldBreak = breakByFlowControl(cmd); + cmd.getInterruptType() == HardBreakPoint || + cmd.getInterruptType() == ExceptionThrown) && m_flow) { + if (!m_flow->is(DebuggerCommand::KindOfContinue)) { + m_flow->onBeginInterrupt(this, cmd); + if (m_flow->complete()) { + fcShouldBreak = true; + m_flow.reset(); + } + } } bpShouldBreak = checkBreakPoints(cmd); + + // This is done before KindOfContinue testing. if (!fcShouldBreak && !bpShouldBreak) { - return false; // this is done before KindOfContinue testing + return false; } - - return true; -} - -bool DebuggerProxy::processFlowBreak(CmdInterrupt &cmd) { - TRACE(2, "DebuggerProxy::processFlowBreak\n"); if ((cmd.getInterruptType() == BreakPointReached || cmd.getInterruptType() == HardBreakPoint) && m_flow) { if (m_flow->is(DebuggerCommand::KindOfContinue)) { - if (!m_flow->decCount()) m_flow.reset(); + m_flow->onBeginInterrupt(this, cmd); + if (m_flow->complete()) m_flow.reset(); return false; } } + return true; } void DebuggerProxy::checkStop() { - TRACE(2, "DebuggerProxy::checkStop\n"); + TRACE(5, "DebuggerProxy::checkStop\n"); if (m_stopped) { Debugger::RemoveProxy(shared_from_this()); m_thrift.close(); @@ -551,7 +547,7 @@ void DebuggerProxy::checkStop() { void DebuggerProxy::processInterrupt(CmdInterrupt &cmd) { TRACE(2, "DebuggerProxy::processInterrupt\n"); - // Do the server-side work for this cmd, which just notifies the client. + // Do the server-side work for this interrupt, which just notifies the client. if (!cmd.onServer(this)) { Debugger::RemoveProxy(shared_from_this()); // on socket error return; @@ -571,10 +567,15 @@ void DebuggerProxy::processInterrupt(CmdInterrupt &cmd) { // Any control flow command gets installed here and we continue execution. m_flow = dynamic_pointer_cast(res); if (m_flow) { - m_flow->onServer(this); - processFlowControl(cmd); - if (m_flow && m_threadMode == Normal) { - switchThreadMode(Sticky); + m_flow->onSetup(this, cmd); + if (!m_flow->complete()) { + if (m_threadMode == Normal) { + switchThreadMode(Sticky); + } + } else { + // The flow cmd has determined that it is done with its work and + // doesn't need to remain for later processing. + m_flow.reset(); } return; } @@ -715,37 +716,6 @@ int DebuggerProxy::getStackDepth() { return depth; } -// Handle a continue cmd, or setup stepping. -void DebuggerProxy::processFlowControl(CmdInterrupt &cmd) { - TRACE(2, "DebuggerProxy::processFlowControl\n"); - switch (m_flow->getType()) { - case DebuggerCommand::KindOfContinue: - if (!m_flow->decCount()) { - m_flow.reset(); - } - break; - case DebuggerCommand::KindOfStep: - { - // allows the breakpoint to be hit again when returns - // from function call - BreakPointInfoPtr bp = getBreakPointAtCmd(cmd); - if (bp) { - bp->setBreakable(getRealStackDepth()); - } - break; - } - case DebuggerCommand::KindOfOut: - case DebuggerCommand::KindOfNext: - m_flow->setStackDepth(getStackDepth()); - m_flow->setVMDepth(g_vmContext->m_nesting); - m_flow->setFileLine(cmd.getFileLine()); - break; - default: - assert(false); - break; - } -} - /** * If a breakpoint is set at that depth, * this function clears the current depth information @@ -764,55 +734,5 @@ void DebuggerProxy::changeBreakPointDepth(CmdInterrupt& cmd) { } } -// Determine if an outstanding flow control cmd has run it's course and we -// should stop execution. -bool DebuggerProxy::breakByFlowControl(CmdInterrupt &cmd) { - TRACE(2, "DebuggerProxy::breakByFlowControl\n"); - switch (m_flow->getType()) { - case DebuggerCommand::KindOfStep: { - if (!m_flow->decCount()) { - // if the line changes and the stack depth is the same - // pop the breakpoint depth stack - m_flow.reset(); - return true; - } - break; - } - case DebuggerCommand::KindOfNext: { - int currentVMDepth = g_vmContext->m_nesting; - int currentStackDepth = getStackDepth(); - - if (currentVMDepth <= m_flow->getVMDepth() && - currentStackDepth <= m_flow->getStackDepth() && - m_flow->getFileLine() != cmd.getFileLine()) { - if (!m_flow->decCount()) { - m_flow.reset(); - return true; - } - } - - break; - } - case DebuggerCommand::KindOfOut: { - int currentVMDepth = g_vmContext->m_nesting; - int currentStackDepth = getStackDepth(); - if (currentVMDepth < m_flow->getVMDepth()) { - // Cut corner here, just break when cross VM boundary no matter how - // many levels we want to go out of - m_flow.reset(); - return true; - } else if (m_flow->getStackDepth() - currentStackDepth >= - m_flow->getCount()) { - m_flow.reset(); - return true; - } - break; - } - default: - break; - } - return false; -} - /////////////////////////////////////////////////////////////////////////////// }} diff --git a/hphp/runtime/eval/debugger/debugger_proxy.h b/hphp/runtime/eval/debugger/debugger_proxy.h index 49a6e30cd..16efe3dc0 100644 --- a/hphp/runtime/eval/debugger/debugger_proxy.h +++ b/hphp/runtime/eval/debugger/debugger_proxy.h @@ -88,11 +88,16 @@ public: bool couldBreakEnterFunc(const StringData* funcFullName); void getBreakClsMethods(std::vector& classNames); void getBreakFuncs(std::vector& funcFullNames); + BreakPointInfoPtr getBreakPointAtCmd(CmdInterrupt& cmd); bool needInterrupt(); - bool needInterruptForNonBreak(); + bool needVMInterrupts(); void interrupt(CmdInterrupt &cmd); bool sendToClient(DebuggerCommand *cmd); + CmdInterrupt& currentInterruptCmd(); + + int getStackDepth(); + int getRealStackDepth(); void startSignalThread(); void pollSignal(); // for signal polling thread @@ -110,18 +115,11 @@ private: bool blockUntilOwn(CmdInterrupt &cmd, bool check); bool checkBreakPoints(CmdInterrupt &cmd); bool checkFlowBreak(CmdInterrupt &cmd); - bool processFlowBreak(CmdInterrupt &cmd); void processInterrupt(CmdInterrupt &cmd); - void processFlowControl(CmdInterrupt &cmd); - bool breakByFlowControl(CmdInterrupt &cmd); DThreadInfoPtr createThreadInfo(const std::string &desc); void changeBreakPointDepth(CmdInterrupt& cmd); - BreakPointInfoPtr getBreakPointAtCmd(CmdInterrupt& cmd); - - int getRealStackDepth(); - int getStackDepth(); bool m_stopped; diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index 7b15c1e71..18718655d 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -1890,6 +1890,7 @@ void VMExecutionContext::enterVM(TypedValue* retval, ExtraArgs* extraArgs) { m_firstAR = ar; ar->m_savedRip = (uintptr_t)tx64->getCallToExit(); + assert(isReturnHelper(ar->m_savedRip)); DEBUG_ONLY int faultDepth = m_faults.size(); SCOPE_EXIT { @@ -2755,6 +2756,7 @@ bool VMExecutionContext::evalUnit(Unit* unit, bool local, ar->m_soff = uintptr_t(m_fp->m_func->unit()->offsetOf(pc) - m_fp->m_func->base()); ar->m_savedRip = (uintptr_t)tx64->getRetFromInterpretedFrame(); + assert(isReturnHelper(ar->m_savedRip)); pushLocalsAndIterators(func); if (local) { ar->m_varEnv = 0; @@ -3004,6 +3006,7 @@ void VMExecutionContext::enterDebuggerDummyEnv() { ar->m_soff = 0; ar->m_savedRbp = 0; ar->m_savedRip = (uintptr_t)tx64->getCallToExit(); + assert(isReturnHelper(ar->m_savedRip)); m_fp = ar; m_pc = s_debuggerDummy->entry(); m_firstAR = ar; @@ -3018,6 +3021,34 @@ void VMExecutionContext::exitDebuggerDummyEnv() { m_globalVarEnv->detach(getFP()); } +// Identifies the set of return helpers that we may set m_savedRip to in an +// ActRec. +bool VMExecutionContext::isReturnHelper(uintptr_t address) { + return ((address == (uintptr_t)tx64->getRetFromInterpretedFrame()) || + (address == (uintptr_t)tx64->getRetFromInterpretedGeneratorFrame()) || + (address == (uintptr_t)tx64->getCallToExit())); +} + +// Walk the stack and find any return address to jitted code and bash it to +// the RetFromInterpretedFrame helper. This ensures that we don't return into +// jitted code and gives the system the proper chance to interpret blacklisted +// tracelets. +void VMExecutionContext::preventReturnsToTC() { + assert(isDebuggerAttached()); + if (RuntimeOption::EvalJit) { + ActRec *ar = getFP(); + while (ar) { + if (!isReturnHelper(ar->m_savedRip)) { + TRACE(2, "Replace RIP with RetFromInterpretedFrame helper in fp %p" + ", savedRip 0x%lx\n", ar, ar->m_savedRip); + ar->m_savedRip = (uintptr_t)tx64->getRetFromInterpretedFrame(); + assert(isReturnHelper(ar->m_savedRip)); + } + ar = getPrevVMState(ar); + } + } +} + static inline StringData* lookup_name(TypedValue* key) { return prepareKey(key); } @@ -5864,6 +5895,7 @@ template void VMExecutionContext::doFCall(ActRec* ar, PC& pc) { assert(ar->m_savedRbp == (uint64_t)m_fp); ar->m_savedRip = (uintptr_t)tx64->getRetFromInterpretedFrame(); + assert(isReturnHelper(ar->m_savedRip)); TRACE(3, "FCall: pc %p func %p base %d\n", m_pc, m_fp->m_func->unit()->entry(), int(m_fp->m_func->base())); @@ -6130,6 +6162,7 @@ bool VMExecutionContext::doFCallArray(PC& pc) { assert(ar->m_savedRbp == (uint64_t)m_fp); assert(!ar->m_func->isGenerator()); ar->m_savedRip = (uintptr_t)tx64->getRetFromInterpretedFrame(); + assert(isReturnHelper(ar->m_savedRip)); TRACE(3, "FCallArray: pc %p func %p base %d\n", m_pc, m_fp->m_func->unit()->entry(), int(m_fp->m_func->base())); @@ -6848,6 +6881,7 @@ void VMExecutionContext::iopContEnter(PC& pc) { contAR->m_soff = m_fp->m_func->unit()->offsetOf(pc) - (uintptr_t)m_fp->m_func->base(); contAR->m_savedRip = (uintptr_t)tx64->getRetFromInterpretedGeneratorFrame(); + assert(isReturnHelper(contAR->m_savedRip)); m_fp = contAR; pc = contAR->m_func->getEntry(); diff --git a/hphp/runtime/vm/debugger_hook.cpp b/hphp/runtime/vm/debugger_hook.cpp index 1515e5e58..46f2a4d9c 100644 --- a/hphp/runtime/vm/debugger_hook.cpp +++ b/hphp/runtime/vm/debugger_hook.cpp @@ -41,7 +41,7 @@ static inline Transl::Translator* transl() { // here and execute any number of commands from the client. Return from here // lets the opcode execute. void phpDebuggerOpcodeHook(const uchar* pc) { - TRACE(5, "in phpDebuggerHook()\n"); + TRACE(5, "in phpDebuggerOpcodeHook()\n"); // Short-circuit when we're doing things like evaling PHP for print command, // or conditional breakpoints. if (UNLIKELY(g_vmContext->m_dbgNoBreak)) { @@ -60,27 +60,24 @@ void phpDebuggerOpcodeHook(const uchar* pc) { !g_vmContext->m_breakPointFilter->checkPC(pc))) { TRACE(5, "not in the PC range for any breakpoints\n"); if (LIKELY(!DEBUGGER_FORCE_INTR)) { - // Implies we left the location for last break. - delete g_vmContext->m_lastLocFilter; - g_vmContext->m_lastLocFilter = nullptr; return; } TRACE(5, "DEBUGGER_FORCE_INTR\n"); } Eval::Debugger::InterruptVMHook(); - TRACE(5, "out phpDebuggerHook()\n"); + TRACE(5, "out phpDebuggerOpcodeHook()\n"); } // Hook called from iopThrow to signal that we are about to throw an exception. // NB: this does not hook any portion of exception unwind. void phpDebuggerExceptionHook(ObjectData* e) { - TRACE(5, "in phpExceptionHook()\n"); + TRACE(5, "in phpDebuggerExceptionHook()\n"); if (UNLIKELY(g_vmContext->m_dbgNoBreak)) { TRACE(5, "NoBreak flag is on\n"); return; } Eval::Debugger::InterruptVMHook(Eval::ExceptionThrown, e); - TRACE(5, "out phpExceptionHook()\n"); + TRACE(5, "out phpDebuggerExceptionHook()\n"); } bool isDebuggerAttachedProcess() { @@ -102,6 +99,10 @@ static void blacklistRangesInJit(const Unit* unit, if (!transl()->addDbgGuards(unit)) { Logger::Warning("Failed to set breakpoints in Jitted code"); } + // In this case, we may be setting a breakpoint in a tracelet which could + // already be jitted, and present on the stack. Make sure we don't return + // to it so we have a chance to honor breakpoints. + g_vmContext->preventReturnsToTC(); } // Ensure we interpret an entire function when the debugger is attached. @@ -112,6 +113,13 @@ static void blacklistFuncInJit(const Func* f) { blacklistRangesInJit(unit, ranges); } +static PCFilter *getBreakPointFilter() { + if (!g_vmContext->m_breakPointFilter) { + g_vmContext->m_breakPointFilter = new PCFilter(); + } + return g_vmContext->m_breakPointFilter; +} + static void addBreakPointsInFile(Eval::DebuggerProxy* proxy, Eval::PhpFile* efile) { Eval::BreakPointInfoPtrVec bps; @@ -131,9 +139,6 @@ static void addBreakPointsInFile(Eval::DebuggerProxy* proxy, if (!unit->getOffsetRanges(bp->m_line1, offsets)) { continue; } - if (!g_vmContext->m_breakPointFilter) { - g_vmContext->m_breakPointFilter = new PCFilter(); - } if (debug && Trace::moduleEnabled(Trace::bcinterp, 5)) { for (OffsetRangeVec::const_iterator it = offsets.begin(); it != offsets.end(); ++it) { @@ -142,7 +147,7 @@ static void addBreakPointsInFile(Eval::DebuggerProxy* proxy, it->m_base, it->m_past); } } - g_vmContext->m_breakPointFilter->addRanges(unit, offsets); + getBreakPointFilter()->addRanges(unit, offsets); if (RuntimeOption::EvalJit) { blacklistRangesInJit(unit, offsets); } @@ -151,12 +156,9 @@ static void addBreakPointsInFile(Eval::DebuggerProxy* proxy, static void addBreakPointFuncEntry(const Func* f) { PC pc = f->unit()->at(f->base()); - if (!g_vmContext->m_breakPointFilter) { - g_vmContext->m_breakPointFilter = new PCFilter(); - } TRACE(5, "func() break %s : unit %p offset %d)\n", f->fullName()->data(), f->unit(), f->base()); - g_vmContext->m_breakPointFilter->addPC(pc); + getBreakPointFilter()->addPC(pc); if (RuntimeOption::EvalJit) { if (transl()->addDbgBLPC(pc)) { // if a new entry is added in blacklist @@ -178,6 +180,30 @@ static void addBreakPointsClass(Eval::DebuggerProxy* proxy, } } +void phpAddBreakPoint(const Unit* unit, Offset offset) { + PC pc = unit->at(offset); + getBreakPointFilter()->addPC(pc); + if (RuntimeOption::EvalJit) { + if (transl()->addDbgBLPC(pc)) { + // if a new entry is added in blacklist + if (!transl()->addDbgGuards(unit)) { + Logger::Warning("Failed to set breakpoints in Jitted code"); + } + // In this case, we may be setting a breakpoint in a tracelet which could + // already be jitted, and present on the stack. Make sure we don't return + // to it so we have a chance to honor breakpoints. + g_vmContext->preventReturnsToTC(); + } + } +} + +void phpRemoveBreakPoint(const Unit* unit, Offset offset) { + if (g_vmContext->m_breakPointFilter) { + PC pc = unit->at(offset); + g_vmContext->m_breakPointFilter->removePC(pc); + } +} + void phpDebuggerEvalHook(const Func* f) { if (RuntimeOption::EvalJit) { blacklistFuncInJit(f); @@ -325,5 +351,9 @@ int PCFilter::addRanges(const Unit* unit, const OffsetRangeVec& offsets) { return counter; } +void PCFilter::removeOffset(const Unit* unit, Offset offset) { + removePC(unit->at(offset)); +} + ////////////////////////////////////////////////////////////////////////// }} diff --git a/hphp/runtime/vm/debugger_hook.h b/hphp/runtime/vm/debugger_hook.h index 74456f3a2..4fc93ed48 100644 --- a/hphp/runtime/vm/debugger_hook.h +++ b/hphp/runtime/vm/debugger_hook.h @@ -49,6 +49,10 @@ void phpDebuggerDefFuncHook(const Func* func); // Helper to apply pending breakpoints to all files. void phpSetBreakPointsInAllFiles(Eval::DebuggerProxy* proxy); +// Add/remove breakpoints at a specific offset. +void phpAddBreakPoint(const Unit* unit, Offset offset); +void phpRemoveBreakPoint(const Unit* unit, Offset offset); + // Is this thread being debugged? static inline bool isDebuggerAttached() { return ThreadInfo::s_threadInfo.getNoCheck()->m_reqInjectionData.debugger; @@ -64,7 +68,7 @@ static inline bool isDebuggerAttached() { bool isDebuggerAttachedProcess(); // This flag ensures two things: first, that we stay in the interpreter and -// out of JIT code. Second, that the phpDebuggerHook will continue to allow +// out of JIT code. Second, that phpDebuggerOpcodeHook will continue to allow // debugger interrupts for every opcode executed (modulo filters.) #define DEBUGGER_FORCE_INTR \ (ThreadInfo::s_threadInfo.getNoCheck()->m_reqInjectionData.debuggerIntr) @@ -102,13 +106,22 @@ private: public: PCFilter() {} + + // Add/remove offsets, either individually or by range. int addRanges(const Unit* unit, const OffsetRangeVec& offsets); + void removeOffset(const Unit* unit, Offset offset); + + // Add/remove/check explicit PCs. void addPC(const uchar* pc) { m_map.setPointer((void*)pc, (void*)pc); } + void removePC(const uchar* pc) { + m_map.setPointer((void*)pc, nullptr); + } bool checkPC(const uchar* pc) { return m_map.getPointer((void*)pc) == (void*)pc; } + void clear() { m_map.clear(); } diff --git a/hphp/test/test_debugger.cpp b/hphp/test/test_debugger.cpp index 92f45feb3..d89448131 100644 --- a/hphp/test/test_debugger.cpp +++ b/hphp/test/test_debugger.cpp @@ -344,6 +344,7 @@ void TestDebugger::runServer() { "/test"; const char *argv[] = {"hphpd_test", "--mode=server", "--config=test/config-debugger-server.hdf", + "-vEval.JitWarmupRequests=0", portConfig.c_str(), srcRootConfig.c_str(), includePathConfig.c_str(),