You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Replace ZEND_ASSUME() by ZEND_ASSERT() in zend_hash_*_ptr setters (#14466)
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.
0 commit comments