Skip to content

Commit cf9b030

Browse files
committed
Fix GH-8841: php-cli core dump calling a badly formed function
It's actually not php-cli specific, nor SAPI specific. We should delay the registration of the function into the function table until after the compilation was successful, otherwise the function is mistakingly registered and a NULL dereference will happen when trying to call it. I based my test of Nikita's test, so credits to him for the test: #8933 (comment) Closes GH-10989.
1 parent 66ce205 commit cf9b030

File tree

3 files changed

+47
-13
lines changed

3 files changed

+47
-13
lines changed

NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ PHP NEWS
44

55
- Core:
66
. Fix inconsistent float negation in constant expressions. (ilutov)
7+
. Fixed bug GH-8841 (php-cli core dump calling a badly formed function).
8+
(nielsdos)
79

810
- DOM:
911
. Fixed bug #80602 (Segfault when using DOMChildNode::before()).

Zend/tests/gh8841.phpt

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
GH-8841 (php-cli core dump calling a badly formed function)
3+
--FILE--
4+
<?php
5+
register_shutdown_function(function() {
6+
echo "Before calling g()\n";
7+
g(1);
8+
echo "After calling g()\n";
9+
});
10+
11+
register_shutdown_function(function() {
12+
echo "Before calling f()\n";
13+
f(1);
14+
echo "After calling f()\n";
15+
});
16+
17+
eval('function g($x): int { return $x; }');
18+
eval('function f($x): void { return $x; }');
19+
?>
20+
--EXPECTF--
21+
Fatal error: A void function must not return a value in %s on line %d
22+
Before calling g()
23+
After calling g()
24+
Before calling f()
25+
26+
Fatal error: Uncaught Error: Call to undefined function f() in %s:%d
27+
Stack trace:
28+
#0 [internal function]: {closure}()
29+
#1 {main}
30+
thrown in %s on line %d

Zend/zend_compile.c

+15-13
Original file line numberDiff line numberDiff line change
@@ -7126,7 +7126,7 @@ static uint32_t zend_add_dynamic_func_def(zend_op_array *def) {
71267126
return def_offset;
71277127
}
71287128

7129-
static void zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_ast_decl *decl, bool toplevel) /* {{{ */
7129+
static zend_string *zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_ast_decl *decl, bool toplevel) /* {{{ */
71307130
{
71317131
zend_string *unqualified_name, *name, *lcname;
71327132
zend_op *opline;
@@ -7157,11 +7157,7 @@ static void zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_as
71577157

71587158
zend_register_seen_symbol(lcname, ZEND_SYMBOL_FUNCTION);
71597159
if (toplevel) {
7160-
if (UNEXPECTED(zend_hash_add_ptr(CG(function_table), lcname, op_array) == NULL)) {
7161-
do_bind_function_error(lcname, op_array, 1);
7162-
}
7163-
zend_string_release_ex(lcname, 0);
7164-
return;
7160+
return lcname;
71657161
}
71667162

71677163
uint32_t func_ref = zend_add_dynamic_func_def(op_array);
@@ -7175,7 +7171,8 @@ static void zend_begin_func_decl(znode *result, zend_op_array *op_array, zend_as
71757171
LITERAL_STR(opline->op1, zend_string_copy(lcname));
71767172
opline->op2.num = func_ref;
71777173
}
7178-
zend_string_release_ex(lcname, 0);
7174+
7175+
return lcname;
71797176
}
71807177
/* }}} */
71817178

@@ -7187,7 +7184,7 @@ static void zend_compile_func_decl(znode *result, zend_ast *ast, bool toplevel)
71877184
zend_ast *stmt_ast = decl->child[2];
71887185
zend_ast *return_type_ast = decl->child[3];
71897186
bool is_method = decl->kind == ZEND_AST_METHOD;
7190-
zend_string *method_lcname = NULL;
7187+
zend_string *lcname;
71917188

71927189
zend_class_entry *orig_class_entry = CG(active_class_entry);
71937190
zend_op_array *orig_op_array = CG(active_op_array);
@@ -7219,9 +7216,9 @@ static void zend_compile_func_decl(znode *result, zend_ast *ast, bool toplevel)
72197216

72207217
if (is_method) {
72217218
bool has_body = stmt_ast != NULL;
7222-
method_lcname = zend_begin_method_decl(op_array, decl->name, has_body);
7219+
lcname = zend_begin_method_decl(op_array, decl->name, has_body);
72237220
} else {
7224-
zend_begin_func_decl(result, op_array, decl, toplevel);
7221+
lcname = zend_begin_func_decl(result, op_array, decl, toplevel);
72257222
if (decl->kind == ZEND_AST_ARROW_FUNC) {
72267223
find_implicit_binds(&info, params_ast, stmt_ast);
72277224
compile_implicit_lexical_binds(&info, result, op_array);
@@ -7264,7 +7261,7 @@ static void zend_compile_func_decl(znode *result, zend_ast *ast, bool toplevel)
72647261
}
72657262

72667263
zend_compile_params(params_ast, return_type_ast,
7267-
is_method && zend_string_equals_literal(method_lcname, ZEND_TOSTRING_FUNC_NAME) ? IS_STRING : 0);
7264+
is_method && zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME) ? IS_STRING : 0);
72687265
if (CG(active_op_array)->fn_flags & ZEND_ACC_GENERATOR) {
72697266
zend_mark_function_as_generator();
72707267
zend_emit_op(NULL, ZEND_GENERATOR_CREATE, NULL, NULL);
@@ -7280,9 +7277,14 @@ static void zend_compile_func_decl(znode *result, zend_ast *ast, bool toplevel)
72807277
if (is_method) {
72817278
CG(zend_lineno) = decl->start_lineno;
72827279
zend_check_magic_method_implementation(
7283-
CG(active_class_entry), (zend_function *) op_array, method_lcname, E_COMPILE_ERROR);
7284-
zend_string_release_ex(method_lcname, 0);
7280+
CG(active_class_entry), (zend_function *) op_array, lcname, E_COMPILE_ERROR);
7281+
} else if (toplevel) {
7282+
/* Only register the function after a successful compile */
7283+
if (UNEXPECTED(zend_hash_add_ptr(CG(function_table), lcname, op_array) == NULL)) {
7284+
do_bind_function_error(lcname, op_array, true);
7285+
}
72857286
}
7287+
zend_string_release_ex(lcname, 0);
72867288

72877289
/* put the implicit return on the really last line */
72887290
CG(zend_lineno) = decl->end_lineno;

0 commit comments

Comments
 (0)