Could you elaborate on what this patch is trying to do?

There's more information here https://bugs.php.net/bug.php?id=63914 and here http://bugs.xdebug.org/view.php?id=897.

The setup for the bug looks something like this:

<?php
class PHPUnit_Util_ErrorHandler
{
    public static function handleError($errno, $errstr, $errfile, $errline)
    {
        throw new Exception;
    }
}

set_error_handler(
  array('PHPUnit_Util_ErrorHandler', 'handleError'),
  E_ALL | E_STRICT
);

$dom = new DOMDocument;
$dom->saveXML('foo');

When zend_verify_arg_type is called in zend_do_fcall_common_helper_SPEC for $dom->saveXML('foo') it triggers the user-defined error handler which throws an uncaught exception. After this, there's really no sense in calling the internal function.

In fact, in the case that you're using an extension that hooks into zend_execute_internal and later calls execute_internal (like xdebug), this can actually cause the internal function to be called with EX(opline) containing exception handling operations from the earlier zend_throw_exception_internal. In PHP <= 5.4, this causes a segfault as the result structure is not initialized properly. In PHP >= 5.5, the segfault is no longer present, but $dom->saveXML('foo') is still executed to completion as if it were called with no arguments.

The best way to follow this is to set up the gdb sessions from https://bugs.php.net/bug.php?id=63914 and step through the execution to see what's happening.

This patch intends to check for an exception after zend_verify_arg_type and avoid the internal function call if it is not necessary.

Alright, that’s above my head :)

A little more context:

This bug is very closely related to https://bugs.php.net/bug.php?id=45805 which was fixed here: 1ff61ab.

The bug exists in the following section: https://github.com/php/php-src/blob/642721b3/Zend/zend_vm_def.h#L1997-L2013.

My patch adds a check to handle the uncaught exception originating from zend_verify_arg_type: https://github.com/whatthejeff/php-src/blob/493b3c374c77587e96ef15527f6bfd954cf87368/Zend/zend_vm_def.h#L1997-L2017.

This patch can be backported to PHP 5.4 (you'll need to update the zend_execute_internal signature), and it fixes the segfault described here: http://bugs.xdebug.org/view.php?id=897.

Maybe @derickr or @sebastianbergmann can take a look at this if they get a minute :)

Bump :)

Goes a bit over my head as well! Maybe @dstogov knows?

Patch looks good to me. At least it makes sense that if an exception is thrown during argument type verification the function should not be run.

Only thing you might want to do is shift the order of the code around so you only initialize the retval if there is no exception (so you don't have to alloc+dtor it when one is thrown):

        if (fbc->common.arg_info) {
            zend_uint i=0;
            zval **p = (zval**)EX(function_state).arguments;
            ulong arg_count = opline->extended_value;

            while (arg_count>0) {
                zend_verify_arg_type(fbc, ++i, *(p-arg_count), 0 TSRMLS_CC);
                arg_count--;
            }
        }

        if (EXPECTED(EG(exception) == NULL)) {
            temp_variable *ret = &EX_T(opline->result.var);

            MAKE_STD_ZVAL(ret->var.ptr);
            ZVAL_NULL(ret->var.ptr);
            ret->var.ptr_ptr = &ret->var.ptr;
            ret->var.fcall_returned_reference = (fbc->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) != 0;

            if (!zend_execute_internal) {
                /* saves one function call if zend_execute_internal is not used */
                fbc->internal_function.handler(opline->extended_value, ret->var.ptr, (fbc->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) ? &ret->var.ptr : NULL, EX(object), RETURN_VALUE_USED(opline) TSRMLS_CC);
            } else {
                zend_execute_internal(execute_data, NULL, RETURN_VALUE_USED(opline) TSRMLS_CC);
            }

            if (!RETURN_VALUE_USED(opline)) {
                zval_ptr_dtor(&ret->var.ptr);
            }
        }

Good point, @nikic. I've updated the fix to reflect your suggestions.

Looks like @dstogov added this at git.php.net. Thanks :)