We should use normal function renaming if the function is declared
during preloading itself, rather than afterwards.
This fixes a regression introduced by
68f80be9d1.
We should only disable early binding during the opcache_compile_file()
calls, not inside the preloading script or anything it includes.
The right condition to check for is whether we compile the file
without execution, as declaring classes is "execution".
This is a bit annoying: When preloading is used, types might be
resolved during inheritance checks, so we need to deal with CE
types rather than just NAME types everywhere.
While fixing bugs in mbstring, one of my new test cases failed with a strange
error message stating: 'Warning: Undefined array key 1...', when clearly the
array key had been set properly.
GDB'd that sucker and found that JIT'd PHP code was calling directly into
`zend_hash_add_new` (which was not converting the numeric string key to an
integer properly). But where was that code coming from? I examined the disasm,
looked up symbols to figure out where call instructions were going, then grepped
the codebase for those function names. It soon became clear that the disasm I
was looking at was compiled from `zend_jit_fetch_dim_w_helper`.
A recent bug fix regarding symlinks claimed:
> After resolving reparse points, the path still may be a reparse
> point; in that case we have to resolve that reparse point as well.
While that is basically correct, some reparse points may point to
inaccessible system folders (e.g. `IO_REPARSE_TAG_DEDUP` points to
"\System Volume Information"). Since we don't know details about
arbitrary reparse points, and are mainly interested in nested symlinks,
we take a step back, and only resolve `IO_REPARSE_TAG_SYMLINK` for now.
Close GH-6354.
While the initial threshold is set to 10001 roots, the threshold
adjustment logic may then set it to 10000. The exact value really
doesn't matter, but we should make it consistent.
This makes a number of related changes to the generator tree
management, that should hopefully make it easier to understand,
more robust and faster for the common linear-chain case. Fixes
https://bugs.php.net/bug.php?id=80240, which was the original
motivation here.
* Generators now only add a ref to their direct parent.
* Nodes only store their children, not their leafs, which avoids
any need for leaf updating. This means it's no longer possible
to fetch the child for a certain leaf, which is something we
only needed in one place (update_current). If multi-children
nodes are involved, this will require doing a walk in the other
direction (from leaf to root). It does not affect the common
case of single-child nodes.
* The root/leaf pointers are now seen as a pair. One leaf generator
can point to the current root. If a different leaf generator is
used, we'll move the root pointer over to that one. Again, this
is a cache to make the common linear chain case fast, trees may
need to scan up the parent link.
Closes GH-6344.
As filenames are no longer interned, we need to keep a reference
to the zend_string to make sure it isn't freed.
To avoid a nominal source compatibility break, create a new member
in the globals.
We need to perform trait scope fixup for both methods involved
in the inheritance check. For that purpose we already need to
thread through a separate fn scope through the entire inheritance
checking machinery.
If we have an undefined variable and null is not accepted by the
return type, we want to throw just the undef var error.
In this case this lead to an infinite loop, because we overwrite
the exception opline in SAVE_OPLINE and it does not get reset
when chaining into a previous exception. Add an assertiong to
catch this case earlier.
If the RHS has INDIRECT elements, we do not those to be added to
the LHS verbatim. As we're using UPDATE_INDIRECT, we might even
create a nested INDIRECT that way.
This is a side-quest of oss-fuzz #26245.
ZEND_HANDLE_EXCEPTION might call zend_observer_fcall_end() even if exception is cought by function. The fix moved zend_observer_fcall_end() into a right place and remove OBSERVER sepecialization for ZEND_HANDLE_EXCEPTION handler.
Remove arbitrary restriction that attributes cannot be applied
to property/constant groups.
The attribute applies to all elements of the group, just like
modifiers and types do.
See also https://externals.io/message/111914.
Closes GH-6186.
Instead of setting the old AST type to zero, replace the AST with
the compiled constexpr AST zval. This requires passing in a
zend_ast** instead of a zend_ast*.
This allows compiling ASTs containing constexprs multiple times
-- the second time, the existing compiled representation will be
resused.
This means we no longer need to copy the attributes AST for
promoted properties.
Make ReflectionAttribute::newInstance() respect the strict_types=1
declaration at the attribute use-site. More generally, pretend that
we are calling the attribute constructor from the place where the
attribute is used, which also means that the attribute location will
show up properly in backtraces and inside "called in" error information.
This requires us to store the attributes strict_types scope (as flags),
as well as the attribute line number. The attribute filename can be
recovered from the symbol it is used on. We might want to expose the
attribute line number via reflection as well.
See also https://externals.io/message/111915.
Closes GH-6201.
The motivation for this change is to prevent extensions from having to check executor globals for the current execute_data during function call init. A previous implementation of the observer API initialized the function call from runtime cache initialization before execute_data was allocated which is why zend_function was passed in.
But now that the observer API is implemented via opcode specialization, it makes sense to pass in the execute_data. This also keeps the API a bit more consistent for existing extensions that already hook zend_execute_ex.
Closes GH-6209
Noticed this while working on attributes strict_types handling.
We sometimes insert dummy frames internally, but I don't think
these should show up in debug_backtrace output unless they're
needed, either to display an include call or to preserve file/line
information that would otherwise get lost.
Closes GH-6195.
We missed the change to make this an Error exception in PHP 8,
but at least elevate it to a warning, to avoid a notice -> exception
jump at a later time.
Checking the linker compatibility with extranous `ImageLoad()` calls is
possible, but unnecessary, since the modules are either already loaded
or loaded shortly afterwards, so that we can get the required
information directly from the module handles. And actually, doing
`ImageLoad()` as well as `LoadLibrary()` leaves a tiny room for a race
condition, because both functions will lookup the module in the search
path, so there is no *guarantee* that both are dealing with the same
module. Dropping the `ImageLoad()` calls also has the advantage to no
longer face the issue reported in bug #79557. A very minor additional
advantage is that we no longer have to link against Imagehlp.dll.
Furthermore, there is no need to check for CRT compatibility multiple
times, so we can simplify the signature of `php_win32_crt_compatible`,
and at the same time clean up main.c a bit.
These changes require to change the signature of the exported
`php_win32_image_compatible` and `php_win32_crt_compatible` functions,
which now expect a `HMODULE` and nothing, respectively, instead of the
module name.
Due to improvements to early binding, the opcode based check is
no longer accurate. Reuse the syntactic check we're already using
for declares instead.
Closure::bind() and Closure::bindTo() are currently reported as aliases in stubs because they have a single implementation. They are not aliases in fact though, they just use zend_parse_method_parameters() cleverly.
Thus, let's separate their implementation so that we don't have to alias Closure::bindTo() anymore. This will also have the advantage that the two ZPP implementations become more clear.
Closes GH-6169
The `zend_system_id` is a (true global) system ID that fingerprints a process state. When extensions add engine hooks during MINIT/startup, entropy is added the system ID for each hook. This allows extensions to identify that changes have been made to the engine since the last PHP process restart.
Closes GH-5871
This is an annoying edge case that regularly gets broken. As we're
not aware of significant users of this API, and there are other
ways to hook this, remove support for EXT_NOP.
The "member" string here does not necessarily have a pre-calculated
hash value. In particular this is not the case if the class has no
properties.
Fixes oss-fuzz #25546.
This addresses two issues:
* ~ throws for a number of types, and we should not compile-time
evaluate in that case. Add a check similar to what we do for
binary ops.
* Unary +/- may produce a different error message due to
canonicalization of the constant operand to the RHS. To avoid
this, put the constant operand on the RHS right away.
Fixes oss-fuzz #25649.
* The array "subject" of a function gets called $array.
* Further parameters should be self-descriptive if used
as a named parameter, and a full word, not an abbreviation.
* If there is a "bunch more arrays" variadic, it gets
called $arrays (because that's what was already there).
* A few functions have a variadic "a bunch more arrays,
and then a callable", and were already called $rest.
I left those as is and died a little inside.
* Any callable provided to an array function that acts
on the array is called $callback. (Nearly all were already,
I just fixed the one or two outliers.)
* array_multisort() is beyond help so I ran screaming.
These tests segfault with tracing JIT on Windows as of a recent
commit[1]. A link to a backtrace is available in a comment[2]. For
now, we mark these tests as XFAIL, to keep AppVeyor CI helpful.
[1] <7b0a053169>
[2] <7b0a053169 (commitcomment-42267637)>
If an argument error refers to a variadic argument, we normally
do not print the name of the variadic (as it is not referring to
an individual argument, but to the collection of all of them).
However, this was not the case for the userland argument type
error message, which did it's own formatting.
Closes GH-6101.
See https://github.com/nikic/php-ast/pull/181
> Hm, I'm thinking it would make more sense to change the structure in php-src.
> All the function types have consistent AST structure, but there's no reason at
> all why classes should be consistent with functions.
It's unusual to have an unused child node between other child nodes that are
used (for name, extends, implements, and attributes of AST_CLASS)
> That gap is a leftover from a previous refactoring. An earlier version of
> attributes extended `zend_ast_decl` with a new member called `attributes` and
> therefore did not need to handle functions and classes in different ways.
Closes GH-6088
Both of these functions are well-defined when used with a single
array argument -- rejecting this case was an artificial limitation.
This is not useful when called with explicit arguments, but removes
edge-cases when used with argument unpacking:
// OK even if $excludes is empty.
array_diff($array, ...$excludes);
// OK even if $arrays contains a single array only.
array_intersect(...$arrays);
This matches the behavior of functions like array_merge() and
array_push(), which also allow calls with no array or a single
array respectively.
Closes GH-6097.
Currently we treat paths with null bytes as a TypeError, which is
incorrect, and rather inconsistent, as we treat empty paths as
ValueError. We do this because the error is generated by zpp and
it's easier to always throw TypeError there.
This changes the zpp implementation to throw a TypeError only if
the type is actually wrong and throw ValueError for null bytes.
The error message is also split accordingly, to be more precise.
Closes GH-6094.
Otherwise the assignment will have the same number as the default arm
which will 1. mis-trigger a breakpoint and 2. mark the line as covered
even when it isn't.
Closes GH-6083
When using zpp 'f' or Z_PARAM_FUNC, if the fcc points to a call
trampoline release it immediately and force zend_call_function
to refetch it. This may require additional callability checks
if __call is used, but avoids the need to carefully free fcc
values in all internal functions -- in some cases this is not
simple, as a type error might be triggered by a later argument
in the same zpp call.
This fixes oss-fuzz #25390.
Closes GH-6073.
For php-ast interning the file name is an effective memory leak,
see php-ast#134.
I don't think there's any reason to do this. At some point this
was needed due to bugs in the interned string mechanism that
caused issues if the string was later interned, e.g. through a
__FILE__ reference. These issues have since been resolved.
In conjunction with the filenames_table removal in c4016ecd44
this means that filenames now need to be refcounted like normal
strings. In particular the filename reference in op_arrays and CEs
are refcounted.
Applying the obvious fix ... however, I think we may need to
rething how we handle trampoline fcc for "f" zpp. It might make
sense to use fcc->function_handler == NULL for that case and
force it to be fetched in zend_call_function instead (it will
be reset to that after the call anyway). Otherwise we will keep
chasing these leaks, as it's the only instance where it's
necessary to free a zpp result.
Do not decrement the refcount before allocating the new string,
as the allocation operation may bail out and cause a use-after-free
lateron. We can only decrement the refcount once the allocation
has succeeded.
Fixes oss-fuzz #25384.
Based on:
"Fast CRC Computation for Generic Polynomials Using PCLMULQDQ Instruction"
V. Gopal, E. Ozturk, et al., 2009, http://intel.ly/2ySEwL0
Signed-off-by: Frank Du <frank.du@intel.com>
Closes GH-6018
Use the general zend_generator_throw_exception() helper for this.
Otherwise we don't handle the off-by-one opline correctly (should
we maybe just stop doing that?)
This is a followup to ad750c3bb6,
which fixed a different yield from exception handling problem that
happened to show up in the same test case from oss-fuzz #25321.
Now both issues should be fixed.
Shift the responsibility for emitting MAKE_REF to the list assignment
code, to make sure that LIST_W and MAKE_REF are directly adjacent,
and there are no opcodes in between that could modify the LIST_W
result.
Additionally, adjust the zend_wrong_string_offset() code to not
perform a loop over opcodes and assert that the next opcode is
a relevant one. The VM write-safety model requires this.
This is a followup to a07c1f56aa
and the full fix for oss-fuzz #25352.
There is a deeper underlying issue here, in that the opcodes violate
VM write-fetch safety, but let's fix the infinite loop first.
This fixes oss-fuzz #25352.
This can happen if zend_call_function inserted a dummy frame,
and we already switched to the dummy frame in leave_helper,
and an exception is thrown during CV destruction.
Fixes oss-fuzz #25343.
FE_RESET sets the result to UNDEF in this case. We should infer
some type here, because no type generally implies unreachable
code. In this case SCCP ended up replacing the FE_RESET result
with null, including in FE_FREE.
We can't just remove the uses, we need to replace uses.
The test case only fails on master with SSA integrity violations,
but I believe the root issue also existed previously.
If we're accessing an uninitialized typed property and __get is
defined, don't perform a read_property callback, as __get is
supposed to have no effect on uninitialized typed properties.
Usually it doesn't, but by-reference assignments cannot be
performed through read_property.
I'm deleting the test for bug #80039 again, as it doesn't really
make sense anymore with this fix.
To perform fast shutdown without full table cleanup we need all
internal functions to be in one continuous chunk. This was
violated when functions were deleted via disable_functions.
This drops the zend_disable_function() API in favor of
zend_disable_functions(), which disables the given list of
functions and performs the necessary rehash afterwards.
Also drop PG(disabled_functions), which is no longer used.
Voidification of Zend API which always succeeded
Use bool argument types instead of int for boolean arguments
Use bool return type for functions which return true/false (1/0)
Use zend_result return type for functions which return SUCCESS/FAILURE as they don't follow normal boolean semantics
Closes GH-6002
A very basic limit (for single allocations) was already enforced.
This extends it to count the total memory allocations.
This is useful to avoid out of memory conditions while fuzzing.
Set HAS_TYPE_HINTS flag if the variadic parameter is types as well,
and make sure it has a distinct name. This was previously missed,
because the variadic parameter is not part of num_args.
Refactor the zend_is_callable implementation to check callability
at a particular frame (this is an implementation detail for now,
but could be exposed in the API if useful). Pick the first parent
user frame as the one to check.
This is targeting 8.0.
`$arg` seems like a poor choice of a name,
especially if the function were to have arguments added.
In many cases, the php.net documentation already has $array for these functions.
E.g. https://www.php.net/manual/en/function.array-intersect.php
I'd assume that since named arguments was added to 8.0 near the feature freeze,
PHP's maintainers had planned to make the names consistent
and gradually use the same name for docs and implementation.
The primary issue was already resolved in 7c3e487289,
but the particular example used in this bug report ran into an
additional issue on PHP 8, because I forgot to drop a number of
zend_bailout calls when switch require failure to throw.
Make sure we don't execute further scripts if one of them encountered
an exit exception.
Also make sure that we free file handles that end up unused due to
an early abort in php_execute_scripts(), which turned up as an
issue in the added test case.
Finally, make use of EG(exit_status) in the places where we
zend_eval_string_ex, instead of unconditionally assigning exit
code 254. If an error occurs, the error handler will already set
exit status 255.
Unconditionally strip shebang lines when using the CLI SAPI,
independently of whether they occur in the primary or non-primary
script. It's unlikely that someone intentionally wants to print
that shebang line when including a script, and this regularly
causes issues when scripts are used in multiple contexts, e.g.
for direct invocation and as a phar bootstrap.
Move the FREE_OP for op_data out of the zend_binary_assign_op_dim_slow()
slow path, so it can be used by the other error path as well. This
makes ASSIGN_DIM_OP structurally more similar to ASSIGN_DIM.
