I had a case where I was intentionally storing a `NULL` pointer within a
HashTable to mark an entry as “blocked”, without paying for the overhead of
checking the entry type when reading the pointer, resulting in semantics that
are similar to using `isset()` instead of `array_key_exists()` in userland.
This worked fine in unoptimized test builds, but due to the `ZEND_ASSUME()` in
the `zend_hash_find_ptr` functions, the optimized release builds turned the
logic of:
my_pointer = zend_hash_find_ptr(ht, key);
if (my_pointer == NULL) {
return;
}
*my_pointer;
into
zv = zend_hash_find(ht, key);
if (zv) {
*Z_PTR_P(zv);
} else {
return;
}
thus introducing a hard-to-debug and compiler-dependent crash when the entry
exists, but the stored pointer is `NULL`.
Change the `ZEND_ASSUME()` in the setters to `ZEND_ASSERT()`. This would have
made my mistake immediately obvious in debug builds when storing the pointer.
The getters still use `ZEND_ASSUME()` under the assumption that they are called
much more often, reducing the impact on debug builds: Assuming the developer
uses the `_ptr` variants for both reading and writing the entries, the mistake
will be reliably caught during writing, making the assert during reading
unnecessary.
For release builds the `ZEND_ASSERT()` will be equivalent to `ZEND_ASSUME()`,
avoiding any performance impact for those.
Previously this returned `int`. Many functions actually take advantage
of the fact this returns exactly 0 or 1. For instance,
`main/streams/xp_socket.c` does:
sockopts |= STREAM_SOCKOP_IPV6_V6ONLY_ENABLED * zend_is_true(tmpzval);
And `Zend/zend_compile.c` does:
child = &ast->child[2 - zend_is_true(zend_ast_get_zval(ast->child[0]))];
I changed a few places trivially from `int` to `bool`, but there are
still many places such as the object handlers which return `int` that
should eventually be `bool`.
* Mark many functions as static
Multiple functions are missing the static qualifier.
* remove unused struct sigactions
struct sigaction act, old_term, old_quit, old_int;
all unused.
* optimizer: minXOR and maxXOR are unused
* 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
This is annoying for multiple reasons:
1. LSPs can show many errors for these files.
2. LSP can stop parsing these files completely if there are too many errors,
resulting in spotty LSP features.
This reduces -Wunused-variable and -Wunused-but-set-variable warnings
generated in the Autoconf's config.log to comply with possible stricter
default compiler configuration on the system to avoid having false
results.
`zend_call_method_with_1_params()` causes needless overhead, due to the
capability of looking up the function by name. It’s also very rarely used (only
in ext/spl).
`zend_call_known_function()` is the standard methodology to call a known
function and going through `Z_OBJCE_P()` avoids the repeated hardcoding of the
CE name.
* Add `zend_get_attribute_object()`
This makes the implementation for `ReflectionAttribute::newInstance()`
reusable.
* Add test for the stack trace behavior of ReflectionAttribute::newInstance()
This test ensures that the `filename` parameter for the fake stack frame is
functional. Without it, the stack trace would show `[internal function]` for
frame `#0`.
* Fix return type of `call_attribute_constructor`
* Fix prototype for trait methods
Fixes GH-14009
* Clenup do_inheritance_check_on_method()
Remove wierd checks and define the behavior by explicit set of flags
* Fix naming and indentation
---------
Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>
