[php-src] Issue #7946: Rewrite zend_eval_const_expr to use a stack on the heap instead of recursion where possible?
| From: | TysonAndre | 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