Zend: compile time assert on Bucket size - #23079
Conversation
TimWolla
left a comment
TimWolla
left a comment
There was a problem hiding this comment.
Should we just force the alignment with https://en.cppreference.com/c/language/_Alignas instead?
|
@Sjord please search for ZEND_SET_ALIGNED |
| } Bucket; | ||
| }) Bucket; | ||
|
|
||
| ZEND_STATIC_ASSERT(sizeof(Bucket) % 8 == 0, "Bucket size not compatible with storing flags in lower three bits"); |
There was a problem hiding this comment.
|
I have added |
|
An alternative would be to shift by 1 in Line 5589 in 360cff4 sizeof(Bucket) is not a multiple of 8. This way we can keep the size of Bucket as small as possible. Otherwise we are adding 4 bytes to every Bucket for a single use-case.
|
If this is undesirable, I suggest we keep only the static assert on bucket size for now, and look for a better solution if that assert breaks. |
In zend_compile.c, flags are stored in the lower bits of the Bucket address. If Bucket is aligned to 8 bytes, the lower three bits are always zero and this gives no problems. If the Bucket is not aligned, this results in non-obvious errors because the memory address and the flags overlap. This is difficult to debug when it happens, so add this assertion to make it more obvious what is wrong. The flags are ZEND_BIND_REF, ZEND_BIND_IMPLICIT, ZEND_BIND_EXPLICIT. Related to php#19079
1c5f9f5 to
2f30c08
Compare
|
Yes, this makes sense. We should add some context to the assertion so that we can find why it exists. We should either mention Line 5589 in 360cff4 I prefer the latter but I let you chose. |
Instead of close to Bucket. This way, it is clearer what to look for if the assertion fails.
|
I moved it to zend_compile_static_var_common. |
4 participants
In zend_compile.c, flags are stored in the lower bits of the Bucket address. If Bucket is aligned to 8 bytes, the lower three bits are always zero and this gives no problems. If the Bucket is not aligned, this results in non-obvious errors because the memory address and the flags overlap. This is difficult to debug when it happens, so add this assertion to make it more obvious what is wrong.
The flags are ZEND_BIND_REF, ZEND_BIND_IMPLICIT, ZEND_BIND_EXPLICIT.
Related to #19079