Detect misuse of ZEND_MAP_PTR_NEW()

[arnaud-lb]

ZEND_MAP_PTR_NEW() is only safe during startup and when the opcache shm is locked, however it's easy to forget about that and to misuse it. Similarly, ZEND_MAP_PTR_NEW_STATIC() can only be used during startup.

Here I add assertions to check that we are only using ZEND_MAP_PTR_NEW() and ZEND_MAP_PTR_NEW_STATIC() when it's safe to do so.

[dstogov]

The CG(in_compilation) looks suspicious. Is it necessary for PHP without opcache?

I think, it may be better to use CG(map_ptr_protected) initialized by false, then setting it to true in opcache post_starup and then updated by lock/unlock.

[arnaud-lb]

Without opcache ZEND_MAP_PTR_NEW() is always safe, but I think it's better to implement the same restrictions when opcache is not enabled, so that code that works without opcache also works with it.

The CG(in_compilation) looks suspicious

Indeed. I was convinced that compilation occurred while opcache was locked, so this was to handle the non-opcache case, but actually it's not.

I've simplified as you suggested, but I still deny usage of ZEND_MAP_PTR_NEW() after startup when opcache is not enabled.

[dstogov]

Without opcache, map_ptr buffer belongs to process/thread and it may allocate new slots at any time.

With opcache. map_ptr buffer is addressed through opcache SHM, so we shouldn't add new slots without opcache lock. I think, protection should be obtained and managed by opcache. In case, we manage it outside opcache (e.g. in zend_alloc_ce_cache() we just negate the protection. (I may be wrong)

[arnaud-lb]

Yes, zend_alloc_ce_cache() is what originally convinced me that the lock was held during compilation, since it calls ZEND_MAP_PTR_NEW_OFFSET(). But actually, the conditions in zend_alloc_ce_cache() ensure that we never call ZEND_MAP_PTR_NEW_OFFSET() after startup when opcache is enabled (a map ptr is only allocated when the class name is interned and not permanent, which only happens without opcache).

[dstogov]

I still think that the modification of CG(map_ptr_protected) in zend_alloc_ce_cache() is a bad decision.
The protection should have a single point of responsibility.

But actually, the conditions in zend_alloc_ce_cache() ensure that we never call ZEND_MAP_PTR_NEW_OFFSET() after startup when opcache is enabled (a map ptr is only allocated when the class name is interned and not permanent, which only happens without opcache).

This is too complex verbal condition and it may be easily violated by inaccurate future modification.
You introduce protection exactly against inaccurate future modifications, so it's better not to make exceptions without a big reason.