Skip to content

Avoid throw expression leaks #5450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Avoid throw expression leaks #5450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions Zend/tests/throw/leaks.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
throw expression should not leak temporaries
--FILE--
<?php

try {
new stdClass(throw new Exception);
} catch (Exception $e) {
echo "Caught\n";
}

try {
$a = [];
($a + [1]) + throw new Exception;
} catch (Exception $e) {
echo "Caught\n";
}

try {
@throw new Exception;
} catch (Exception $e) {
echo "Caught\n";
}
var_dump(error_reporting());

?>
--EXPECT--
Caught
Caught
Caught
int(32767)
Copy link
Contributor

@TysonAndre TysonAndre Apr 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this use the following to override the users php.ini settings instead, to avoid test failures when a tester already installed something to the configured --prefix and added ini settings?

--INI--
error_reporting=E_ALL

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already enforced by run-tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See

php-src/run-tests.php

Line 236 in d5e51fc

$ini_overwrites = array(
for list of ini settings that run-tests overwrites.

14 changes: 10 additions & 4 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4548,10 +4548,13 @@ void zend_compile_throw(znode *result, zend_ast *ast) /* {{{ */
znode expr_node;
zend_compile_expr(&expr_node, expr_ast);

zend_emit_op(NULL, ZEND_THROW, &expr_node, NULL);

result->op_type = IS_CONST;
ZVAL_BOOL(&result->u.constant, 1);
zend_op *opline = zend_emit_op(NULL, ZEND_THROW, &expr_node, NULL);
if (result) {
/* Mark this as an "expression throw" for opcache. */
opline->extended_value = ZEND_THROW_IS_EXPR;
result->op_type = IS_CONST;
ZVAL_BOOL(&result->u.constant, 1);
}
}
/* }}} */

Expand Down Expand Up @@ -8789,6 +8792,9 @@ void zend_compile_stmt(zend_ast *ast) /* {{{ */
case ZEND_AST_HALT_COMPILER:
zend_compile_halt_compiler(ast);
break;
case ZEND_AST_THROW:
zend_compile_throw(NULL, ast);
break;
default:
{
znode result;
Expand Down
2 changes: 2 additions & 0 deletions Zend/zend_compile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,8 @@ ZEND_API zend_string *zend_type_to_string(zend_type type);
#define ZEND_SEND_BY_REF 1u
#define ZEND_SEND_PREFER_REF 2u

#define ZEND_THROW_IS_EXPR 1u

/* The send mode and is_variadic flag are stored as part of zend_type */
#define _ZEND_SEND_MODE_SHIFT _ZEND_TYPE_EXTRA_FLAGS_SHIFT
#define _ZEND_IS_VARIADIC_BIT (1 << (_ZEND_TYPE_EXTRA_FLAGS_SHIFT + 2))
Expand Down
8 changes: 7 additions & 1 deletion ext/opcache/Optimizer/zend_cfg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,11 +296,17 @@ int zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t b
case ZEND_RETURN_BY_REF:
case ZEND_GENERATOR_RETURN:
case ZEND_EXIT:
case ZEND_THROW:
if (i + 1 < op_array->last) {
BB_START(i + 1);
}
break;
case ZEND_THROW:
/* Don't treat THROW as terminator if it's used in expression context,
* as we may lose live ranges when eliminating unreachable code. */
if (opline->extended_value != ZEND_THROW_IS_EXPR && i + 1 < op_array->last) {
BB_START(i + 1);
}
break;
case ZEND_INCLUDE_OR_EVAL:
flags |= ZEND_FUNC_INDIRECT_VAR_ACCESS;
case ZEND_GENERATOR_CREATE:
Expand Down