An attempt to fix all the failures to clobber data by user error hand…

[dstogov]

We detect if user error handler deallocates op1 or op2 oprand of the currently executed VM instruction and throw fatal error instead of attempt to repair.

All the "repair" code is commented to prove that all the case are caught by this patch.

The behavior of Zend/tests/str_offset_008.phpt is different with tracing JIT, because reference-counting operations may be eliminated.

[dstogov]

@nikic please take a look. I think this idea may be improved.

[dstogov]

@nikic I've refactored this, but probably, we may improve this even better.

[TysonAndre]

Only partially familiar with the indirect handling code - this says it includes opcodes that entail setting array fields by reference.

Is destroying the only concern? Is a erealloc that would cause Z_INDIRECT_P(zv) (found within arData/arPacked) to point to a different value possible (freed memory region, or freed then allocated for something else), e.g. if the user error handler adds elements to the array/object and the size grows from 6 to 12 and the capacity needs to grow from 8 to 16 (or if it converts packed to hash)?

zend_fetch_dimension_address returns an IS_INDIRECT pointer to a zval within the arData/arPacked (for opcodes writing to a field of an array?)

				if (EXPECTED(GC_DELREF(op1_fake_ref) == 1)) {
					/* This is a life fake reference. Convert it back to regular zval. */
					zend_execute_data *execute_data = EG(current_execute_data);

					zval *zv = EX_VAR(opline->op1.var);
					ZEND_ASSERT(Z_TYPE_P(zv) == IS_INDIRECT);
					zv = Z_INDIRECT_P(zv);
					ZEND_ASSERT(Z_TYPE_P(zv) == IS_REFERENCE && Z_REF_P(zv) == op1_fake_ref);
					ZVAL_UNREF(zv);

The call to SEPARATE_ARRAY previously (with the related commented out uses of GC_ADDREF) seem like they may have guarded against that possibility, though I'm not familiar with the history of that file.

static zend_always_inline void zend_fetch_dimension_address(zval *result, zval *container, zval *dim, int dim_type, int type EXECUTE_DATA_DC)
{
	zval *retval;

	if (EXPECTED(Z_TYPE_P(container) == IS_ARRAY)) {
try_array:
		SEPARATE_ARRAY(container);

I see there's a GC_ADDREF getting added, but if I understood correctly, that's for the element of the array, not the array itself?

[dstogov]

IS_INDIRECT zval is used when we pass a pointer to zval embedded into the enclosing array or object. It's necessary only for "write" context. (When we write back to arrayor object element). The write operation on arrays and strings are possible only when they have refcount == 1. They might be "separated".

The new code in zend_error converts op1 into a fake reference if necessary and increments refcounter, so it cannot escape. But I afraid, you are right. The fact that we keep reference a live, dosn't necessary mean that the IS_INDIRECT zval holder is a live as well. It might be reallocated. It should be easy to rove this with a test case.