print_r should not produce a fatal error #7922

Activity

1. mvorisek

   added

   Feature

   Status: Needs Triage

   on Jan 10, 2022
2. cmb69 commented on Jan 10, 2022

   

   cmb69

   on Jan 10, 2022

   Member

   More actions

   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)
3. cmb69

   added

   Category: Engine

   Status: Verified

   and removed

   Status: Needs Triage

   on Jan 10, 2022
4. TysonAndre commented on Jan 12, 2022

   

   TysonAndre

   on Jan 12, 2022

   Contributor

   More actions

   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

   👍React with 👍2Reacted by Christoph M. Becker and Cristian Rodríguez
5. cmb69

   added a commit that references this issue on Jan 18, 2022

   Implement phpGH-7922: print_r should not produce a fatal error

   ...

   ca77cbc
6. cmb69

   linked a pull request that will close this issueImplement GH-7922: print_r should not produce a fatal error #7964on Jan 18, 2022
7. flack commented on Jun 28, 2022

   

   flack

   on Jun 28, 2022

   More actions

   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
8. mvorisek commented on May 11, 2025

   

   mvorisek

   on May 11, 2025

   ContributorAuthor

   More actions

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