Fix GH-12837: Combination of phpdbg and Generator method causes memory leak

[ndossche]

The fix for bug #72523 (d1dd474) added a check on zend_execute_ex to add an extra refcount to This. This is necessary because the VM does a re-entry here:

php-src/Zend/zend_vm_def.h

Lines 4294 to 4299 in ecb90c1

	SAVE_OPLINE_EX();
	ZEND_OBSERVER_FCALL_BEGIN(execute_data);
	execute_data = EX(prev_execute_data);
	LOAD_OPLINE();
	ZEND_ADD_CALL_FLAG(call, ZEND_CALL_TOP);
	zend_execute_ex(call);

and then cleans up This here:

php-src/Zend/zend_vm_def.h

Lines 4358 to 4360 in ecb90c1

	if (UNEXPECTED(ZEND_CALL_INFO(call) & ZEND_CALL_RELEASE_THIS)) {
	OBJ_RELEASE(Z_OBJ(call->This));
	}

So if we don't add the refcount, we destroy This both in the VM and in zend_generator_close.

The reason this causes a leak in phpdbg is because phpdbg temporarily changes zend_execute_ex to the default here for ZEND_DO_FCALL:

php-src/sapi/phpdbg/phpdbg_prompt.c

Lines 1820 to 1827 in ecb90c1

	if ((execute_data->opline->opcode == ZEND_DO_FCALL ||
	execute_data->opline->opcode == ZEND_DO_UCALL ||
	execute_data->opline->opcode == ZEND_DO_FCALL_BY_NAME) &&
	execute_data->call->func->type == ZEND_USER_FUNCTION) {
	zend_execute_ex = execute_ex;
	}
	PHPDBG_G(vmret) = zend_vm_call_opcode_handler(execute_data);
	zend_execute_ex = phpdbg_execute_ex;

which means that we only execute OBJ_RELEASE on the This object once in zend_generator_close instead of also doing it in the ZEND_DO_FCALL handler.

To solve this, we make sure the check for the re-entrancy is consistently done between ZEND_DO_FCALL and ZEND_GENERATOR_CREATE by checking the ZEND_CALL_TOP flag instead of relying solely on execute_ex pointer, because this flag is set when zend_execute_ex was changed:

php-src/Zend/zend_vm_def.h

Line 4298 in ecb90c1

ZEND_ADD_CALL_FLAG(call, ZEND_CALL_TOP);

It may over-approximate if called via zend_call_function, as that sets the ZEND_CALL_TOP flag too, but we could filter that by checking for ZEND_CALL_DYNAMIC too.

Alternatively, we can fix this solely in phpdbg by also executing GENERATOR_CREATE with the normal execution handler, but I think the real problem is in the VM inconsistency.

[dstogov]

The problem is a bit more complex.
It seems the phpdbg "stupid hack" doesn't work.
Somehow zend_vm_call_opcode_handler() leads to execution of iterator's code with original execute_ex.

Anyway, changing zend_execute_ex at run-time is really bad idea.
This is completely incompatible with ZTS and optimizations in zend_compile.c.
I think this should be solved in phpdbg.
@bwoebi please also take a loook.

@nielsdos may be #72523 might be fixed in some better way. May be introduce some special ZEND_CALL_??? flag. Could you please think a bit more.

Anyway, this won't fix weird phpdbg behavior.