Fix GH-15657: Segmentation fault in ext/opcache/jit/ir/dynasm/dasm_x86.h

[nielsdos]

The crash happens because the zend_persist.c code tries to JIT the hook's op_array while the JIT buffer memory is still protected. This happens in zend_persist_property_info called via zend_persist_class_entry through the inheritance cache. You can check that this is true by surrounding the JIT call with zend_jit_unprotect() and zend_jit_protect().

We shouldn't JIT the property hook code when persisting property info for the inheritance cache.

This is a simple workaround by temporarily disabling the JIT so that the property hook code is not JITted when persisting the property info.

An alternative solution would be to move the JITting of the property hooks to a different place in zend_persist.c by doing an additional pass over the classes.

[nielsdos]

Actually, maybe this is not complete, because if I add the following code to the test at the bottom, it crashes (both with and without this patch):

for ($i=0;$i<2;$i++)
    echo (new A)->prop;

I don't have time anymore today to analyse that crash though. It still happens even if I remove the inheritance for interface I, so maybe it's a slightly different bug.

[dstogov]

This is probably right.

cc: @iluuu1994

[nielsdos]

Actually, maybe this is not complete, because if I add the following code to the test at the bottom, it crashes (both with and without this patch):

for ($i=0;$i<2;$i++)
    echo (new A)->prop;

I don't have time anymore today to analyse that crash though. It still happens even if I remove the inheritance for interface I, so maybe it's a slightly different bug.

I analysed this and this is a different bug related to a cache slot optimization.
As far as I understand, this happens for when the cache slot satisfies the ZEND_IS_PROPERTY_HOOK_SIMPLE_GET condition. Then we set up a function call frame and re-enter the VM to execute the hook function:

php-src/Zend/zend_vm_def.h

Lines 2094 to 2126 in 7c2204c

	} else if (EXPECTED(ZEND_IS_PROPERTY_HOOK_SIMPLE_GET(prop_offset))) {
	zend_function *hook = prop_info->hooks[ZEND_PROPERTY_HOOK_GET];
	ZEND_ASSERT(hook->type == ZEND_USER_FUNCTION);
	ZEND_ASSERT(RUN_TIME_CACHE(&hook->op_array));
	uint32_t call_info = ZEND_CALL_NESTED_FUNCTION | ZEND_CALL_HAS_THIS;
	if (OP1_TYPE & IS_CV) {
	GC_ADDREF(zobj);
	}
	if (OP1_TYPE & (IS_CV|IS_VAR|IS_TMP_VAR)) {
	call_info |= ZEND_CALL_RELEASE_THIS;
	}
	zend_execute_data *call = zend_vm_stack_push_call_frame(call_info, hook, 0, zobj);
	call->prev_execute_data = execute_data;
	call->call = NULL;
	call->return_value = EX_VAR(opline->result.var);
	call->run_time_cache = RUN_TIME_CACHE(&hook->op_array);
	execute_data = call;
	EG(current_execute_data) = execute_data;
	zend_init_cvs(0, hook->op_array.last_var EXECUTE_DATA_CC);
	#if defined(ZEND_VM_IP_GLOBAL_REG) && ((ZEND_VM_KIND == ZEND_VM_KIND_CALL) || (ZEND_VM_KIND == ZEND_VM_KIND_HYBRID))
	opline = hook->op_array.opcodes;
	#else
	EX(opline) = hook->op_array.opcodes;
	#endif
	LOAD_OPLINE_EX();
	ZEND_OBSERVER_SAVE_OPLINE();
	ZEND_OBSERVER_FCALL_BEGIN(execute_data);
	ZEND_VM_ENTER_EX();
	}

This seems incompatible with how the minimal JIT works, getting the property will be skipped.
Indeed if we get rid of ZEND_SET_PROPERTY_HOOK_SIMPLE_GET in zend_object_handlers.c or go to a higher optimization level the problem disappears. I'm not sure yet how to solve that.

[dstogov]

I think you should commit the existent fix and open a new bug report.

[nielsdos]

Merged and opened #15834