Debugger tests that run hphp in non local mode (i.e. with the "-h some-server:some-port" option specified) currently use a clunky specialized C++ test harness that invokes PHP scripts that talk to the debugger client via the obsolete client API. This revision moves those tests to the new PHP only framework. The debugger client is now controlled by piping in commands, just like the other debugger tests. The main difference is that the debugger client is connected to a server instance and a separate PHP driver is used to load the server, load the client, and then load pages from the server so that the client can hit breakpoints. It also checks that the client can respond to ctrl-c by actually sending a SIGINT signal to the client process.
The exit path for the debugger client has always been a little odd. We'd call a shutdown function which would destroy the client, then later call a stop function which would first make a new client, then stop it, then that one would get destroyed later. Made it so we stop and destroy just one client.
In HHVM (and HPHPc before it) we've been piggybacking resources on the
KindOfObject machinery. At the language level, resource is considered to
be a different type than object, and there are a number of differences
in behavior between objects and resources (ex. resources don't allow for
dynamic properties, resources don't work with the clone operator, the
"(object)" cast behaves differently for resources vs. objects, etc).
Piggybacking resources on the KindOfObject machinery has some downsides.
Code that deals with KindOfObject values often needs to check if the value
is a resource and go down a different code path. This makes things harder
to maintain and harder to keep parity with Zend. Also, these extra branches
hurt performance a little, and they make it harder for the JIT to do a good
job in some cases when its generating machine code that operates on objects.
This diff prepares the code base for a new KindOfResource type by adding a
new "Resource" smart pointer type (currently a typedef for the Object smart
pointer type) and it updates the C++ code and the idl files appropriately.
This diff is essentially a cosmetic change and should not impact run time
behavior. In the next diff (part 2) we'll actually add a new KindOfResource
type, detach ResourceData from the ObjectData inheritence hierarchy, and
provide a real implementation for the Resource smart pointer type (instead
of just aliasing the Object smart pointer type).
The [z]end command in hphpd runs the last manually entered snippet of php in "zend". If you typed 'z' before actually entering any php, it would segfault. Fixed a minor bug in Process::Exec, and present help if none entered. Also clarified that it will simply use your system's default php, and made it so you can override which exe to use with a hphpd config variable. The default is "php", since that is most commonly used.
Add a bit more usage logging for the debugger, and ensure that there is a common field between clients and servers thats easy to search on, i.e., sandbox id.
C++11 cleanup (clean up easy enums)
This is for runtime/base/... and ended up touching a lot of files
because it turns out we have a lot of reasonably behaved enums.
People sometimes make mistakes and launch hphpd without connecting to a server, but expect to be in the server environment when evaluating expressions. Modified the –h option to not require a host specification, and default to localhost, so "hphpd –h" is a valid way to launch now. Provide a warning on launch when no args are specified at all warning the user that they're not connected, and giving suggestions for how to connect if necessary. There are reasonable use cases for launching hphpd with no args (so, no connection and no local script to debug) and I didn't want to take those away by automatically connecting to a server at localhost if launched with no args. I'm hoping the warning is sufficiently helpful.
There was a similar-but-different event loop used when receiving command results from the server which was close, but not quite right. Unified it with the main event loop to ensure that all error cases are handled properly when we put up a prompt at a nested interrupt, like when hitting a breakpoint during an eval. The event loop is now shared, with a few different "kinds" to control some of the special needs of the loop when executed from a command. Most commands don't cause the server to run more PHP, so they don't change the machine state or cause more interrupts. But some do (Eval and Print) and certainly the top-level loop does, too. Made sure to throw a protocol error if any command causes this to happen when we don't expect it.
Cleanup a lot of hangs with either the debugger client or server in a variety of error conditions, mostly related to communication errors or the client or server exiting unexpectedly. One of the biggest fixes is that all cases where the client was left in a state where Ctrl-C wouldn't work have been fixed.
Remove lots of little snippets of dead code. If you see a function (or small set of functions/fields) deleted then it was actually dead.
I debated whether to keep throwing DebuggerClientExitException on the server, and I decided to keep it. I think it's reasonable that if you've got the server stopped and you quit the debugger that the request gets terminated rather than continuing to run.
I also considered a big change to the way Ctrl-C works, but ended up staying with what was there with just a bit of cleanup. We need to guard against people banging on Ctrl-C, which is a reasonable behavior, and I think it feels pretty reasonable with the updated message.
Finally, added many comments about how this stuff works.
Small stepping, which is stepping over sub-expressions (kinda), worked but was a little goofy. The mode was set on the client, passed over with control flow commands, placed on the execution context, then retrieved from there and used by those same flow commands. Removed the execution context part of it, since it was useless, and factored grabbing the offsets into the flow cmds where they belong instead of doing it all the time.
The run cmd also had some notion of small stepping, but you'll note it was never sent over the wire. Nuked that, since it never mattered anyway.
The code for reading in the personalized debugger options, always wrote out the file to account for the case where there may not be a file in the first place. This is not necessary and can lead to the file being changed when running various kinds of debugger tests. Since the tests use a checked in file, it is not desirable for the file to change as a result of a test run. With this change, the file will only be written out when it is missing in the first place. There are still some scenarios where it is possible to write a test that will change the file. Currently no such tests exist. Also, in those scenarios, the test may well want to verify that the file is changed, so more work will needed later on. Right now, that can wait.
The debugger prompt string depends on user/machine specific configuration settings, which makes its inclusion in expected test output problematic. There is already an option in (the user controlled) ~/.hphpd.hdf to turn off the prompt. Now there is an option in normal config files to do the same.
Add reasonable behavior for stepping within continuations (generators). Stepping over a yield now does what one would expect. When the generator is driven from a C++ extension like ASIO, the next logical execution point is after the yield statement, and that's where we'll stop now. When driven from PHP, say in a loop calling send(), the next execution point is in fact the call site of send(), so we go there. Stepping off the end of the generator function, goes to the caller of send(), or the caller of the C++ extension. Stepping _out_ of a generator driven by a C++ extension ensures that we go to the caller and not back into the generator again. The logic for both cases is exactly the same. The difference comes from the fact that we don't actually debug C++ extensions.
This also fixes a long-standing problem where breakpoints would interfere with control flow cmds on the same source line. This caused funny behavior, like taking multiple steps to get off of a breakpoint.
The ~/hphpd.hdf file provides the ability to customize (or turn off) colorization in hphpd as a user preference. The EnableDebuggerColor option turns of colorization for a single run only and should not persist into hphpd.hdf.
Herman Venter