This diff removes initializing stores to TypedValue._count, renames
_count to m_aux, and makes m_aux a union with members typed
and named according to their specialized uses. The few remaining
uses of that field for random tweaks are more obvious and easy to
grep for.
TypedValue no longer extends Value, (allowing m_data to move to a
different offset in the future), and Variant now extends TypedValue,
so we only have to maintain one definition.
HphpArray now explicitly uses TypedValue.m_pad instead of overlapping
TypedValue with an anonymous struct, again so we don't have to maintain
another structure to match TypedValue's layout.
The JIT's were using offsetof(TypedValue, _count) all over the place
for access to String/Array/Object/RefData::_count. Instead, use
FAST_REFCOUNT_OFFSET.
This actually was pretty broken already. If you defined a new function in the ##create_function## string it would return that function but it wouldn't move it to the right unit so it can't execute. A big mess. For example:
$ cat a.php
<?php
$a = create_function('', 'function b() { return 3; }');
$a();
var_dump(b());
$ php a.php
HipHop Fatal error: Undefined function: b in /data/users/ptarjan/hphp/hphp/a.php on line 4
$ hhvm a.php
int(3)
Support == and != operators for collections. Also fix some bugs the <, <=,
>, and >= operators when comparing two objects or when comparing an array
with an object.
We only used it to get the values of certain class constants,
and to define the ObjectStaticCallbacks for every class.
We can put the class constants directly into the class_map
(we should have done that before for perf reasons), and then
the only remaining use of ObjectStaticCallbacks is to proxy
the Class* for each builtin class. So just use the Class*
directly.
Once this is in, Im just a small step away from eliminating
make -C hphp/system - so Im leaving a lot of dead code here.
Its going to be easier to delete it en masse, rather than
try to pick and chose now.
This basically targetted symbols.php, and Globals, but ended up
killing a lot more. I could keep adding more and more, but
this seems like a good point to stop and continue with
another diff.
The logic for unwinding generator frames was assuming that the previous
frame was always a VM frame. This used to be a correct assumption, but now
we have invokeContFunc which allows native code to call generator functions.
This diff fixes the unwinder to correctly computer the bottom of the eval
stack in the case where the previous frame is a native frame. It also
updates invokeContFunc to properly decref the return value.
Instead of having the body of the closure be in the ##__invoke()## on the ##Closure## class, instead we make an anonymous function on the real class and put the body there. The signature for this function is:
function methodForClosure$1234($arg1, $arg2, ..., $use1, $use2, ...)
and then ##__invoke## now just takes all the params that were passed to it, puts them as the first args to the anonymous function, then takes all the use variables it had saved up and passed them in as the next params.
I tried to not have an ##__invoke## at all, but I ended up basically doing the same parameter and use var repacking in iopFCall (and would have had to do it in x86 code too). I opted for doing the rejiggering in bytecode. If I did it in raw PHP I think it would have been much slower with many ##func_get_args()## and array operations.
We don't display the stack frame for ##isNoInjection()## functions, but the backtrace does set the previous call lines wrong.
This came up when I made the Closure::__invoke to be hidden, but the line numbers were wrong.
I was debugging this method and jumping into a macro that is 20 lines is much harder than a method. Shouldn't the compiler inline it if that is the right thing to do anyways?
There also was a hidden variable ##numArgs## which is what was messing me up.
It's not ok for the unwinder to use a reference to elements
living in the m_faults array, since the unwinder can re-enter the VM
when calling destructors (or the FunctionExit hook). If one of those
re-entries does exception handling, it can modify m_faults.
Additionally, gets rid of VMPrepareThrow and instead just throw Object
and use the same case as we do when exceptions came from an extension.
I had to fix an assembler test catch handler to actually catch to keep
the assertion about m_faults on re-entry correct.
When interp'ing an N-instruction tracelet, we ask the interpreter
to step through N instructions. The interpreter treats FCall as one of
those instructions, and happily starts interp'ing into the callee. This can
cause us to form weird tracelets in the callee that have no business
existing. Instead, break on any control-flow we encounter.
Array is already a SmartPtr, wrapping it in another is pointless.
Except it turns out that there are include dependencies preventing
us from using Array here. Use boost::intrusive_ptr to wrap an
ArrayData instead.
This diff refactors some of the VM's logic for iterators (with a focus on
mutable iteration), delivering several improvements:
1) MIterCtx was renamed to MArrayIter, and the m_key and m_val fields
were eliminated.
2) Eliminated the need for MArrayIter to dynamically allocate a
MutableArrayIter object, and removed other layers of indirection as
well.
3) Reduced the size of HPHP::VM::Iter from 64 bytes down to 32 bytes.
4) Removed the "if (siPastEnd())" check when adding a new element to an
HphpArray or a ZendArray.
5) Moved all of the iterator logic into a single .cpp file.
This diff reworks FullPos's to point to current element instead of pointing
to the next element. It also splits up the IterFree instruction into two
instructions (IterFree and MIterFree). These changes allowed various logic
to be simplified and data structures to be reduced in size. There is
definitely more opportunity for refactoring, but I know the JIT helpers for
iteration have been carefully tuned and so I'll leave further refactoring
for future diffs.
Finally, I spent a little time cleaning up the bytecode spec a bit, mostly
with respect to iteration.
When we skip EHEnts for catch blocks that we aren't going to
enter, we still need to count them in the handledCount so that we run
the appropriate fault funclet (see the unit test). Before this
change, it would fail to account for the catch handler and end up
running the fault funclet twice, leading to an assertion about
IterFree trying to free an already-freed iterator.
Since we now use the original throw location to figure out
which EHEnt applies when propagating from a fault funclet, we need to
check whether we've already unwound the ActRec before doing it again.
Doing it again could cause issues with nested FPI regions (hitting an
assert in sandboxes).
... is that once you put them into the codebase, the odds of them
showing up where you dont want them go up dramatically.
With a gcc 4.7.1 -O0 build I hit a consistent crash in production
where debugBacktrace had stepped too far, thinking that the
first non-vm frame was in fact a vm frame. It did so because
it looked at the word above the frame, and saw that it contained
c_Continuation::kMagic.
It turns out that fixupWork's isVMFrame left kMagic in $rdx if the
frame really was a generator ($rdx was dead, but still). We then
return from the tc via a serviceReq that doesnt need $rdx (so doesnt
set it), and then (with a following wind, so that $rdx hasnt been
smashed yet) enterTCHelper stores $rdx in info.args[1].
By another staggering coincidence, info.args[1] is at exactly the
right address to make the call into enterTCHelper /look/ like a
a continuation (based on kMagic). So if we then have a catch
which re-enters the TC there's a good chance kMagic is still there
and the next debugBacktrace (or uncaught exception) will crash.
This diff rewrites everything in terms of the C++ stack; we
now say that its a VM frame if its not on the C++ stack.
In the interpreter, neither ContEnter nor ContExit checked the
surprise flags, resulting in the time being attributed to the
callers. In the jit, ContEnter checked the surprise flags, but
ContExit did not, resulting in what appeared to be highly
recursive profiles.
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.
mwilliams