Clean up how the DebuggerDummyEnv is set up and torn down

A recent diff called my attention to the logic in enterDebuggerDummyEnv()
and exitDebuggerDummyEnv() and I noticed it didn't look quite right.

The first time enterDebuggerDummyEnv() is called it creates a frame on an
empty call stack, but then exitDebuggerDummyEnv() does not correctly tear
down this frame and null out m_fp and m_pc, and this leads to subtle issues.
For example, invokeFunc() checks if m_fp is null to decide whether to call
enterVM() or reenterVM(). I found a case with the "flow_gen_excep.php" test
where invokeFunc was incorrectly calling reenterVM (because m_fp hadn't
been nulled out) and it was pushing bogus VM state info into m_nestedVMs.
This in turn was causing the logic in CmdNext::onBeginInterrupt() to get
confused when comparing the original stack depth with the current stack
depth.

This diff updates exitDebuggerDummyEnv() to correctly tear down the frame
and null out m_fp and m_pc, and it updates enterDebuggerDummyEnv() to
assume the callstack is always empty (which should be the case). I've also
beefed up the asserts in both of these methods.
Esse commit está contido em:
Drew Paroski
2013-06-25 22:35:35 -07:00
commit de Sara Golemon
commit e7b90fa691
3 arquivos alterados com 43 adições e 18 exclusões
+36 -18
Ver Arquivo
@@ -2693,30 +2693,48 @@ void VMExecutionContext::evalPHPDebugger(TypedValue* retval, StringData *code,
}
void VMExecutionContext::enterDebuggerDummyEnv() {
static Unit* s_debuggerDummy = nullptr;
if (!s_debuggerDummy) {
s_debuggerDummy = compile_string("<?php?>", 7);
}
if (!getFP()) {
assert(m_stack.count() == 0);
ActRec* ar = m_stack.allocA();
ar->m_func = s_debuggerDummy->getMain();
ar->setThis(nullptr);
ar->m_soff = 0;
ar->m_savedRbp = 0;
ar->m_savedRip = reinterpret_cast<uintptr_t>(tx()->getCallToExit());
assert(isReturnHelper(ar->m_savedRip));
m_fp = ar;
m_pc = s_debuggerDummy->entry();
m_firstAR = ar;
}
static Unit* s_debuggerDummy = compile_string("<?php?>", 7);
// Ensure that the VM stack is completely empty (m_fp should be null)
// and that we're not in a nested VM (reentrancy)
assert(getFP() == nullptr);
assert(m_nestedVMs.size() == 0);
assert(m_nesting == 0);
assert(m_stack.count() == 0);
ActRec* ar = m_stack.allocA();
ar->m_func = s_debuggerDummy->getMain();
ar->setThis(nullptr);
ar->m_soff = 0;
ar->m_savedRbp = 0;
ar->m_savedRip = reinterpret_cast<uintptr_t>(tx()->getCallToExit());
assert(isReturnHelper(ar->m_savedRip));
m_fp = ar;
m_pc = s_debuggerDummy->entry();
m_firstAR = ar;
m_fp->setVarEnv(m_globalVarEnv);
m_globalVarEnv->attach(m_fp);
}
void VMExecutionContext::exitDebuggerDummyEnv() {
assert(m_globalVarEnv);
m_globalVarEnv->detach(getFP());
// Ensure that m_fp is valid
assert(getFP() != nullptr);
// Ensure that m_fp points to the only frame on the call stack.
// In other words, make sure there are no VM frames directly below
// this one and that we are not in a nested VM (reentrancy)
assert(m_fp->arGetSfp() == m_fp);
assert(m_nestedVMs.size() == 0);
assert(m_nesting == 0);
// Teardown the frame we erected by enterDebuggerDummyEnv()
const Func* func = m_fp->m_func;
try {
frame_free_locals_inl_no_hook<true>(m_fp, func->numLocals());
} catch (...) {}
m_stack.ndiscard(func->numSlotsInFrame());
m_stack.discardAR();
// After tearing down this frame, the VM stack should be completely empty
assert(m_stack.count() == 0);
m_fp = nullptr;
m_pc = nullptr;
}
// Identifies the set of return helpers that we may set m_savedRip to in an
@@ -9,6 +9,12 @@ break foo()
Breakpoint 1 set upon entering foo()
next
Program %s/flow_gen_excep.php loaded. Type '[r]un' or '[c]ontinue' to go.
next
Break on line 40 of %s/flow_gen_excep.php
39
40 main(1);
41
next
Breakpoint 1 reached at foo() on line 23 of %s/flow_gen_excep.php
22 function foo($a) {
@@ -14,6 +14,7 @@ next
next
next
next
next
break clear all
continue
quit