In repo mode this does the right thing and returns the right constant, but for non-repo it thinks it is an undefined non-namespace constant. I'll have to do something more hardcore if that starts to matter
While I was in there, I made the correct name for `ReflectionParameter` that was added in 5.4 instead of the draft we copied.
Continuation argument values are copied to the continuation object and the locals are unset - in this way we don't need to change the reference coutners.
We were maintaining a flag that said if we were interpreting on the EC. It was only used in the implementation of hphp_break(), for pretty minimal benefit. I yanked it… it seems fine to me to throw the switch exception if someone steps out of the hard break even if we're already interpreting.
Also yank out a vestige of the previous stepping and breakpoint logic from the dispatch loop I noticed today. I'd previously missed it, but I made a change a while ago to put the state of the last location filter into the hands of the flow control commands. This was unnecessary and should have been removed then.
This is to clear the runway for getting rid of
GlobalNameValueTableWrapper. It moves aside these three items that were
in there for no particular reason other than convenience. I moved them
aside into another struct that I arena-allocate and initialize at the
same time as the global VarEnv (which initializes the GlobalNVTW).
I called the struct where these live "EnvConstants" since they look like
constants to PHP but their values are determined at startup time (by the
environment, like whether we're in server mode). lvalProxy doesn't fit
that mold, but oh well.
This converter enables much more thorough testing of the
region translator. It currently passes all tests, though it does punt
on one or more Tracelets in roughly 1/6th of them. The two big
unimplemented features for the whole pipeline are interpOne (should be
pretty easy) and parameter reffiness checks (probably
nontrivial). I'll attack those in separate diffs next.
This was spewing on mediawiki and we need to update. It is backwards compat.
I tried to put the constants in PHP but failed. I spent 30 mins on it. I'll come back to it next time I get the itch.
Closes#812
I noticed that directorty structure of hphp/system was a bit scattered, so
I consolidated things to reduce the total number of folders and to put
related things together with each other.
This diff moves the contents of "hphp/system/classes_hhvm" into
"hphp/system", it moves the contents of "hphp/system/lib" into
"hphp/system", moves "hphp/idl" to "hphp/system/idl", and moves the
contents of "hphp/system/globals" into "hphp/system/idl".
If a function was called via (re)enterVM, and it didn't yet
have a prolog, and it was unable to get the write lease, and it was
intercepted, and the intercept handler asked for the original function
to be skipped, fcallHelper didn't return correctly, typically resulting
in the original function being re-executed, and then re-intercepted -
except that it was one level deeper in the vm-nesting hierarchy.
Eventually, it crashed.
Hopefully a little bit simpler, and duplicating less logic
with Stack::toString. Contains some fixes to visitStackElems to make
it work for this (it is printing some stacks slightly incorrectly
right now, too ... :)
Move all the stack unwinding code to its own module, delete
some redundant enumerations, document a few things. Gets rid of most
of the remnants of the old setjmp/longjmp-based implementation at the
enterVM level but doesn't do much to the unwinder itself (coming in
separate diff to hopefully be easier to review).
This stuff almost is an abstraction layer, but it's in a
strange place. Also, the layout of TypedValue isn't something we've
been using through an abstraction layer, and if we want to later I
think we won't want to do so using this one.
Remove unnecessary optimization that can be achieved in a more general
way and simplify continuation creation code.
VMExecutionContext::createContinuationHelper was renamed to
VMExecutionContext::createCont{Func,Meth}. The createContFunc no longer
takes this/class arguments, the createContMeth transfers them in one
Type::Ctx pointer that is used natively by ActRec's m_this. Since the
whole logic of this function is to set this single field, the logic is
now simpler.
Interpreter: iopCreateCont() just passes the m_this field of the parent
ActRec.
Translator: CreateCont opcode loads m_this field using LdCtx opcode.
This opcode optimizes into LdThis, which optimizes into
DefInlineFP->SpillFrame->object and allows frame to be eliminated, a
case previously covered by InlineCreateCont.
This diff uncovered a bug in trace builder, where LdCtx in static
methods could be optimized into LdThis, if the method was called thru
object. Fixed.
Save 8 bytes of m_args and its initialization for Continuations without
func_get_args() call (does not save real memory due to 16-byte alignment).
Store variable arguments in optional local.
Vector statements ending with a SetProp now properly free
memory when run with JIT enabled. Additionally, in both
JIT and interpreted mode memory will still be freed if
exceptions are thrown during evaluation of vector operations.
There were multiple issues with flow control when exceptions occur. Fixed these by ditching the reliance on the exception thrown interrupt and introduce an exception handler interrupt, which indicates control is about to pass to a catch clause. This gives us much better insight into how execution is flowing and how we might need to adjust an in-flight stepping operation.
Users of m_localsOffset used this offset to calculate pointer where
local variables starts. Since they are stored in the reverse order, the
equation was:
cont_ptr + m_localsOffset + num_locals - local_id - 1
We are already storing pointer to ActRec (m_arPtr), which is equal to:
cont_ptr + m_localsOffset + num_locals
This diffs kills m_localsOffset and uses m_arPtr to simplify
calculations. Continuation fields were rearranged so that
sizeof(c_Continuation) is reduced by 16 bytes.
I need to walk a live eval stack, handling FPI regions and
generators, and duplicating that logic in yet another place seems
wrong. This makes the Stack::toString stuff use a reusable one---at
some point I'll see if the unwinder can use it as well, but it works
also for my use case. Also, Stack::toString was broken for mutable
array iterators---fixes that. And use a union now for struct Iter.
And don't include bytecode.h from func.h.
m_obj in c_Continuation is redundant. It is only used at the time
Continuation is created. First, it is set in init() call. Few
instructions later, it is read so that $this in the Continuation's
ActRec can be populated.
Kill it.
nvSet() only casts the value from TypedValue* to const Variant&; do it
at callsites. Inlined array_setm_ik1_v0() and array_setm_s0k1_v0() into
their only remaining callsites in translator-runtime.cpp.
This is the first results from profiling callsites of
StringData::initLiteral. This diff converts a handful more
string literals to StaticString, removes overloaded Variant
comparison operators (operator==, etc), and avoids
constructing new strings in a few cases in tvCastToString
and tvCastToStringInPlace.
We had a lot of odd behavior with both Next and Out. Previously the debugger would interpret the world until it saw that it was no longer on a specific line, or at a specific stack depth. I changed it recently to let the program run normally, and use an internal breakpoint to control step outs. Next's become like a step out temporarily when you descend into a function and need to get back out to the original line, so some bugs with out showed as bugs with Next, too.
Specifically, any time more PHP code was executed as a "side-effect" of a non-fcall instruction, step out would get lost. In these cases the stack trace gives us the offset of the instruction causing the PHP to run, not the instruction that control will return to as is the case with a Fcall. A breakpoint set there would get missed. This changes the step out logic to recognize such cases (via the fact that a nested VM state was pushed to execute the code) and step out more intelligently. We look at the instruction in question, and determine where execution may go, which might be multiple places. I also made a small change to ensure that we don't stop in generated functions when stepping out, which cleans up the iterator experience quite a bit, and sets us up for proper generator stepping, which will come next.
runtime/eval is a relic of a bygone era. As long as we're cleaning up
our directory structure, let's move FileRepository (the only remaining
thing in runtime/eval/runtime) to where it makes sense.
runtime/eval still contains the debugger, which would probably make more
sense as runtime/debugger, but I don't want to throw a wrench in the
works for @mikemag and @hermanv unnecessarily.
I was going to #include translator.h in a header I had for
talking to the region selector thing and decided to just get this over
with instead. (It shouldn't need to #include that.) Found a few
other unused things to remove while at it.
Previously it returned a bool to say whether or not it had
succeeded, but always initialized the iter. The other iterators
branch on failure.
This adds the branch target which brings it closer to the other
iterators, and avoids the need to do a (pointless) CIterFree on
the failure path just to keep the validator happy.
I've also added a translation for it.
This was barely a demonstrable win in sandbox mode when I first wrote
it and it's not even used in hhir. It's probably been bitrotting and is causing
crashes for some people. Time to say goodbye.
Most of the SpillStacks we do are just to keep the in-memory VM stack
clean in case a helper call throws an exception. Many of these exceptions are
vanishingly rare, so let's stop making the fast path slower for them. This diff
adds support for catch traces, which are just like normal exit traces with one
major exception: they're never jumped to from translated code. If we're
unwinding a TC frame and discover that the current rip has a catch trace
registered, the unwinder will execute it before resuming unwinding.
The unwinder parts were fairly straightfoward, then I ran into a bunch of
issues with SetM. The SetElem instruction sometimes consumes a reference to one
of its inputs, which is bad news for our optimizations. To make everything work
again, I changed SetElem to throw an exception in cases where it would've
previously decreffed the value input. This is a special exception that the new
unwinding code recognizes. When one of these is caught, the catch trace
executed for that TC frame will finish up the vector instruction and push the
value provided by the exception on the stack. This allows us to optimize most
traces under the assumption that SetM's output is the same as its input,
letting the catch trace clean things up if the helper decides that's not the
case and throws.
There is one common case where SetM's output isn't the same as its input:
setting an offset in a string base returns a new String. Luckily, we can detect
most of these at compile time so SetElem returns a new StringData* when the
base is known to be a string. If the base might be a string, SetElem will
return nullptr if the base wasn't a string, and a StringData* if it was. We
test the output in these cases and side exit if the return value of SetElem is
non-null (I have yet to see this side exit happen outside of artificially
created test cases).
Once I got things working in the vector translator, I went through every other
call to spillStack and exceptionBarrier, replacing them with catch traces as
appropriate.
Functions like array_filter and array_map need "withRef" semantics,
and while it can be simulated in php via copy-on-write, doing so
is very inefficient.
This adds
SetWithRefLM
SetWithRefRM
- similar to SetM but binds the value if it was a reference. L reads a local, R reads a return value
WIterInit
WIterInitK
WIterNext
WIterNextK
- essentially the same as the corresponding opcodes without W, but the value local is set by reference if the array element was a reference.
Will be needed for array_filter/array_map etc
This sets things up so that if we define a builtin in systemlib, we rename
the corresponding c++ builtin with the prefix __builtin_, so its still available
(in case the php builtin wants to delegate some edge cases, and to make
it easy to run comparisons between the php and c++ implementations).
Also did a little reorganization to get rid of Func::isPHPBuiltin,
and use an Attr to identify functions as builtins. C++ builtins can
still be identified by checking the Func::info() method. This is needed
to allow builtin methods defined in php (such as array_map) to lookup their
arguments in the correct context.
The change to stop zeroing locals during RetC was too
aggressive---we stopped doing it during unwinding also. When
unwinding, we need to zero locals and $this because another
destructing object may still run debug_backtrace, and we won't have
the *pc == OpRet{C,V} trick to tell us to ignore the junk.
Make sure m_received is always null at ContSend, ContRaise,
and ContNext, and this information to generate better code for these
bytecode instructions.
Paul Tarjan