print_r should not produce a fatal error

[mvorisek]

Description

print_r (and any function calling __debugInfo()) should not produce a fatal error, it should generate an \Error exception instead

demo https://3v4l.org/YTfSl

[cmb69]

Yeah, that zend_error_noreturn() looks overly strict, especially since return NULL would be accepted and properly handled. We could treat other return types likely like NULL:

 Zend/zend_object_handlers.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
index 28d0077878..f1e3c46299 100644
--- a/Zend/zend_object_handlers.c
+++ b/Zend/zend_object_handlers.c
@@ -150,7 +150,8 @@ ZEND_API HashTable *zend_std_get_debug_info(zend_object *object, int *is_temp) /
 	}
 
 	zend_call_known_instance_method_with_0_params(ce->__debugInfo, object, &retval);
-	if (Z_TYPE(retval) == IS_ARRAY) {
+	switch (Z_TYPE(retval)) {
+		case IS_ARRAY:
 		if (!Z_REFCOUNTED(retval)) {
 			*is_temp = 1;
 			return zend_array_dup(Z_ARRVAL(retval));
@@ -163,14 +164,15 @@ ZEND_API HashTable *zend_std_get_debug_info(zend_object *object, int *is_temp) /
 			zval_ptr_dtor(&retval);
 			return Z_ARRVAL(retval);
 		}
-	} else if (Z_TYPE(retval) == IS_NULL) {
+		default:
+		zend_throw_error(NULL, ZEND_DEBUGINFO_FUNC_NAME "() must return an array");
+		/* fallthrough */
+		case IS_NULL:
 		*is_temp = 1;
 		ht = zend_new_array(0);
 		return ht;
 	}
 
-	zend_error_noreturn(E_ERROR, ZEND_DEBUGINFO_FUNC_NAME "() must return an array");
-
 	return NULL; /* Compilers are dumb and don't understand that noreturn means that the function does NOT need a return value... */
 }
 /* }}} */

(improper indentation to simply the diff)

[TysonAndre]

In addition to throwing, I think you'd need to call zval_ptr_dtor to free incorrect return values of type strings/objects/resources from __debugInfo but it seems reasonable otherwise

[flack]

I've recently encountered what I think is a manifestation of this bug:

class test
{
    public function __debugInfo()
    {
        throw new Exception('X');
    }
}

function error(test $object)
{
    throw new Exception('X');
}

try {
    error(new test);
} catch (Exception $e) {
    echo 'All is well';
}

When you run this on standard PHP, it'll print All is well. But when you have XDebug enabled, you'll get something like:

PHP Warning:  Uncaught Exception: X in test.php:5
Stack trace:
#0 test.php(11): test->__debugInfo()
#1 test.php(15): error()
#2 {main}
  thrown in test.php on line 5
PHP Stack trace:
PHP   1. {main}() test.php:0
PHP   2. error($object = class test {  }) test.php:14
PHP Fatal error:  __debuginfo() must return an array in test.php on line 11
PHP Stack trace:
PHP   1. {main}() test.php:0
PHP   2. error($object = class test {  }) test.php:15

I'm not sure, but my guess is that when the Exception is created, XDebug tries to augment the backtrace by adding the arguments, which internally calls var_dump or similar, which then triggers a fatal error. So you can have code that works flawlessly in a production environment, but it breaks when you're trying to debug it

[mvorisek]

print_r might be a good candidate for plain php impl. once #18204 is merged.