Remove the Jump command from hphpd.

Hphpd's Jump command has been fundamentally broken for a long time. It was originally implemented to run the byte code in a modified way which didn't make state changes, and wait for the destination offset to be reached. We lost the ability to do that long ago, and the implementation of this command has atrophied since. As it stands now, if you're lucky it might act like "run until", which is the opposite of what it is documented to do.

I've removed the command entirely. Fixing it is a very large effort which we might consider some time in the future.
Esse commit está contido em:
Mike Magruder
2013-04-23 18:45:15 -07:00
commit de Sara Golemon
commit a41a8a74f0
13 arquivos alterados com 37 adições e 279 exclusões
+1 -8
Ver Arquivo
@@ -53,17 +53,13 @@ public:
std::string &url() const { return m_url;}
std::string desc() const;
bool isJumping() const { return m_jumping;}
void setJumping() { m_jumping = true;}
protected:
explicit InterruptSite(CVarRef e = null_variant, const char *cls = nullptr,
const char *function = nullptr,
StringData *file = nullptr, int line0 = 0,
int char0 = 0, int line1 = 0, int char1 = 0)
: m_exception(e), m_class(cls), m_function(function), m_file(file),
m_line0(line0), m_char0(char0), m_line1(line1), m_char1(char1),
m_jumping(false) { }
m_line0(line0), m_char0(char0), m_line1(line1), m_char1(char1) {}
Variant m_exception;
@@ -77,9 +73,6 @@ protected:
int32_t m_char0;
int32_t m_line1;
int32_t m_char1;
// jump instruction
bool m_jumping;
};
// Forms an InterruptSite by looking at the current thread's current PC and
-1
Ver Arquivo
@@ -27,7 +27,6 @@
#include <runtime/eval/debugger/cmd/cmd_global.h>
#include <runtime/eval/debugger/cmd/cmd_help.h>
#include <runtime/eval/debugger/cmd/cmd_info.h>
#include <runtime/eval/debugger/cmd/cmd_jump.h>
#include <runtime/eval/debugger/cmd/cmd_constant.h>
#include <runtime/eval/debugger/cmd/cmd_list.h>
#include <runtime/eval/debugger/cmd/cmd_machine.h>
@@ -35,7 +35,6 @@ void CmdHelp::HelpAll(DebuggerClient *client) {
"[s]tep *", "steps into a function call or an expression",
"[n]ext *", "steps over a function call or a line",
"[o]ut *", "steps out a function call",
"[j]ump", "jumps to specified line of code for execution",
"Display Commands", "",
"[p]rint", "prints a variable's value",
+7 -13
Ver Arquivo
@@ -27,7 +27,9 @@ void CmdInterrupt::sendImpl(DebuggerThriftBuffer &thrift) {
thrift.write(m_program);
thrift.write(m_errorMsg);
thrift.write(m_threadId);
thrift.write(m_pendingJump);
// Used to be m_pendingJump, but that's been removed. Write false until
// we rev the protocol.
thrift.write(false);
if (m_site) {
thrift.write(true);
thrift.write(m_site->getFile());
@@ -60,7 +62,10 @@ void CmdInterrupt::recvImpl(DebuggerThriftBuffer &thrift) {
thrift.read(m_program);
thrift.read(m_errorMsg);
thrift.read(m_threadId);
thrift.read(m_pendingJump);
// Used to be m_pendingJump, but that's been removed. Read a dummy bool until
// we rev the protocol.
bool dummy;
thrift.read(dummy);
m_bpi = BreakPointInfoPtr(new BreakPointInfo());
bool site; thrift.read(site);
if (site) {
@@ -133,17 +138,6 @@ bool CmdInterrupt::onClient(DebuggerClient *client) {
}
client->setMatchedBreakPoints(m_matched);
switch (m_interrupt) {
case SessionEnded:
case RequestEnded:
case PSPEnded:
if (m_pendingJump) {
client->error("Your jump point cannot be reached. You may only jump "
"to certain parallel or outer execution points.");
}
break;
}
switch (m_interrupt) {
case SessionStarted:
if (!m_program.empty()) {
+2 -5
Ver Arquivo
@@ -28,13 +28,13 @@ class CmdInterrupt : public DebuggerCommand {
public:
CmdInterrupt()
: DebuggerCommand(KindOfInterrupt),
m_interrupt(-1), m_threadId(0), m_site(nullptr), m_pendingJump(false) {}
m_interrupt(-1), m_threadId(0), m_site(nullptr) {}
CmdInterrupt(InterruptType interrupt, const char *program,
InterruptSite *site, const char *error)
: DebuggerCommand(KindOfInterrupt),
m_interrupt(interrupt), m_program(program ? program : ""),
m_site(site), m_pendingJump(false) {
m_site(site) {
m_threadId = (int64_t)Process::GetThreadId();
if (error) m_errorMsg = error;
}
@@ -56,8 +56,6 @@ public:
InterruptSite *getSite() { return m_site;}
void setPendingJump() { m_pendingJump = true;}
private:
int16_t m_interrupt;
std::string m_program; // informational only
@@ -66,7 +64,6 @@ private:
InterruptSite *m_site; // server side
BreakPointInfoPtr m_bpi; // client side
BreakPointInfoPtrVec m_matched;
bool m_pendingJump;
};
///////////////////////////////////////////////////////////////////////////////
-126
Ver Arquivo
@@ -1,126 +0,0 @@
/*
+----------------------------------------------------------------------+
| HipHop for PHP |
+----------------------------------------------------------------------+
| Copyright (c) 2010- Facebook, Inc. (http://www.facebook.com) |
+----------------------------------------------------------------------+
| This source file is subject to version 3.01 of the PHP license, |
| that is bundled with this package in the file LICENSE, and is |
| available through the world-wide-web at the following url: |
| http://www.php.net/license/3_01.txt |
| If you did not receive a copy of the PHP license and are unable to |
| obtain it through the world-wide-web, please send a note to |
| license@php.net so we can mail you a copy immediately. |
+----------------------------------------------------------------------+
*/
#include <runtime/eval/debugger/cmd/cmd_jump.h>
namespace HPHP { namespace Eval {
///////////////////////////////////////////////////////////////////////////////
void CmdJump::sendImpl(DebuggerThriftBuffer &thrift) {
DebuggerCommand::sendImpl(thrift);
thrift.write(m_label);
thrift.write(m_file);
thrift.write(m_line);
}
void CmdJump::recvImpl(DebuggerThriftBuffer &thrift) {
DebuggerCommand::recvImpl(thrift);
thrift.read(m_label);
thrift.read(m_file);
thrift.read(m_line);
}
void CmdJump::list(DebuggerClient *client) {
client->addCompletion(DebuggerClient::AutoCompleteFileNames);
}
bool CmdJump::help(DebuggerClient *client) {
client->helpTitle("Jump Command");
client->helpCmds(
"[j]ump", "jumps over one expression",
"[j]ump {line}", "goto the specified line",
"[j]ump {file}:{line}", "goto the specified line",
"[j]ump {label}", "goto the specified label",
nullptr
);
client->helpBody(
"This command changes program execution to the specified place without "
"executing remaining code on current line. When no label or source "
"location is specified, it jumps over just one expression without "
"evaluating it. This may be useful to not throw an exception when "
"breaks at a throw, for example."
);
return true;
}
bool CmdJump::onClient(DebuggerClient *client) {
if (DebuggerCommand::onClient(client)) return true;
if (client->argCount() > 1) {
return help(client);
}
string arg = client->argValue(1);
m_line = 0;
if (!arg.empty()) {
if (DebuggerClient::IsValidNumber(arg)) {
BreakPointInfoPtr loc = client->getCurrentLocation();
if (loc) {
m_file = loc->m_file;
}
if (m_file.empty()) {
client->error("There is no current source file. Please specify a file "
"name as well.");
return true;
}
m_line = atoi(arg.c_str());
} else {
size_t pos = arg.find(':');
if (pos == string::npos) {
m_label = arg;
} else {
m_file = arg.substr(0, pos);
m_line = atoi(arg.substr(pos + 1).c_str());
if (m_file.empty() || m_line <= 0) {
client->error("Invalid source location. Cannot jump there.");
return true;
}
}
}
}
client->sendToServer(this);
throw DebuggerConsoleExitException();
}
bool CmdJump::onClientVM(DebuggerClient *client) {
client->error("not supported\n");
return true;
}
bool CmdJump::onServer(DebuggerProxy *proxy) {
return true;
}
bool CmdJump::onServerVM(DebuggerProxy *proxy) {
return true;
}
bool CmdJump::match(InterruptSite *site) {
if (site) {
if (m_line) {
if (m_line == site->getLine0()) {
const char *file = site->getFile();
return BreakPointInfo::MatchFile(file, strlen(file), m_file);
}
} else {
// label is not implemented
}
}
return false;
}
///////////////////////////////////////////////////////////////////////////////
}}
-52
Ver Arquivo
@@ -1,52 +0,0 @@
/*
+----------------------------------------------------------------------+
| HipHop for PHP |
+----------------------------------------------------------------------+
| Copyright (c) 2010- Facebook, Inc. (http://www.facebook.com) |
+----------------------------------------------------------------------+
| This source file is subject to version 3.01 of the PHP license, |
| that is bundled with this package in the file LICENSE, and is |
| available through the world-wide-web at the following url: |
| http://www.php.net/license/3_01.txt |
| If you did not receive a copy of the PHP license and are unable to |
| obtain it through the world-wide-web, please send a note to |
| license@php.net so we can mail you a copy immediately. |
+----------------------------------------------------------------------+
*/
#ifndef incl_HPHP_EVAL_DEBUGGER_CMD_JUMP_H_
#define incl_HPHP_EVAL_DEBUGGER_CMD_JUMP_H_
#include <runtime/eval/debugger/debugger_command.h>
namespace HPHP { namespace Eval {
///////////////////////////////////////////////////////////////////////////////
DECLARE_BOOST_TYPES(CmdJump);
class CmdJump : public DebuggerCommand {
public:
CmdJump() : DebuggerCommand(KindOfJump) {}
virtual void list(DebuggerClient *client);
virtual bool help(DebuggerClient *client);
virtual bool onClient(DebuggerClient *client);
virtual bool onClientVM(DebuggerClient *client);
virtual bool onServer(DebuggerProxy *proxy);
virtual bool onServerVM(DebuggerProxy *proxy);
virtual void sendImpl(DebuggerThriftBuffer &thrift);
virtual void recvImpl(DebuggerThriftBuffer &thrift);
bool match(InterruptSite *site);
private:
std::string m_label;
std::string m_file;
int32_t m_line;
};
///////////////////////////////////////////////////////////////////////////////
}}
#endif // incl_HPHP_EVAL_DEBUGGER_CMD_JUMP_H_
+3 -3
Ver Arquivo
@@ -70,9 +70,9 @@ bool CmdThread::help(DebuggerClient *client) {
"\n"
"Some debugging commands will automatically turn thread mode to sticky. "
"These include continue, step, next or out commands with a counter of "
"more than 1. Or a jump command. These commands imply non-interruption "
"from another thread. The mode will remain even after these commands "
"until '[t]hread [n]ormal' is issued."
"more than 1. These commands imply non-interruption from another thread. "
"The mode will remain even after these commands until '[t]hread [n]ormal' "
"is issued."
"\n"
"When multple threads hit breakpoints at the same time, use '[t]hread "
"[l]ist' command to display their indices, which can be used to switch "
+1 -2
Ver Arquivo
@@ -1517,7 +1517,7 @@ void DebuggerClient::setTutorial(int mode) {
const char **DebuggerClient::GetCommands() {
static const char *cmds[] = {
"abort", "break", "continue", "down", "exception",
"frame", "global", "help", "info", "jump",
"frame", "global", "help", "info",
"konstant", "list", "machine", "next", "out",
"print", "quit", "run", "step", "thread",
"up", "variable", "where", "x", "y",
@@ -1569,7 +1569,6 @@ do { \
case 'g': MATCH_CMD("global" , CmdGlobal );
case 'h': MATCH_CMD("help" , CmdHelp );
case 'i': MATCH_CMD("info" , CmdInfo );
case 'j': MATCH_CMD("jump" , CmdJump );
case 'k': MATCH_CMD("konstant" , CmdConstant );
case 'l': MATCH_CMD("list" , CmdList );
case 'm': MATCH_CMD("machine" , CmdMachine );
@@ -102,7 +102,6 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift,
case KindOfFrame : cmd = DebuggerCommandPtr(new CmdFrame ()); break;
case KindOfGlobal : cmd = DebuggerCommandPtr(new CmdGlobal ()); break;
case KindOfInfo : cmd = DebuggerCommandPtr(new CmdInfo ()); break;
case KindOfJump : cmd = DebuggerCommandPtr(new CmdJump ()); break;
case KindOfConstant : cmd = DebuggerCommandPtr(new CmdConstant ()); break;
case KindOfList : cmd = DebuggerCommandPtr(new CmdList ()); break;
case KindOfMachine : cmd = DebuggerCommandPtr(new CmdMachine ()); break;
+1 -1
Ver Arquivo
@@ -51,7 +51,7 @@ public:
KindOfGlobal = 7,
KindOfHelp = 8,
KindOfInfo = 9,
KindOfJump = 10,
KindOfJump_UNUSED = 10,
KindOfConstant = 11,
KindOfList = 12,
KindOfMachine = 13,
+20 -62
Ver Arquivo
@@ -16,7 +16,6 @@
#include <runtime/eval/debugger/debugger_proxy.h>
#include <runtime/eval/debugger/cmd/cmd_interrupt.h>
#include <runtime/eval/debugger/cmd/cmd_flow_control.h>
#include <runtime/eval/debugger/cmd/cmd_jump.h>
#include <runtime/eval/debugger/cmd/cmd_signal.h>
#include <runtime/eval/debugger/cmd/cmd_machine.h>
#include <runtime/eval/debugger/debugger.h>
@@ -131,7 +130,6 @@ void DebuggerProxy::switchThreadMode(ThreadMode mode,
m_thread = (int64_t)Process::GetThreadId();
}
if (mode == Normal) {
m_jump.reset();
m_flow.reset();
}
}
@@ -241,7 +239,7 @@ void DebuggerProxy::interrupt(CmdInterrupt &cmd) {
if (!blockUntilOwn(cmd, true)) {
return;
}
if (processJumpFlowBreak(cmd)) {
if (processFlowBreak(cmd)) {
while (true) {
try {
Lock lock(m_signalMutex);
@@ -431,7 +429,7 @@ bool DebuggerProxy::blockUntilOwn(CmdInterrupt &cmd, bool check) {
Lock lock(this);
if (m_thread && m_thread != self) {
if (check && (m_threadMode == Exclusive || !checkBreakPoints(cmd))) {
// jumps and flow control commands only belong to sticky thread
// Flow control commands only belong to sticky thread
return false;
}
m_threads[self] = createThreadInfo(cmd.desc());
@@ -444,7 +442,6 @@ bool DebuggerProxy::blockUntilOwn(CmdInterrupt &cmd, bool check) {
m_threadMode = Normal;
m_thread = self;
m_newThread.reset();
m_jump.reset();
m_flow.reset();
}
}
@@ -454,7 +451,7 @@ bool DebuggerProxy::blockUntilOwn(CmdInterrupt &cmd, bool check) {
// The breakpoint might have been removed while I'm waiting
return false;
}
} else if (check && !checkJumpFlowBreak(cmd)) {
} else if (check && !checkFlowBreak(cmd)) {
return false;
}
@@ -473,69 +470,41 @@ bool DebuggerProxy::checkBreakPoints(CmdInterrupt &cmd) {
}
// Check if we should stop due to flow control, breakpoints, and signals.
bool DebuggerProxy::checkJumpFlowBreak(CmdInterrupt &cmd) {
TRACE(2, "DebuggerProxy::checkJumpFlowBreak\n");
bool DebuggerProxy::checkFlowBreak(CmdInterrupt &cmd) {
TRACE(2, "DebuggerProxy::checkFlowBreak\n");
// If there is an outstanding Ctrl-C from the client, go ahead and break now.
// Note: this stops any flow control command we might have in-flight.
if (m_signum == CmdSignal::SignalBreak) {
Lock lock(m_signumMutex);
if (m_signum == CmdSignal::SignalBreak) {
m_signum = CmdSignal::SignalNone;
m_jump.reset();
m_flow.reset();
return true;
}
}
// jump command skips everything until the file:line or label is reached
bool jumpBreak = false;
if (m_jump) {
InterruptSite *site = cmd.getSite();
if (site) {
if (!m_jump->match(site)) {
site->setJumping();
return false;
}
m_jump.reset();
jumpBreak = true;
}
// 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.
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);
}
if (!jumpBreak) {
// 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.
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);
}
bpShouldBreak = checkBreakPoints(cmd);
if (!fcShouldBreak && !bpShouldBreak) {
return false; // this is done before KindOfContinue testing
}
bpShouldBreak = checkBreakPoints(cmd);
if (!fcShouldBreak && !bpShouldBreak) {
return false; // this is done before KindOfContinue testing
}
return true;
}
bool DebuggerProxy::processJumpFlowBreak(CmdInterrupt &cmd) {
TRACE(2, "DebuggerProxy::processJumpFlowBreak\n");
if (m_jump) {
switch (cmd.getInterruptType()) {
case SessionEnded:
case RequestEnded:
case PSPEnded:
cmd.setPendingJump();
m_jump.reset();
break;
default:
break;
}
}
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)) {
@@ -584,17 +553,6 @@ void DebuggerProxy::processInterrupt(CmdInterrupt &cmd) {
}
return;
}
if (res->is(DebuggerCommand::KindOfJump)) {
m_jump = static_pointer_cast<CmdJump>(res);
if (m_threadMode == Normal) {
switchThreadMode(Sticky);
}
InterruptSite *site = cmd.getSite();
if (site) {
site->setJumping();
}
return;
}
if (res->is(DebuggerCommand::KindOfQuit)) {
Debugger::RemoveProxy(shared_from_this());
throw DebuggerClientExitException();
+2 -4
Ver Arquivo
@@ -49,7 +49,6 @@ class CmdInterrupt;
DECLARE_BOOST_TYPES(DebuggerProxy);
DECLARE_BOOST_TYPES(DebuggerCommand);
DECLARE_BOOST_TYPES(CmdFlowControl);
DECLARE_BOOST_TYPES(CmdJump);
class DebuggerProxy : public Synchronizable,
public boost::enable_shared_from_this<DebuggerProxy> {
@@ -128,7 +127,6 @@ protected:
StringDataMap m_breaksEnterFunc;
CmdFlowControlPtr m_flow; // c, s, n, o commands that can skip breakpoints
CmdJumpPtr m_jump;
Mutex m_signalMutex; // who can talk to client
AsyncFunc<DebuggerProxy> m_signalThread; // polling signals from client
@@ -139,8 +137,8 @@ protected:
// helpers
bool blockUntilOwn(CmdInterrupt &cmd, bool check);
virtual bool checkBreakPoints(CmdInterrupt &cmd);
bool checkJumpFlowBreak(CmdInterrupt &cmd);
virtual bool processJumpFlowBreak(CmdInterrupt &cmd);
bool checkFlowBreak(CmdInterrupt &cmd);
virtual bool processFlowBreak(CmdInterrupt &cmd);
void processInterrupt(CmdInterrupt &cmd);
virtual void processFlowControl(CmdInterrupt &cmd);
virtual bool breakByFlowControl(CmdInterrupt &cmd);