Skip to content

Commit 249e490

Browse files
committed
Fix constant evaluation of && and ||
The "return" in the for loop should have been a break on the switch, otherwise the result is just ignored... but because it prevents evaluation of the other operand, it also violates the invariant that everything has been constant evaluated, resulting in an assertion failure. The for loop isn't correct in any case though, because it's not legal to determine the result based on just the second operand, as the first one may have a side-effect that cannot be optimized away.
1 parent fac43d6 commit 249e490

File tree

2 files changed

+24
-12
lines changed

2 files changed

+24
-12
lines changed

Zend/tests/const_eval_and.phpt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
--TEST--
2+
Incorrect constant evaluation of and/or (OSS-Fuzz #19255)
3+
--FILE--
4+
<?php
5+
const C = 0 && __namespace__;
6+
var_dump(C);
7+
?>
8+
--EXPECT--
9+
bool(false)

Zend/zend_compile.c

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8549,25 +8549,28 @@ void zend_eval_const_expr(zend_ast **ast_ptr) /* {{{ */
85498549
case ZEND_AST_AND:
85508550
case ZEND_AST_OR:
85518551
{
8552-
int i;
8553-
for (i = 0; i <= 1; i++) {
8554-
zend_eval_const_expr(&ast->child[i]);
8555-
if (ast->child[i]->kind == ZEND_AST_ZVAL) {
8556-
if (zend_is_true(zend_ast_get_zval(ast->child[i])) == (ast->kind == ZEND_AST_OR)) {
8557-
ZVAL_BOOL(&result, ast->kind == ZEND_AST_OR);
8558-
return;
8559-
}
8560-
}
8552+
zend_bool child0_is_true, child1_is_true;
8553+
zend_eval_const_expr(&ast->child[0]);
8554+
zend_eval_const_expr(&ast->child[1]);
8555+
if (ast->child[0]->kind != ZEND_AST_ZVAL) {
8556+
return;
85618557
}
85628558

8563-
if (ast->child[0]->kind != ZEND_AST_ZVAL || ast->child[1]->kind != ZEND_AST_ZVAL) {
8559+
child0_is_true = zend_is_true(zend_ast_get_zval(ast->child[0]));
8560+
if (child0_is_true == (ast->kind == ZEND_AST_OR)) {
8561+
ZVAL_BOOL(&result, ast->kind == ZEND_AST_OR);
8562+
break;
8563+
}
8564+
8565+
if (ast->child[1]->kind != ZEND_AST_ZVAL) {
85648566
return;
85658567
}
85668568

8569+
child1_is_true = zend_is_true(zend_ast_get_zval(ast->child[1]));
85678570
if (ast->kind == ZEND_AST_OR) {
8568-
ZVAL_BOOL(&result, zend_is_true(zend_ast_get_zval(ast->child[0])) || zend_is_true(zend_ast_get_zval(ast->child[1])));
8571+
ZVAL_BOOL(&result, child0_is_true || child1_is_true);
85698572
} else {
8570-
ZVAL_BOOL(&result, zend_is_true(zend_ast_get_zval(ast->child[0])) && zend_is_true(zend_ast_get_zval(ast->child[1])));
8573+
ZVAL_BOOL(&result, child0_is_true && child1_is_true);
85718574
}
85728575
break;
85738576
}

0 commit comments

Comments
 (0)