When redeclaring an overridden static property with a trait we're removing the
property from the class. However, because the property itself does not belong to
the class we must not free its associated data.
This issue is exposed by 9a250cc9d6 in PHP 8.3+ because duplicate static
properties in traits are no longer skipped, but redeclared.
Fixes GH-12468
This currently uses ZVAL_COPY_OR_DUP, which copies the value and handles
refcounting. However, internal classes cannot have refcounted default
properties because of constraints imposed by
zend_declare_typed_property(). So copying the value is sufficient.
While this doesn't really improve the performance for our benchmarks, it
improves performance for cases where a lot of temporary internal objects
are instantiated. For example, when instantiating DOM classes: DOM
objects are transient, so lots of temporary objects are created.
Commit d86314939c added an additional parameter to Z_PARAM_FUNC_EX.
To maintain compatibility with third-party extensions, we keep
Z_PARAM_FUNC_EX as it used to be, and add Z_PARAM_FUNC_EX2 instead.
The constant name is usually interend. Without opcache, compilation always
interns strings. Without opcache, compilation does not intern (new) strings, but
persisting of script does. If a script is not stored in shm the constant name
will not be interned.
The building of enum backing stores was missing a addref for the constant name,
leading to a double-free when releasing constants and backing stores of enums.
Fixes GH-12366
Closes GH-12405
* PHP-8.3:
Fix compile error with -Werror=incompatible-function-pointer-types and old libxml2
Fix GH-10008: Narrowing occurred during type inference of ZEND_ADD_ARRAY_ELEMENT
Fix type error on XSLTProcessor::transformToDoc return value with SimpleXML
* PHP-8.2:
Fix GH-10008: Narrowing occurred during type inference of ZEND_ADD_ARRAY_ELEMENT
Fix type error on XSLTProcessor::transformToDoc return value with SimpleXML
* PHP-8.1:
Fix GH-10008: Narrowing occurred during type inference of ZEND_ADD_ARRAY_ELEMENT
Fix type error on XSLTProcessor::transformToDoc return value with SimpleXML
This test triggers narrowing for two ops: first ZEND_ADD_ARRAY_ELEMENT,
and then ZEND_ASSIGN.
The type inference happens in the following order:
1) The ZEND_ADD_ARRAY_ELEMENT infers type 0x40e04080 (packed flag is set),
arr_type=0 at this point because it hasn't been set by ZEND_INIT_ARRAY yet.
2) The ZEND_INIT_ARRAY infers type 0x40804080
3) The ZEND_ADD_ARRAY_ELEMENT infers type 0x40e04080, arr_type=0x40804080,
which does not have the packed flag set while the existing result of
ZEND_ADD_ARRAY_ELEMENT has the packed flag set.
This seems to occur because of the phi node introduced by the while
loop. If I remove the loop the problem goes away.
As Arnaud noted, this seems to be caused by a too wide type inference
for arr_type==0. We should keep the invariant that if x>=y then
key_type(x) >= key_type(y).
If we write the possible results down in a table we get:
```
arr_type resulting key type
--------------- --------------------------------------------------------------------------
HASH_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH
PACKED_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG)
HASH || PACKED -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG)
0 -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG)
```
As we can see, `HASH_ONLY > 0` but
`MAY_BE_ARRAY_NUMERIC_HASH < MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED`,
which violates the invariant.
Instead if we modify the zero case to have MAY_BE_ARRAY_NUMERIC_HASH instead,
we get the following table which satisfies the invariant.
```
arr_type resulting key type
--------------- --------------------------------------------------------------------------
HASH_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH
PACKED_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG)
HASH || PACKED -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG)
0 -> MAY_BE_ARRAY_NUMERIC_HASH
```
Broke in 1ffbb73.
Closes GH-10294.
* Make sure core module has number 0
Some places, possibly also outside PHP, assume the core extension has
module number 0. After 8a812c3fda this wasn't the case anymore as
reported in [1]. Fix it by changing how the next module ID is computed.
[1] https://github.com/php/php-src/pull/12246#issuecomment-1731432377
* Add assertion
* Add test
When we try to load an extension multiple times, we still overwrite the
type, module number, and handle. If the module number is used to
indicate module boundaries (e.g. in reflection and in dom, see e.g.
dom_objects_set_class_ex), then all sorts of errors can happen.
In the case of ext/dom, OP's error happens because the following
happens:
- The property handler is set up incorrectly in
dom_objects_set_class_ex() because the wrong module number is
specified. The class highest in the hierarchy is DOMNode, so the
property handler is incorrectly set to that of DOMNode instead of
DOMDocument.
- The documentElement property doesn't exist on DOMNode, it only exists
on DOMDocument, so it tries to read using zend_std_read_property().
As there is no user property called documentElement, that read
operation returns an undef value.
However, the type is still checked, resulting in the strange exception.
Solve this by changing the API such that the data is only overwritten if
it's owned data.
Closes GH-12246.
ZEND_UNREACHABLE() currently expands to the following in GCC:
if (__builtin_expect(!(0), 0)) __builtin_unreachable();
Even though the branch is always executed, GCC does not recognize the outer
branch as unreachable. Removing the if fixes some unexpected warnings.
Closes GH-12248
When declaring the same static property with a doc block in a class and in a trait,
the doc block of the property in the class is leaked. While at it, possibly fix doc
comment for internal classes.
Close GH-12238
These tests intermittently crash asan. It might be due to some function invoking
dl(), which is known to crash lsan. It might also be something else, the version
of asan shipped with ubuntu 22.04 is flaky.
Despite being OpenBSD's predecessor, the approach is in fact
a lot closer to Linux, at least in principle. We purposely
avoid reading /proc/N/maps to be more future-proof.
Close GH-11637
Reorder when we assign the property value to NULL which is identical to
a3a3964497
Just for the declared property case instead of dynamic.
Closes GH-12114
Addref to relevant fields before allocating any memory. Also only set/remove the
ZEND_ACC_HEAP_RT_CACHE flag after allocating memory.
Fixes GH-12073
Closes GH-12074
With the fix in https://github.com/php/php-src/pull/12114, the behaviour
would change for non-dynamic properties. Align the behaviour for dynamic
properties to be the same.
Closes GH-12117.
Because the error handler is invoked after the property is updated,
the error handler has the opportunity to remove it before the property
is returned.
Switching the order around fixes this issue. The comments mention that
the current ordering prevents overwriting the EG(std_property_info)
field in the error handler. EG(std_property_info) no longer exists as it
was removed in 7471c217. Back then a global was used to store the
returned property info, but as this is no longer the case there is no
longer a need to protect against overwriting a global.
Closes GH-12062.
When executing a foreach ($ht as &$ref), foreach calls zend_hash_iterator_pos_ex() on every iteration. If the HashTable contained in the $ht variable is not the tracked HashTable, it will reset the position to the internal array pointer of the array currently in $ht.
This behaviour is generally fine, but undesirable for copy-on-write copies of the iterated HashTable. This may trivially occur when the iterated over HashTable is assigned to some variable, then the iterated over variable modified, leading to array separation, changing the HashTable pointer in the variable. Thus foreach happily restarting iteration.
This behaviour (despite existing since PHP 7.0) is considered a bug, if not only for the behaviour being unexpected to the user, also copy-on-write should not have trivially observable side-effects by mere assignment.
The bugfix consists of duplicating HashTableIterators whenever zend_array_dup() is called (the primitive used on array separation).
When a further access to the HashPosition through the HashTableIterators API happens and the HashTable does not match the tracked one, all the duplicates (which are tracked by single linked list) are searched for the wanted HashTable. If found, the HashTableIterator is replaced by the found copy and all other copies are removed.
This ensures that we always end up tracking the correct HashTable.
Fixes GH-11244.
Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
This PR introduces a new way of recursion protection in JSON, var_dump
and friends. It fixes issue in master for __debugInfo and also improves
perf for jsonSerializable in some cases. More info can be found in
GH-10020.
Closes GH-11812
* Zend: Make zend_strnlen available for use outside zend_compile
* exif: remove local php_strnlen, use zend_strnlen instead
* main: remove local strnlen, use zend_strnlen instead
* phar: remove local strnlen, use zend_strnlen
Evaluating constants at comptime can result in arrays that contain objects. This
is problematic for printing the default value of constant ASTs containing
objects, because we don't actually know what the constructor arguments were.
Avoid this by not propagating array constants.
Fixes GH-11937
Closes GH-11947
- GH-11958: DNF types in trait properties do not get bound properly
- GH-11883: Memory leak in zend_type_release() for non-arena allocated DNF types
- Internal trait bound to userland class would not be arena allocated
- Property DNF types were not properly deep copied during lazy loading
Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>
Co-authored-by: ju1ius <jules.bernable@gmail.com>
We may OOM during object initialization. In this case, free_obj needs to guard
against NULL values. There may be more cases where this is an issue, these were
the ones I was able to discover via script.
Fixes GH-11734
* opcache: use zend_ast_size helper in zend_persist_ast
* opcache: use zend_ast_size helper in zend_persist_ast_calc
* Zend: fix zend_ast_size definition
It is better not to use sizeof(struct_with_flexible_array)
and instead rely on offsetof(type, member) like most
other similar wrappers do.
Cirrus will no longer offer unlimited free builds starting next month. We don't
have an alternative for FreeBSD and ARM, so move what we can for now.
Closes GH-11898
opnum_start denotes the start of the ZEND_FREE block of skipped consuming
opcodes. Storing the number before zend_compile_expr(..., label_ast) makes it
seem like it denotes the start of the label block. However, label_ast must only
be a zval string AST, and as such never results in an actual opcode.
We require valid code for compilation to succeed, but these paths should always
be guarded by OPx_TYPE checks and never execute. Add an assertion to verify.
This does still deviate from USE_ZEND_ALLOC=0 in that we're not rounding up the
size of the allocation to fixed sizes. Doing so would suppress some
out-of-bounds errors checked by ASAN. Rounding up the size in
_zend_mm_block_size would not be good either as it would break code like
memset(ptr, 0 _zend_mm_block_size(ptr)).
The typo in HAVE_PTHREAD_ATTR_GET_STACK (might be due to pthread_attr_get_np being different from Linux's pthread_getattr_np) led to this code path never get called on FreeBSD.
We don't want to invoke calls twice, even if they are considered "variables",
i.e. might be writable if returning a reference. Function calls behave the same
in all BP contexts so they don't need to be invoked twice. The singular
exception to this is nullsafe coalesce in isset/empty, because it needs to
return false/true respectively when short-circuited. However, since nullsafe
calls are not allwed in write context we may ignore this problem.
Closes GH-11592
* Add behavioural tests for incdec operators
* Add support to ++/-- for objects castable to _IS_NUMBER
* Add str_increment() function
* Add str_decrement() function
RFC: https://wiki.php.net/rfc/saner-inc-dec-operators
Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>
Co-authored-by: Arnaud Le Blanc <arnaud.lb@gmail.com>
We transform the arrow function by nesting the expression into a return
statement. If we compile the arrow function twice this would be done twice,
leading to a compile assertion.
Fix oss-fuzz #60411
Closes GH-11632
Normally, PHP evaluates all expressions in offsets (property or array), as well
as the right hand side of assignments before actually fetching the offsets. This
is well explained in this blog post.
https://www.npopov.com/2017/04/14/PHP-7-Virtual-machine.html#writes-and-memory-safety
For ??= we have a bit of a problem in that the rhs must only be evaluated if the
lhs is null or undefined. Thus, we have to first compile the lhs with BP_VAR_IS,
conditionally run the rhs and then re-fetch the lhs with BP_VAR_W to to make
sure the offsets are valid if they have been invalidated.
However, we don't want to just re-evaluate the entire lhs because it may contain
side-effects, as in $array[$x++] ??= 42;. In this case, we don't want to
re-evaluate $x++ because it would result in writing to a different offset than
was previously tested. The same goes for function calls, like
$array[foo()] ??= 42;, where the second call to foo() might result in a
different value. PHP behaves correctly in these cases. This is implemented by
memoizing sub-expressions in the lhs of ??= and reusing them when compiling the
lhs for the second time. This is done for any expression that isn't a variable,
i.e. anything that can (potentially) be written to.
Unfortunately, this also means that function calls are considered writable due
to their return-by-reference semantics, and will thus not be memoized. The
expression foo()['bar'] ??= 42; will invoke foo() twice. Even worse,
foo(bar()) ??= 42; will call both foo() and bar() twice, but
foo(bar() + 1) ??= 42; will only call foo() twice. This is likely not by design,
and was just overlooked in the implementation. The RFC does not specify how
function calls in the lhs of the coalesce assignment behaves. This should
probably be improved in the future.
Now, the problem this commit actually fixes is that ??= may memoize expressions
inside assert() function calls that may not actually execute. This is not only
an issue when using the VAR in the second expression (which would usually also
be skipped) but also when freeing the VAR. For this reason, it is not safe to
memoize assert() sub-expressions.
There are two possible solutions:
1. Don't memoize any sub-expressions of assert(), meaning they will execute
twice.
2. Throw a compile error.
Option 2 is not quite simple, because we can't disallow all memoization inside
assert(), as that would break assertions like assert($array[foo()] ??= 'bar');.
Code like this is highly unlikely (and dubious) but possible. In this case, we
would need to make sure that a memoized value could not be used across the
assert boundary it was created in. The complexity for this is not worthwhile. So
we opt for option 1 and disable memoization immediately inside assert().
Fixes GH-11580
Closes GH-11581
