[php-src] Issue #7946: Rewrite zend_eval_const_expr to use a stack on the heap instead of recursion where possible?

From: Date: Sat, 15 Jan 2022 15:19:33 +0000
Subject: [php-src] Issue #7946: Rewrite zend_eval_const_expr to use a stack on the heap instead of recursion where possible?
Groups: php.bugs 
Request: Send a blank email to php-bugs+get-239022@lists.php.net to get a copy of this message
Issue: https://github.com/php/php-src/issues/7946 Author: TysonAndre ### Description This is the same as https://bugs.php.net/bug.php?id=79307 with a proposed solution for ZEND_BINARY_OP. It was brought up in https://github.com/php/php-src/pull/7940 . Filing this approach to track refactoring zend_eval_const_expr (or the reason why we don't do this) for future reference. This would help with binary operators, but not with ast kinds that call different recursive methods, e.g. ZEND_AST_ARRAY The following code: ```php php > $x = eval('return ' . var_export(str_repeat("\0", 100000), true) . ';'); ``` Resulted in this output: ``` [1] 16259 segmentation fault (core dumped) php -a ``` But I expected this output instead: $x is assigned. ------ An alternative approach would be to rewrite zend_eval_const_expr with a C stack (pointer, capacity, size) on the heap (with emalloc/erealloc) in cases where it calls itself. (https://softwareengineering.stackexchange.com/questions/279004/general-way-to-convert-a-loop-while-for-to-recursion-or-from-a-recursion-to-a) I'm not sure if there'd be objections to that for readability/maintainability/performance, though, and it doesn't help with deeply nested arrays - the stack allocation can be avoided if nothing is pushed. @nikic are there reasons to not do that? > Could be addressed by converting concat into a list node with some special handling, not sure > if that's worth the bother. (Incomplete) Pseudocode is written as a comment alongside the unmodified original code. ```c static void zend_eval_const_expr(zend_ast **ast_ptr) /* {{{ */ { zend_ast *ast = *ast_ptr; zval result; if (!ast) { return; } // Stack would be: // 1. Child nodes to zend_eval_const_expr inside of type != ZEND_AST_ZVAL // 2. State - whether this is starting or being completed (0, 1) // // Pseudocode // node_state = UNPROCESSED; // do { // switch(ast->kind) { // case ZEND_AST_BINARY_OP: // if (node_state == UNPROCESSED) { // // push_stack(stack, ast->child[i], PROCESSED) // unless there's nothing needed and all are already ZEND_AST_ZVAL // for child nodes needed where kind != ZEND_AST_ZVAL // push_stack(stack, ast->child[i], UNPROCESSED) // break; // break, unless there's nothing needed // } // finish evaluating this ast // break; // case OTHERS....: // } // ast_ptr, node_state = pop_stack(stack, ast); // e.g. use low bit // } switch (ast->kind) { case ZEND_AST_BINARY_OP: zend_eval_const_expr(&ast->child[0]); zend_eval_const_expr(&ast->child[1]); if (ast->child[0]->kind != ZEND_AST_ZVAL || ast->child[1]->kind != ZEND_AST_ZVAL) { return; } if (!zend_try_ct_eval_binary_op(&result, ast->attr, zend_ast_get_zval(ast->child[0]), zend_ast_get_zval(ast->child[1])) ) { return; } break; ``` ### PHP Version 8.1/any ### Operating System Linux Mint

« previous php.bugs (#239022) next »