Expected test output for hphpd command line tests is difficult to read when colorization is present. Although there was an internal switch to turn off colorization (for the benefit of API clients), there was no command line option and no config file option to turn off colorization. This diff adds such an option and also fixes a few places where colorization was added regardless of the value of the internal switch.
- boost::shared_ptr now has "explicit operator bool", which means we
can't "return <a shared ptr>" from a function with return type bool.
- Our use of shared_ptr<T[]> in debugger_client.h was screwed.
Unfortunately it's not straightforward to make it a unique_ptr as per
our discussion; there are other objects that call setLiveLists, and
it's not clear to me that this is a total transfer of ownership. I'm
just fixing the immediate problem using shared_array. (Also made the
typedef less confusing, hopefully.)
- Our specialization of graph_traits<G> for ControlFlowGraph didn't
define the null_vertex member function. This didn't matter in older
versions of boost because they didn't use it internally, but now they
do (the call to depth_first_search in control_flow.cpp was failing to
compile).
The proximal purpose of this differential is to fix the debugger client so that it does not write raw strings to the console (with control characters doing their thing to the console and unprintable characters pretending not to exist). This change in behavior means that the serializer used to convert values to strings can no longer be instantiated with the PrintR option, as it has been in a subset of cases. (The reason being that print_r() must not change its behavior.)
Rather than introduce yet another serializer option, I decided to always make the debugger client serialize user visible values using the existing DebuggerDump option, which is already used in a number of such cases and which has no other use. To make the overall change less painful for users, I preferred to change behavior of DebuggerDump to be more like PrintR in a number of cases, primarily the formatting of objects (where the current behavior is to make a JSON like string).
This does not impact the behavior of the debugger client API or the behavior of FBIDE, since these do their own thing directly with the DebuggerSerialization format and never see the result of the DebuggerDump format.
Log details about debugger usage from both client and server side. Added a new runtime option to control whether the logging is used or not. There's a base class for a usage logger defined and used under runtime/eval/debugger.
I'm adding the count of connected debuggers, and whether or not the process is hphpd, to the crash reports. In another diff I'll wire these up to hphpcrash_categorizer.py and get these as columns in the Hphpcrash Scuba data set so we can filter crash reports based on whether or not it's from hphpd, and whether or not a server was being debugged when it crashed.
This is a rather mechanical refactor that uses references (&) rather than pointers (*) for parameters that are not permitted to ever be given null arguments. In effect, the onus for checking null pointers is shifted from the callee to the caller. The & type annotation makes it clear that the callee is not prepared to deal with a null pointer.
These commands invariably return true. This is more than a tad confusing to a new reader of the code. Refactor them to return void. Remove a few pieces of dead code that would log something if they ever returned false.
I was learning from @jdelong and he said that you should use
double quotes for local includes and angle brackets for library
includes. I asked why our code was the way it was, and he said he wanted
to clean it up. I beat him to it :)
Conflicts:
hphp/runtime/base/server/admin_request_handler.cpp
hphp/runtime/vm/named_entity.h
Set the stopped flag for the debugger proxy when it receives a quit command, so that the proxy exits a bit quicker and cleaner. Add a timeout to the cleanup quite of test_debugger, so that it does not hang when a test goes wrong. Add some more tracing.
The debugger's Instrument command does not work with the JIT; any instrumentation requests will be ignored for jitted code. We believe that no one is using this at this time. I'm starting with a small diff to disable the command and remove the impact it has on the interpreter for now, just in case we're wrong. Once the command has been disabled for a few weeks I'll come back and remove all of the code (task 2376711).
InstHelpers was just pure dead code, so I nuked it.
Added comments to every method in cmd_breakpoint.cpp. Also renamed some methods to make their intent more obvious. Moved a few implementation methods from the public to protected space.
Removed the litstr overloads of Array::rvalAt, rvalAtRef, lval,
lvalPtr, lvalAt, and set. The main one left to do is operator[].
Fixed a bug in f_get_html_translation_table() where we were copying
the null terminator of what should be a one-character string, thus
creating a two-character string with s[1] == 0. (cc @jdelong)
In class Extension, store a String for the name instead of
const char*. cc @sgolemon
This gets rid of the (litstr) StringData and StackStringData
constructors, but keeps String(litstr). Also rename all
the instances of AttachLiteral to CopyString, since they now
mean the same thing.
When debugging a script using a local VM (i.e. when there is no remote VM),
the cleanup actions performed by the run command caused the debugger client
to shut down as well. Since this seems to be intentional, processing of the
run command now restarts the debugger client and proxy. Also, following a
ctrl-C, the m_lastLocFilter field must be cleared, otherwise an empty endless
loop cannot be broken into for a second time.
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.
I found myself getting confused, thinking that proxy->send(cmd) sends a command to the proxy, when in fact it causes the proxy to send the command to the client. These are now named sendToClient, sendToServer and recvFromServer so that future readers (including my future self) will not make this mistake again.
It's enabled if the HPHP_TRACE_FILE is "/dev/stdout" or
"/dev/stderr" and also isatty(), or you can set HPHP_TRACE_TTY in your
environment to override it (e.g. if you like to pass to less -R).
g++-4.7.1 treats "FOO"bar as a c++-11 literal operator, even
if bar is a macro with an expansion such as "BAR" - so add a space
after the quote (this seems like a bug, and I fixed a bunch of these
a while ago, but we just added a slew of PRI*64 macros which break
under 4.7.1).
Also, it warned that "explicit by-copy capture of 'this' redundant"
for a lambda declared [=, this] - so I removed the this.
We also needed more than the 60 levels of template expansion that was
allowed by the makefile.
Per @mwilliams' suggestion, this is the first stage in a staggered approach to replacing int64 with int64_t. More precisely I inserted "typedef ::int64_t int64;" in util/base.h and dealt with the consequences.
This change is mostly for FB internal organizational reasons.
Building is not effected beyond the fact that the target now
lands in hphp/hhvm/hhvm rather than src/hhvm/hhvm.
Herman Venter