When functions' or class methods' availability is based on some preprocessor
condition, the generated arginfo header files wrap the declarations in the
preprocessor `#if` conditional blocks, one per declaration, even if they are in
the same conditional block based on comments in the stub file. Instead of
having multiple conditional blocks one after the other with the same condition,
combine them into a single conditional block.
When a class (or enum) has no methods, rather than using an array that only
contains `ZEND_FE_END`, use `NULL` for the functions. The implementation of
class registration for internal classes, `do_register_internal_class()` in
zend_API.c, already skips classes where the functions are `NULL`. By removing
these unneeded arrays, we can reduce the size of the header files, while also
removing an unneeded call to zend_register_functions() for each internal class
with no extra methods.
Currently, internal classes are registered with the following code:
INIT_CLASS_ENTRY(ce, "InternalClass", class_InternalClass_methods);
class_entry = zend_register_internal_class_ex(&ce, NULL);
class_entry->ce_flags |= ...;
This has worked well so far, except if InternalClass is readonly. It is because some inheritance checks are run by zend_register_internal_class_ex before ZEND_ACC_READONLY_CLASS is added to ce_flags.
The issue is fixed by adding a zend_register_internal_class_with_flags() zend API function that stubs can use from now on. This function makes sure to add the flags before running any checks. Since the new API is not available in lower PHP versions, gen_stub.php has to keep support for the existing API for PHP 8.3 and below.
* Allow optimizer to depend on preloaded symbols
It is safe for the optimizer to rely on preloaded symbols. This can occur when
compiling non-preloaded files, referencing preloaded ones.
* Disable inline pass for observer test
* Move duplicated code into functions
* Add comment to specific optimization value
* Optimizer should only rely on preloaded symbols in the symbol table
* Fix skipif for windows
The create_obj handler of InternalIterator is overwritten, but not the
clone_obj handler. This is not allowed.
In PHP 8.2 this didn't cause a segfault because the standard object
handler was used for the clone instead of the internal handler.
So then it allocates and frees the object using the standard object handlers.
In 8.3 however, the object is created using the standard object handler and
freed using the custom handler, resulting in the buffer overflow.
Even though bisect points to 1e1ea4f this only reveals the bug.
Closes GH-14882.
is_zend_ptr() expected zend_mm_heap.huge_list to be circular, but it's in fact NULL-terminated. It could crash when at least one huge block exists and the ptr did not belong to any block.
This is necessary because `zend_get_attribute_object()` will use the persistent
string with the parameter name as the index for a newly created non-persistent
HashTable, which is not legal.
As parameter names are expected to be short-ish, reasonably common terms and
need to sit around in memory anyways, we might as well make them an interned
string, circumstepping the issue without needing to duplicate the parameter
name into a non-persistent string.
Inline the lookup whether a function is observed at all.
This strategy is also used for FRAMELESS calls. If the frameless call is observed, we instead allocate a call frame and push the arguments, to call the the function afterwards.
Doing so is still a performance benefit as opposed to executing individual INIT_FCALL+SEND_VAL ops. Thus, even if the frameless call turns out to be observed, the call overhead is slightly lower than before.
If the internal function is not observed at all, the unavoidable overhead is fetching the FLF zend_function pointer and the run-time cache needs to be inspected.
As part of this work, it turned out to be most viable to put the result operand on the ZEND_OP_DATA instead of ZEND_FRAMELESS_ICALL_3, allowing seamless interoperability with the DO_ICALL opcode.
This is a bit unusual in comparison to all other ZEND_OP_DATA usages, but seems to not pose problems overall.
There is also a small issue resolved: trampolines would always use the ZEND_CALL_TRAMPOLINE_SPEC_OBSERVER function due to zend_observer_fcall_op_array_extension being set to -1 too late.
Although the issue was demonstrated using Curl, the issue is purely in
the streams layer of PHP.
Full analysis is written in GH-11078 [1], but here is the brief version:
Here's what actually happens:
1) We're creating a FILE handle from a stream using the casting mechanism.
This will create a cookie-based FILE handle using funopen.
2) We're reading stream data using fread from the userspace stream. This will
temporarily set a buffer into a field _bf.base [2]. This buffer is now equal
to the upload buffer that Curl allocated and note that that buffer is owned
by Curl.
3) The fatal error occurs and we bail out from the fread function, notice how
the reset code is never executed and so the buffer will still point to
Curl's upload buffer instead of FILE's own buffer [3].
4) The resources are destroyed, this includes our opened stream and because the
FILE handle is cached, it gets destroyed as well.
In fact, the stream code calls through fclose on purpose in this case.
5) The fclose code frees the _bs.base buffer [4].
However, this is not the buffer that FILE owns but the one that Curl owns
because it isn't reset properly due to the bailout!
6) The objects are getting destroyed, and so the curl free logic is invoked.
When Curl tries to gracefully clean up, it tries to free the buffer.
But that buffer is actually already freed mistakingly by the C library!
This also explains why we can't reproduce it on Linux: this bizarre buffer
swapping only happens on macOS and BSD, not on Linux.
To solve this, we switch to an unbuffered mode for cookie-based FILEs.
This avoids any stateful problems related to buffers especially when the
bailout mechanism triggers. As streams have their own buffering
mechanism, I don't expect this to impact performance.
[1] https://github.com/php/php-src/issues/11078#issuecomment-2155616843
[2] 5e566be7a7/stdio/FreeBSD/fread.c (L102-L103)
[3] 5e566be7a7/stdio/FreeBSD/fread.c (L117)
[4] 5e566be7a7/stdio/FreeBSD/fclose.c (L66-L67)
Closes GH-14524.
* Make `ReflectionGenerator::getFunction()` legal after generator termination
* Expose the generator function name via `Generator::__debugInfo()`
* Allow creating `ReflectionGenerator` after termination
* Reorder `struct _zend_generator` to avoid a hole
* Adjust `ext/reflection/tests/028.phpt`
This is legal now.
* Fix Generator Closure collection
* Add test to verify the Closure dies with the generator
* NEWS / UPGRADING
Some modules may reset _fmode, which causes mangling of line endings.
Always be explicit like we do in other places where the native open call
is used.
Closes GH-14218.
Fixes GH-13970
Closes GH-14105
We cannot validate at compile-time for multiple reasons:
* Evaluating the argument naively with zend_get_attribute_value can lead to code
execution at compile time through the new expression, leading to possible
reentrance of the compiler.
* Even if the evaluation was possible, it would need to be restricted to the
current file, because constant values coming from other files can change
without affecting the current compilation unit. For this reason, validation
would need to be repeated at runtime anyway.
* Enums cannot be instantiated at compile-time (the actual bug report). This
could be allowed here, because the value is immediately destroyed. But given
the other issues, this won't be needed.
Instead, we just move it to runtime entirely. It's only needed for
ReflectionAttribute::newInstance(), which is not particularly a hot path. The
checks are also simple.
* Include the source location in Closure names
This change makes stack traces involving Closures, especially multiple
different Closures, much more useful, because it's more easily visible *which*
closure was called for a given stack frame.
The implementation is similar to that of anonymous classes which already
include the file name and line number within their generated classname.
* Update scripts/dev/bless_tests.php for closure naming
* Adjust existing tests for closure naming
* Adjust tests for closure naming that were not caught locally
* Drop the namespace from closure names
This is redundant with the included filename.
* Include filename and line number as separate keys in Closure debug info
* Fix test
* Fix test
* Include the surrounding class and function name in closure names
* Fix test
* Relax test expecations
* Fix tests after merge
* NEWS / UPGRADING
The usage of the current API within an observer handler leads to bugs like https://bugs.xdebug.org/view.php?id=2232.
Given two observer handlers, next to each other. The first one is executed. It removes itself. The second observer handler gets moved to the place of the first. The first one returns. The handler in the second slot is fetched, but is now NULL, because the it's now in the slot of the first observer; i.e. the second handler is skipped and the begin/end symmetry guarantee is violated.
Providing the next handler to the caller is a zero-cost way to avoid any impact in the paths of zend_observe_fcall_begin/end.
Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
- Declared compatibility expectations of stub files are now enforced by a ZEND_STATIC_ASSERT call at the top of arginfo files
- Property registration for PHP 7 is fixed: function zend_declare_property_ex() is used again instead of zend_declare_typed_property(). This has been a regression since I added support for exposing doc comments.
- As a defensive measure, deep cloning is performed before newer features (type declarations, attributes etc.) are discarded before generating legacy arginfo files. Until now, some of the objects were forgotten to be taken care of. These omissions may have resulted in some weird bugs in theory (but probably they didn't have much impact in practice).
- PHP version related conditions inside *non-legacy arginfo files* used to possibly check for the 70000 version iD until now if compatibility with PHP 7.0 was declared in a stub. This was not 100% correct, since non-legacy arginfo files are only for PHP 8.0+. Now, I made sure that at least PHP version ID 80000 is used in the preprocessor conditions. The solution was a bit tricky though...
