From b466e8b7546352f7c2ac7d166d83131d3dcd7b2f Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 20 Jul 2020 10:20:29 +0200 Subject: [PATCH 1/9] Add missing arg num check for __set_state --- Zend/tests/magic_methods_set_state.phpt | 2 +- Zend/zend_API.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Zend/tests/magic_methods_set_state.phpt b/Zend/tests/magic_methods_set_state.phpt index 389798e54291d..9ff728dd0556a 100644 --- a/Zend/tests/magic_methods_set_state.phpt +++ b/Zend/tests/magic_methods_set_state.phpt @@ -4,7 +4,7 @@ Testing __set_state() declaration with wrong modifier Date: Mon, 20 Jul 2020 10:39:43 +0200 Subject: [PATCH 2/9] Unify static/non-static check for magic methods And promote it to be fatal. --- Zend/tests/bug61025.phpt | 6 -- Zend/tests/bug70215.phpt | 3 +- Zend/tests/errmsg_032.phpt | 2 +- Zend/tests/errmsg_033.phpt | 2 +- Zend/tests/errmsg_034.phpt | 2 +- Zend/tests/magic_methods_003.phpt | 2 +- Zend/tests/magic_methods_005.phpt | 2 +- Zend/tests/magic_methods_006.phpt | 2 +- Zend/tests/magic_methods_010.phpt | 2 - Zend/tests/magic_methods_serialize.phpt | 2 - Zend/tests/magic_methods_set_state.phpt | 2 +- Zend/tests/magic_methods_unserialize.phpt | 2 - Zend/zend_API.c | 92 +++++++++-------------- Zend/zend_compile.c | 52 ++++--------- tests/classes/__call_007.phpt | 76 ------------------- 15 files changed, 58 insertions(+), 191 deletions(-) delete mode 100644 tests/classes/__call_007.phpt diff --git a/Zend/tests/bug61025.phpt b/Zend/tests/bug61025.phpt index 49adea85249de..d453d73d26c1d 100644 --- a/Zend/tests/bug61025.phpt +++ b/Zend/tests/bug61025.phpt @@ -3,10 +3,6 @@ Bug #61025 (__invoke() visibility not honored) --FILE-- __invoke(); ?> --EXPECTF-- -Warning: The magic method InvokeAble::__invoke() cannot be static in %sbug61025.php on line %d - Warning: The magic method Bar::__invoke() must have public visibility in %sbug61025.php on line %d Bar Fatal error: Uncaught Error: Call to private method Bar::__invoke() from global scope in %s:%d diff --git a/Zend/tests/bug70215.phpt b/Zend/tests/bug70215.phpt index 60fa802a7f972..50c1d057ca50c 100644 --- a/Zend/tests/bug70215.phpt +++ b/Zend/tests/bug70215.phpt @@ -17,5 +17,4 @@ $b(); ?> --EXPECTF-- -Warning: The magic method A::__invoke() cannot be static in %s on line %d -A +Fatal error: Method A::__invoke() cannot be static in %s on line %d diff --git a/Zend/tests/errmsg_032.phpt b/Zend/tests/errmsg_032.phpt index 20c2762a3220f..401f7073ea6da 100644 --- a/Zend/tests/errmsg_032.phpt +++ b/Zend/tests/errmsg_032.phpt @@ -12,4 +12,4 @@ class test { echo "Done\n"; ?> --EXPECTF-- -Fatal error: Constructor test::__construct() cannot be static in %s on line %d +Fatal error: Method test::__construct() cannot be static in %s on line %d diff --git a/Zend/tests/errmsg_033.phpt b/Zend/tests/errmsg_033.phpt index 72b1490ea5d0b..f71b17ce16293 100644 --- a/Zend/tests/errmsg_033.phpt +++ b/Zend/tests/errmsg_033.phpt @@ -12,4 +12,4 @@ class test { echo "Done\n"; ?> --EXPECTF-- -Fatal error: Destructor test::__destruct() cannot be static in %s on line %d +Fatal error: Method test::__destruct() cannot be static in %s on line %d diff --git a/Zend/tests/errmsg_034.phpt b/Zend/tests/errmsg_034.phpt index 940754ad7b457..97a7f1c2a6efd 100644 --- a/Zend/tests/errmsg_034.phpt +++ b/Zend/tests/errmsg_034.phpt @@ -12,4 +12,4 @@ class test { echo "Done\n"; ?> --EXPECTF-- -Fatal error: Clone method test::__clone() cannot be static in %s on line %d +Fatal error: Method test::__clone() cannot be static in %s on line %d diff --git a/Zend/tests/magic_methods_003.phpt b/Zend/tests/magic_methods_003.phpt index 9f7ff5e9a51c4..d85983a419871 100644 --- a/Zend/tests/magic_methods_003.phpt +++ b/Zend/tests/magic_methods_003.phpt @@ -11,4 +11,4 @@ class foo { ?> --EXPECTF-- -Warning: The magic method foo::__unset() cannot be static in %s on line %d +Fatal error: Method foo::__unset() cannot be static in %s on line %d diff --git a/Zend/tests/magic_methods_005.phpt b/Zend/tests/magic_methods_005.phpt index fe1cfa34e484c..cac833f0ce62a 100644 --- a/Zend/tests/magic_methods_005.phpt +++ b/Zend/tests/magic_methods_005.phpt @@ -9,4 +9,4 @@ interface a { ?> --EXPECTF-- -Warning: The magic method a::__call() cannot be static in %s on line %d +Fatal error: Method a::__call() cannot be static in %s on line %d diff --git a/Zend/tests/magic_methods_006.phpt b/Zend/tests/magic_methods_006.phpt index 00fe599fc7182..b07764ec6f76a 100644 --- a/Zend/tests/magic_methods_006.phpt +++ b/Zend/tests/magic_methods_006.phpt @@ -9,4 +9,4 @@ interface a { ?> --EXPECTF-- -Warning: The magic method a::__callStatic() must be static in %s on line %d +Fatal error: Method a::__callStatic() must be static in %s on line %d diff --git a/Zend/tests/magic_methods_010.phpt b/Zend/tests/magic_methods_010.phpt index b93d6e19f170e..fda1cc1e10a22 100644 --- a/Zend/tests/magic_methods_010.phpt +++ b/Zend/tests/magic_methods_010.phpt @@ -12,6 +12,4 @@ class a { --EXPECTF-- Warning: The magic method a::__toString() must have public visibility in %s on line %d -Warning: The magic method a::__toString() cannot be static in %s on line %d - Fatal error: Method a::__toString() cannot take arguments in %s on line %d diff --git a/Zend/tests/magic_methods_serialize.phpt b/Zend/tests/magic_methods_serialize.phpt index 3cf3b5b159205..2970c4f36cbf0 100644 --- a/Zend/tests/magic_methods_serialize.phpt +++ b/Zend/tests/magic_methods_serialize.phpt @@ -7,6 +7,4 @@ class Foo { } ?> --EXPECTF-- -Warning: The magic method Foo::__serialize() cannot be static in %s on line %d - Fatal error: Method Foo::__serialize() cannot take arguments in %s on line %d diff --git a/Zend/tests/magic_methods_set_state.phpt b/Zend/tests/magic_methods_set_state.phpt index 9ff728dd0556a..301dbb84dae77 100644 --- a/Zend/tests/magic_methods_set_state.phpt +++ b/Zend/tests/magic_methods_set_state.phpt @@ -11,4 +11,4 @@ class Foo { ?> --EXPECTF-- -Warning: The magic method Foo::__set_state() must be static in %s on line %d +Fatal error: Method Foo::__set_state() must be static in %s on line %d diff --git a/Zend/tests/magic_methods_unserialize.phpt b/Zend/tests/magic_methods_unserialize.phpt index 5feedd554df1c..b0a867c74dd43 100644 --- a/Zend/tests/magic_methods_unserialize.phpt +++ b/Zend/tests/magic_methods_unserialize.phpt @@ -7,6 +7,4 @@ class Foo { } ?> --EXPECTF-- -Warning: The magic method Foo::__unserialize() cannot be static in %s on line %d - Fatal error: Method Foo::__unserialize() must take exactly 1 argument in %s on line %d diff --git a/Zend/zend_API.c b/Zend/zend_API.c index f6e96df42fb16..a697c453ce264 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2033,6 +2033,22 @@ static void zend_check_magic_method_args( } } +static void zend_check_magic_method_non_static( + const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) +{ + if (fptr->common.fn_flags & ZEND_ACC_STATIC) { + zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(ce->name), name); + } +} + +static void zend_check_magic_method_static( + const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) +{ + if (!(fptr->common.fn_flags & ZEND_ACC_STATIC)) { + zend_error(error_type, "Method %s::%s() must be static", ZSTR_VAL(ce->name), name); + } +} + ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce, const zend_function *fptr, zend_string *lcname, int error_type) /* {{{ */ { if (ZSTR_VAL(fptr->common.function_name)[0] != '_' @@ -2040,32 +2056,49 @@ ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce, return; } - if (zend_string_equals_literal(lcname, ZEND_DESTRUCTOR_FUNC_NAME)) { + if (zend_string_equals_literal(lcname, ZEND_CONSTRUCTOR_FUNC_NAME)) { + zend_check_magic_method_non_static("__construct", ce, fptr, error_type); + } else if (zend_string_equals_literal(lcname, ZEND_DESTRUCTOR_FUNC_NAME)) { zend_check_magic_method_args(0, "__destruct", ce, fptr, error_type); + zend_check_magic_method_non_static("__destruct", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CLONE_FUNC_NAME)) { zend_check_magic_method_args(0, "__clone", ce, fptr, error_type); + zend_check_magic_method_non_static("__clone", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) { zend_check_magic_method_args(1, "__get", ce, fptr, error_type); + zend_check_magic_method_non_static("__get", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) { zend_check_magic_method_args(2, "__set", ce, fptr, error_type); + zend_check_magic_method_non_static("__set", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) { zend_check_magic_method_args(1, "__unset", ce, fptr, error_type); + zend_check_magic_method_non_static("__unset", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) { zend_check_magic_method_args(1, "__isset", ce, fptr, error_type); + zend_check_magic_method_non_static("__isset", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) { zend_check_magic_method_args(2, "__call", ce, fptr, error_type); + zend_check_magic_method_non_static("__call", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) { zend_check_magic_method_args(2, "__callStatic", ce, fptr, error_type); + zend_check_magic_method_static("__callStatic", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) { zend_check_magic_method_args(0, "__toString", ce, fptr, error_type); + zend_check_magic_method_non_static("__toString", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) { zend_check_magic_method_args(0, "__debugInfo", ce, fptr, error_type); + zend_check_magic_method_non_static("__debugInfo", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__serialize")) { zend_check_magic_method_args(0, "__serialize", ce, fptr, error_type); + zend_check_magic_method_non_static("__serialize", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__unserialize")) { zend_check_magic_method_args(1, "__unserialize", ce, fptr, error_type); + zend_check_magic_method_non_static("__unserialize", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__set_state")) { zend_check_magic_method_args(1, "__set_state", ce, fptr, error_type); + zend_check_magic_method_static("__set_state", ce, fptr, error_type); + } else if (zend_string_equals_literal(lcname, "__invoke")) { + zend_check_magic_method_non_static("__invoke", ce, fptr, error_type); } } /* }}} */ @@ -2285,7 +2318,7 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio } if (reg_function) { zend_check_magic_method_implementation( - scope, reg_function, lowercase_name, error_type); + scope, reg_function, lowercase_name, E_CORE_ERROR); } } ptr++; @@ -2325,62 +2358,7 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio scope->__unserialize = __unserialize; if (ctor) { ctor->common.fn_flags |= ZEND_ACC_CTOR; - if (ctor->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Constructor %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(ctor->common.function_name)); - } - } - if (dtor) { - if (dtor->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Destructor %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(dtor->common.function_name)); - } - } - if (clone) { - if (clone->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "%s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(clone->common.function_name)); - } - } - if (__call) { - if (__call->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(__call->common.function_name)); - } } - if (__callstatic) { - if (!(__callstatic->common.fn_flags & ZEND_ACC_STATIC)) { - zend_error(error_type, "Method %s::%s() must be static", ZSTR_VAL(scope->name), ZSTR_VAL(__callstatic->common.function_name)); - } - __callstatic->common.fn_flags |= ZEND_ACC_STATIC; - } - if (__tostring) { - if (__tostring->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(__tostring->common.function_name)); - } - } - if (__get) { - if (__get->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(__get->common.function_name)); - } - } - if (__set) { - if (__set->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(__set->common.function_name)); - } - } - if (__unset) { - if (__unset->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(__unset->common.function_name)); - } - } - if (__isset) { - if (__isset->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(__isset->common.function_name)); - } - } - if (__debugInfo) { - if (__debugInfo->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(scope->name), ZSTR_VAL(__debugInfo->common.function_name)); - } - } - if (ctor && (ctor->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE)) { zend_error_noreturn(E_CORE_ERROR, "Constructor %s::%s() cannot declare a return type", ZSTR_VAL(scope->name), ZSTR_VAL(ctor->common.function_name)); } diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 4529aea5ba008..dbb37bbadbe6e 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -6443,25 +6443,13 @@ static void zend_compile_implicit_closure_uses(closure_info *info) ZEND_HASH_FOREACH_END(); } -static void zend_check_magic_method_attr(uint32_t attr, zend_class_entry *ce, const char* method, zend_bool is_static) /* {{{ */ +static void zend_check_magic_method_attr(uint32_t attr, zend_class_entry *ce, const char* method) /* {{{ */ { if (!(attr & ZEND_ACC_PUBLIC)) { zend_error(E_WARNING, "The magic method %s::%s() must have public visibility", ZSTR_VAL(ce->name), method); } - - if (is_static) { - if (!(attr & ZEND_ACC_STATIC)) { - zend_error(E_WARNING, - "The magic method %s::%s() must be static", - ZSTR_VAL(ce->name), method); - } - } else if (attr & ZEND_ACC_STATIC) { - zend_error(E_WARNING, - "The magic method %s::%s() cannot be static", - ZSTR_VAL(ce->name), method); - } } /* }}} */ @@ -6540,44 +6528,44 @@ zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string *name, } else if (zend_string_equals_literal(lcname, ZEND_CLONE_FUNC_NAME)) { ce->clone = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__call", 0); + zend_check_magic_method_attr(fn_flags, ce, "__call"); ce->__call = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__callStatic", 1); + zend_check_magic_method_attr(fn_flags, ce, "__callStatic"); ce->__callstatic = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__get", 0); + zend_check_magic_method_attr(fn_flags, ce, "__get"); ce->__get = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__set", 0); + zend_check_magic_method_attr(fn_flags, ce, "__set"); ce->__set = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__unset", 0); + zend_check_magic_method_attr(fn_flags, ce, "__unset"); ce->__unset = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__isset", 0); + zend_check_magic_method_attr(fn_flags, ce, "__isset"); ce->__isset = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__toString", 0); + zend_check_magic_method_attr(fn_flags, ce, "__toString"); ce->__tostring = (zend_function *) op_array; add_stringable_interface(ce); } else if (zend_string_equals_literal(lcname, ZEND_INVOKE_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__invoke", 0); + zend_check_magic_method_attr(fn_flags, ce, "__invoke"); } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__debugInfo", 0); + zend_check_magic_method_attr(fn_flags, ce, "__debugInfo"); ce->__debugInfo = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, "__serialize")) { - zend_check_magic_method_attr(fn_flags, ce, "__serialize", 0); + zend_check_magic_method_attr(fn_flags, ce, "__serialize"); ce->__serialize = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, "__unserialize")) { - zend_check_magic_method_attr(fn_flags, ce, "__unserialize", 0); + zend_check_magic_method_attr(fn_flags, ce, "__unserialize"); ce->__unserialize = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, "__set_state")) { - zend_check_magic_method_attr(fn_flags, ce, "__set_state", 1); + zend_check_magic_method_attr(fn_flags, ce, "__set_state"); } return lcname; @@ -7173,10 +7161,6 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) / if (ce->constructor) { ce->constructor->common.fn_flags |= ZEND_ACC_CTOR; - if (ce->constructor->common.fn_flags & ZEND_ACC_STATIC) { - zend_error_noreturn(E_COMPILE_ERROR, "Constructor %s::%s() cannot be static", - ZSTR_VAL(ce->name), ZSTR_VAL(ce->constructor->common.function_name)); - } if (ce->constructor->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { zend_error_noreturn(E_COMPILE_ERROR, "Constructor %s::%s() cannot declare a return type", @@ -7184,20 +7168,14 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) / } } if (ce->destructor) { - if (ce->destructor->common.fn_flags & ZEND_ACC_STATIC) { - zend_error_noreturn(E_COMPILE_ERROR, "Destructor %s::%s() cannot be static", - ZSTR_VAL(ce->name), ZSTR_VAL(ce->destructor->common.function_name)); - } else if (ce->destructor->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { + if (ce->destructor->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { zend_error_noreturn(E_COMPILE_ERROR, "Destructor %s::%s() cannot declare a return type", ZSTR_VAL(ce->name), ZSTR_VAL(ce->destructor->common.function_name)); } } if (ce->clone) { - if (ce->clone->common.fn_flags & ZEND_ACC_STATIC) { - zend_error_noreturn(E_COMPILE_ERROR, "Clone method %s::%s() cannot be static", - ZSTR_VAL(ce->name), ZSTR_VAL(ce->clone->common.function_name)); - } else if (ce->clone->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { + if (ce->clone->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { zend_error_noreturn(E_COMPILE_ERROR, "Clone method %s::%s() cannot declare a return type", ZSTR_VAL(ce->name), ZSTR_VAL(ce->clone->common.function_name)); diff --git a/tests/classes/__call_007.phpt b/tests/classes/__call_007.phpt deleted file mode 100644 index e2edb8a5304f7..0000000000000 --- a/tests/classes/__call_007.phpt +++ /dev/null @@ -1,76 +0,0 @@ ---TEST-- -Ensure exceptions are handled properly when thrown in a statically declared __call. ---FILE-- - Invoke __call via simple method call.\n"; -try { - $a->unknown(); -} catch (Exception $e) { - echo "Exception caught OK; continuing.\n"; -} - -echo "\n\n---> Invoke __call via scope resolution operator within instance.\n"; -try { - $a->test(); -} catch (Exception $e) { - echo "Exception caught OK; continuing.\n"; -} - -echo "\n\n---> Invoke __call via scope resolution operator within child instance.\n"; -$b = new B(); -try { - $b->test(); -} catch (Exception $e) { - echo "Exception caught OK; continuing.\n"; -} - -echo "\n\n---> Invoke __call via callback.\n"; -try { - call_user_func(array($b, 'unknownCallback'), 1,2,3); -} catch (Exception $e) { - echo "Exception caught OK; continuing.\n"; -} -?> ---EXPECTF-- -Warning: The magic method A::__call() cannot be static in %s on line 3 ----> Invoke __call via simple method call. -object(A)#1 (0) { -} -Exception caught OK; continuing. - - ----> Invoke __call via scope resolution operator within instance. -object(A)#1 (0) { -} -Exception caught OK; continuing. - - ----> Invoke __call via scope resolution operator within child instance. -object(B)#2 (0) { -} -Exception caught OK; continuing. - - ----> Invoke __call via callback. -object(B)#2 (0) { -} -Exception caught OK; continuing. From 312fe2bdce066576f692d73c5a986213319cb605 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 20 Jul 2020 10:51:48 +0200 Subject: [PATCH 3/9] Unify magic method return type checks --- Zend/tests/return_types/014.phpt | 2 +- Zend/tests/return_types/018.phpt | 2 +- Zend/zend_API.c | 23 ++++++++++++----------- Zend/zend_compile.c | 19 ------------------- 4 files changed, 14 insertions(+), 32 deletions(-) diff --git a/Zend/tests/return_types/014.phpt b/Zend/tests/return_types/014.phpt index 25bd79fb814b7..00f5e288bd5c4 100644 --- a/Zend/tests/return_types/014.phpt +++ b/Zend/tests/return_types/014.phpt @@ -7,4 +7,4 @@ class Foo { function __construct() : Foo {} } --EXPECTF-- -Fatal error: Constructor %s::%s() cannot declare a return type in %s on line %d +Fatal error: Method Foo::__construct() cannot declare a return type in %s on line %d diff --git a/Zend/tests/return_types/018.phpt b/Zend/tests/return_types/018.phpt index 8c745a1f65131..6c2f48c8c902c 100644 --- a/Zend/tests/return_types/018.phpt +++ b/Zend/tests/return_types/018.phpt @@ -7,4 +7,4 @@ class Foo { function __destruct() : Foo {} } --EXPECTF-- -Fatal error: Destructor %s::%s() cannot declare a return type in %s on line %d +Fatal error: Method Foo::__destruct() cannot declare a return type in %s on line %d diff --git a/Zend/zend_API.c b/Zend/zend_API.c index a697c453ce264..734c792fd5999 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2049,6 +2049,15 @@ static void zend_check_magic_method_static( } } +static void zend_check_magic_method_no_return_type( + const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) +{ + if (fptr->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { + zend_error_noreturn(error_type, "Method %s::%s() cannot declare a return type", + ZSTR_VAL(ce->name), name); + } +} + ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce, const zend_function *fptr, zend_string *lcname, int error_type) /* {{{ */ { if (ZSTR_VAL(fptr->common.function_name)[0] != '_' @@ -2058,12 +2067,15 @@ ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce, if (zend_string_equals_literal(lcname, ZEND_CONSTRUCTOR_FUNC_NAME)) { zend_check_magic_method_non_static("__construct", ce, fptr, error_type); + zend_check_magic_method_no_return_type("__construct", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_DESTRUCTOR_FUNC_NAME)) { zend_check_magic_method_args(0, "__destruct", ce, fptr, error_type); zend_check_magic_method_non_static("__destruct", ce, fptr, error_type); + zend_check_magic_method_no_return_type("__destruct", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CLONE_FUNC_NAME)) { zend_check_magic_method_args(0, "__clone", ce, fptr, error_type); zend_check_magic_method_non_static("__clone", ce, fptr, error_type); + zend_check_magic_method_no_return_type("__clone", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) { zend_check_magic_method_args(1, "__get", ce, fptr, error_type); zend_check_magic_method_non_static("__get", ce, fptr, error_type); @@ -2359,17 +2371,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio if (ctor) { ctor->common.fn_flags |= ZEND_ACC_CTOR; } - if (ctor && (ctor->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE)) { - zend_error_noreturn(E_CORE_ERROR, "Constructor %s::%s() cannot declare a return type", ZSTR_VAL(scope->name), ZSTR_VAL(ctor->common.function_name)); - } - - if (dtor && (dtor->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE)) { - zend_error_noreturn(E_CORE_ERROR, "Destructor %s::%s() cannot declare a return type", ZSTR_VAL(scope->name), ZSTR_VAL(dtor->common.function_name)); - } - - if (clone && (clone->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE)) { - zend_error_noreturn(E_CORE_ERROR, "%s::%s() cannot declare a return type", ZSTR_VAL(scope->name), ZSTR_VAL(clone->common.function_name)); - } efree((char*)lc_class_name); } return SUCCESS; diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index dbb37bbadbe6e..c2d0a117f23ef 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -7161,25 +7161,6 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) / if (ce->constructor) { ce->constructor->common.fn_flags |= ZEND_ACC_CTOR; - if (ce->constructor->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { - zend_error_noreturn(E_COMPILE_ERROR, - "Constructor %s::%s() cannot declare a return type", - ZSTR_VAL(ce->name), ZSTR_VAL(ce->constructor->common.function_name)); - } - } - if (ce->destructor) { - if (ce->destructor->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { - zend_error_noreturn(E_COMPILE_ERROR, - "Destructor %s::%s() cannot declare a return type", - ZSTR_VAL(ce->name), ZSTR_VAL(ce->destructor->common.function_name)); - } - } - if (ce->clone) { - if (ce->clone->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { - zend_error_noreturn(E_COMPILE_ERROR, - "Clone method %s::%s() cannot declare a return type", - ZSTR_VAL(ce->name), ZSTR_VAL(ce->clone->common.function_name)); - } } if ((ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) == ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) { From 015fc638fb2a1f3082f1ef15684120def377f64f Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 20 Jul 2020 10:54:59 +0200 Subject: [PATCH 4/9] Fix tests I missed before --- ext/date/tests/ExtendDateTime.phpt | 2 +- ext/reflection/tests/ReflectionMethod_getModifiers_basic.phpt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/date/tests/ExtendDateTime.phpt b/ext/date/tests/ExtendDateTime.phpt index 212d44ab82215..70ed2793e6efe 100644 --- a/ext/date/tests/ExtendDateTime.phpt +++ b/ext/date/tests/ExtendDateTime.phpt @@ -9,4 +9,4 @@ class MyDateTime extends DateTime { } ?> --EXPECTF-- -Fatal error: Declaration of MyDateTime::__set_state() must be compatible with DateTime::__set_state(array $array) in %s on line %d +Fatal error: Method MyDateTime::__set_state() must take exactly 1 argument in %s on line %d diff --git a/ext/reflection/tests/ReflectionMethod_getModifiers_basic.phpt b/ext/reflection/tests/ReflectionMethod_getModifiers_basic.phpt index e5e967e28e82a..6e0a0645c5441 100644 --- a/ext/reflection/tests/ReflectionMethod_getModifiers_basic.phpt +++ b/ext/reflection/tests/ReflectionMethod_getModifiers_basic.phpt @@ -52,7 +52,7 @@ class TestClass public function __wakeup() {} - public static function __set_state() {} + public static function __set_state($a) {} public function __autoload() {} } From 9f0213fff3ee9455397544acc53d3ee60c46a5fb Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Mon, 20 Jul 2020 11:57:19 +0300 Subject: [PATCH 5/9] Remove old code (BP_VAR_RW warning) --- ext/opcache/jit/zend_jit_disasm_x86.c | 1 - ext/opcache/jit/zend_jit_helpers.c | 6 ------ ext/opcache/jit/zend_jit_x86.dasc | 15 +++------------ 3 files changed, 3 insertions(+), 19 deletions(-) diff --git a/ext/opcache/jit/zend_jit_disasm_x86.c b/ext/opcache/jit/zend_jit_disasm_x86.c index 47292f675dff8..2e73648257ee3 100644 --- a/ext/opcache/jit/zend_jit_disasm_x86.c +++ b/ext/opcache/jit/zend_jit_disasm_x86.c @@ -409,7 +409,6 @@ static int zend_jit_disasm_init(void) REGISTER_HELPER(zend_jit_hash_lookup_w); REGISTER_HELPER(zend_jit_symtable_lookup_rw); REGISTER_HELPER(zend_jit_symtable_lookup_w); - REGISTER_HELPER(zend_jit_fetch_dimension_rw_long_helper); REGISTER_HELPER(zend_jit_undefined_op_helper); REGISTER_HELPER(zend_jit_fetch_dim_r_helper); REGISTER_HELPER(zend_jit_fetch_dim_is_helper); diff --git a/ext/opcache/jit/zend_jit_helpers.c b/ext/opcache/jit/zend_jit_helpers.c index c282dea4e236d..e42fd196aa93f 100644 --- a/ext/opcache/jit/zend_jit_helpers.c +++ b/ext/opcache/jit/zend_jit_helpers.c @@ -810,12 +810,6 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_obj_is_helper(zval *container, zval } } -static zval* ZEND_FASTCALL zend_jit_fetch_dimension_rw_long_helper(HashTable *ht, zend_long hval) -{ - zend_error(E_NOTICE,"Undefined array key " ZEND_LONG_FMT, hval); - return zend_hash_index_update(ht, hval, &EG(uninitialized_zval)); -} - static zend_never_inline zend_long zend_check_string_offset(zval *dim, int type) { zend_long offset; diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index 6a2fa0a7c8f46..f2a5c99321c68 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -4823,7 +4823,7 @@ static int zend_jit_fetch_dimension_address_inner(dasm_State **Dst, const zend_o zend_jit_addr res_addr = ZEND_ADDR_MEM_ZVAL(ZREG_FP, opline->result.var); if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE - && (type == BP_VAR_R || type == BP_VAR_RW) + && type == BP_VAR_R && !exit_addr) { int32_t exit_point = zend_jit_trace_get_exit_point(opline, opline, NULL, ZEND_JIT_EXIT_TO_VM); exit_addr = zend_jit_trace_get_exit_addr(exit_point); @@ -4865,7 +4865,7 @@ static int zend_jit_fetch_dimension_address_inner(dasm_State **Dst, const zend_o } else { | jbe >9 // NOT_FOUND } - } else if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE && (type == BP_VAR_R || type == BP_VAR_RW)) { + } else if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE && type == BP_VAR_R) { | jbe &exit_addr } else if (type == BP_VAR_IS && not_found_exit_addr) { | jbe ¬_found_exit_addr @@ -4904,7 +4904,7 @@ static int zend_jit_fetch_dimension_address_inner(dasm_State **Dst, const zend_o } else { | jbe >9 // NOT_FOUND } - } else if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE && (type == BP_VAR_R || type == BP_VAR_RW)) { + } else if (JIT_G(trigger) == ZEND_JIT_ON_HOT_TRACE && type == BP_VAR_R) { | jbe &exit_addr } else if (type == BP_VAR_IS && not_found_exit_addr) { | jbe ¬_found_exit_addr @@ -5009,16 +5009,7 @@ static int zend_jit_fetch_dimension_address_inner(dasm_State **Dst, const zend_o break; case BP_VAR_RW: |2: - if (JIT_G(trigger) != ZEND_JIT_ON_HOT_TRACE) { - | SAVE_VALID_OPLINE opline, r0 - | // zend_error(E_NOTICE,"Undefined arary offset " ZEND_LONG_FMT, hval); - | //retval = zend_hash_index_update(ht, hval, &EG(uninitialized_zval)); - | EXT_CALL zend_jit_fetch_dimension_rw_long_helper, r0 - } if (op1_info & MAY_BE_ARRAY_KEY_LONG) { - if (JIT_G(trigger) != ZEND_JIT_ON_HOT_TRACE) { - | jmp >8 - } |4: | SAVE_VALID_OPLINE opline, r0 | EXT_CALL zend_jit_hash_index_lookup_rw, r0 From 149029b9d6411c8d8391d125891ac7f770ba277a Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 20 Jul 2020 11:05:33 +0200 Subject: [PATCH 6/9] Unify magic method visibility check This was missing entirely for the internal function case. --- Zend/tests/magic_methods_007.phpt | 2 -- Zend/tests/magic_methods_010.phpt | 2 -- Zend/zend_API.c | 29 +++++++++++++++++++++++------ Zend/zend_compile.c | 30 ++---------------------------- 4 files changed, 25 insertions(+), 38 deletions(-) diff --git a/Zend/tests/magic_methods_007.phpt b/Zend/tests/magic_methods_007.phpt index 86642b4ecb724..5e0379060b4ca 100644 --- a/Zend/tests/magic_methods_007.phpt +++ b/Zend/tests/magic_methods_007.phpt @@ -9,6 +9,4 @@ abstract class b { ?> --EXPECTF-- -Warning: The magic method b::__set() must have public visibility in %s on line %d - Fatal error: Method b::__set() must take exactly 2 arguments in %s on line %d diff --git a/Zend/tests/magic_methods_010.phpt b/Zend/tests/magic_methods_010.phpt index fda1cc1e10a22..3562189c344a3 100644 --- a/Zend/tests/magic_methods_010.phpt +++ b/Zend/tests/magic_methods_010.phpt @@ -10,6 +10,4 @@ class a { ?> --EXPECTF-- -Warning: The magic method a::__toString() must have public visibility in %s on line %d - Fatal error: Method a::__toString() cannot take arguments in %s on line %d diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 734c792fd5999..14f7cd695a8a0 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2049,6 +2049,16 @@ static void zend_check_magic_method_static( } } +static void zend_check_magic_method_public( + const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) +{ + // TODO: Remove this warning after adding proper visibility handling. + if (!(fptr->common.fn_flags & ZEND_ACC_PUBLIC)) { + zend_error(E_WARNING, "The magic method %s::%s() must have public visibility", + ZSTR_VAL(ce->name), name); + } +} + static void zend_check_magic_method_no_return_type( const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) { @@ -2079,38 +2089,50 @@ ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce, } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) { zend_check_magic_method_args(1, "__get", ce, fptr, error_type); zend_check_magic_method_non_static("__get", ce, fptr, error_type); + zend_check_magic_method_public("__get", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) { zend_check_magic_method_args(2, "__set", ce, fptr, error_type); zend_check_magic_method_non_static("__set", ce, fptr, error_type); + zend_check_magic_method_public("__set", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) { zend_check_magic_method_args(1, "__unset", ce, fptr, error_type); zend_check_magic_method_non_static("__unset", ce, fptr, error_type); + zend_check_magic_method_public("__unset", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) { zend_check_magic_method_args(1, "__isset", ce, fptr, error_type); zend_check_magic_method_non_static("__isset", ce, fptr, error_type); + zend_check_magic_method_public("__isset", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) { zend_check_magic_method_args(2, "__call", ce, fptr, error_type); zend_check_magic_method_non_static("__call", ce, fptr, error_type); + zend_check_magic_method_public("__call", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) { zend_check_magic_method_args(2, "__callStatic", ce, fptr, error_type); zend_check_magic_method_static("__callStatic", ce, fptr, error_type); + zend_check_magic_method_public("__callStatic", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) { zend_check_magic_method_args(0, "__toString", ce, fptr, error_type); zend_check_magic_method_non_static("__toString", ce, fptr, error_type); + zend_check_magic_method_public("__toString", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) { zend_check_magic_method_args(0, "__debugInfo", ce, fptr, error_type); zend_check_magic_method_non_static("__debugInfo", ce, fptr, error_type); + zend_check_magic_method_public("__debugInfo", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__serialize")) { zend_check_magic_method_args(0, "__serialize", ce, fptr, error_type); zend_check_magic_method_non_static("__serialize", ce, fptr, error_type); + zend_check_magic_method_public("__serialize", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__unserialize")) { zend_check_magic_method_args(1, "__unserialize", ce, fptr, error_type); zend_check_magic_method_non_static("__unserialize", ce, fptr, error_type); + zend_check_magic_method_public("__unserialize", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__set_state")) { zend_check_magic_method_args(1, "__set_state", ce, fptr, error_type); zend_check_magic_method_static("__set_state", ce, fptr, error_type); + zend_check_magic_method_public("__set_state", ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__invoke")) { zend_check_magic_method_non_static("__invoke", ce, fptr, error_type); + zend_check_magic_method_public("__invoke", ce, fptr, error_type); } } /* }}} */ @@ -2294,11 +2316,9 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio reg_function = NULL; } else if (zend_string_equals_literal(lowercase_name, ZEND_CONSTRUCTOR_FUNC_NAME)) { ctor = reg_function; + ctor->common.fn_flags |= ZEND_ACC_CTOR; } else if (zend_string_equals_literal(lowercase_name, ZEND_DESTRUCTOR_FUNC_NAME)) { dtor = reg_function; - if (internal_function->num_args) { - zend_error(error_type, "Destructor %s::%s() cannot take arguments", ZSTR_VAL(scope->name), ptr->fname); - } } else if (zend_string_equals_literal(lowercase_name, ZEND_CLONE_FUNC_NAME)) { clone = reg_function; } else if (zend_string_equals_literal(lowercase_name, ZEND_CALL_FUNC_NAME)) { @@ -2368,9 +2388,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio scope->__debugInfo = __debugInfo; scope->__serialize = __serialize; scope->__unserialize = __unserialize; - if (ctor) { - ctor->common.fn_flags |= ZEND_ACC_CTOR; - } efree((char*)lc_class_name); } return SUCCESS; diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index c2d0a117f23ef..cabc1087a7fcc 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -6443,16 +6443,6 @@ static void zend_compile_implicit_closure_uses(closure_info *info) ZEND_HASH_FOREACH_END(); } -static void zend_check_magic_method_attr(uint32_t attr, zend_class_entry *ce, const char* method) /* {{{ */ -{ - if (!(attr & ZEND_ACC_PUBLIC)) { - zend_error(E_WARNING, - "The magic method %s::%s() must have public visibility", - ZSTR_VAL(ce->name), method); - } -} -/* }}} */ - static void add_stringable_interface(zend_class_entry *ce) { for (uint32_t i = 0; i < ce->num_interfaces; i++) { if (zend_string_equals_literal(ce->interface_names[i].lc_name, "stringable")) { @@ -6523,49 +6513,36 @@ zend_string *zend_begin_method_decl(zend_op_array *op_array, zend_string *name, /* pass */ } else if (zend_string_equals_literal(lcname, ZEND_CONSTRUCTOR_FUNC_NAME)) { ce->constructor = (zend_function *) op_array; + ce->constructor->common.fn_flags |= ZEND_ACC_CTOR; } else if (zend_string_equals_literal(lcname, ZEND_DESTRUCTOR_FUNC_NAME)) { ce->destructor = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_CLONE_FUNC_NAME)) { ce->clone = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__call"); ce->__call = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__callStatic"); ce->__callstatic = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__get"); ce->__get = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__set"); ce->__set = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__unset"); ce->__unset = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__isset"); ce->__isset = (zend_function *) op_array; ce->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__toString"); ce->__tostring = (zend_function *) op_array; add_stringable_interface(ce); - } else if (zend_string_equals_literal(lcname, ZEND_INVOKE_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__invoke"); } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) { - zend_check_magic_method_attr(fn_flags, ce, "__debugInfo"); ce->__debugInfo = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, "__serialize")) { - zend_check_magic_method_attr(fn_flags, ce, "__serialize"); ce->__serialize = (zend_function *) op_array; } else if (zend_string_equals_literal(lcname, "__unserialize")) { - zend_check_magic_method_attr(fn_flags, ce, "__unserialize"); ce->__unserialize = (zend_function *) op_array; - } else if (zend_string_equals_literal(lcname, "__set_state")) { - zend_check_magic_method_attr(fn_flags, ce, "__set_state"); } return lcname; @@ -6740,6 +6717,7 @@ void zend_compile_func_decl(znode *result, zend_ast *ast, zend_bool toplevel) /* zend_compile_stmt(stmt_ast); if (is_method) { + CG(zend_lineno) = decl->start_lineno; zend_check_magic_method_implementation( CG(active_class_entry), (zend_function *) op_array, method_lcname, E_COMPILE_ERROR); zend_string_release_ex(method_lcname, 0); @@ -7159,10 +7137,6 @@ void zend_compile_class_decl(znode *result, zend_ast *ast, zend_bool toplevel) / /* Reset lineno for final opcodes and errors */ CG(zend_lineno) = ast->lineno; - if (ce->constructor) { - ce->constructor->common.fn_flags |= ZEND_ACC_CTOR; - } - if ((ce->ce_flags & (ZEND_ACC_IMPLICIT_ABSTRACT_CLASS|ZEND_ACC_INTERFACE|ZEND_ACC_TRAIT|ZEND_ACC_EXPLICIT_ABSTRACT_CLASS)) == ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) { zend_verify_abstract_class(ce); } From 19de727e04ed5be3d9a386f1b7b49f31933e527c Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 20 Jul 2020 11:11:21 +0200 Subject: [PATCH 7/9] Report magic method names as written Report the name the way the user has written it, the same way we always do. --- Zend/tests/magic_methods_006.phpt | 2 +- Zend/tests/magic_methods_009.phpt | 2 +- Zend/zend_API.c | 115 +++++++++++++++--------------- 3 files changed, 60 insertions(+), 59 deletions(-) diff --git a/Zend/tests/magic_methods_006.phpt b/Zend/tests/magic_methods_006.phpt index b07764ec6f76a..0a83a448ef51d 100644 --- a/Zend/tests/magic_methods_006.phpt +++ b/Zend/tests/magic_methods_006.phpt @@ -9,4 +9,4 @@ interface a { ?> --EXPECTF-- -Fatal error: Method a::__callStatic() must be static in %s on line %d +Fatal error: Method a::__callstatic() must be static in %s on line %d diff --git a/Zend/tests/magic_methods_009.phpt b/Zend/tests/magic_methods_009.phpt index 97b54d2412de2..4ed33225ff4b8 100644 --- a/Zend/tests/magic_methods_009.phpt +++ b/Zend/tests/magic_methods_009.phpt @@ -10,4 +10,4 @@ class a { ?> --EXPECTF-- -Warning: The magic method a::__callStatic() must have public visibility in %s on line %d +Warning: The magic method a::__callstatic() must have public visibility in %s on line %d diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 14f7cd695a8a0..99b9100f61e04 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2008,63 +2008,64 @@ ZEND_API zend_module_entry* zend_register_internal_module(zend_module_entry *mod /* }}} */ static void zend_check_magic_method_args( - uint32_t num_args, const char *name, - const zend_class_entry *ce, const zend_function *fptr, int error_type) + uint32_t num_args, const zend_class_entry *ce, const zend_function *fptr, int error_type) { if (fptr->common.num_args != num_args) { if (num_args == 0) { zend_error(error_type, "Method %s::%s() cannot take arguments", - ZSTR_VAL(ce->name), name); + ZSTR_VAL(ce->name), ZSTR_VAL(fptr->common.function_name)); } else if (num_args == 1) { zend_error(error_type, "Method %s::%s() must take exactly 1 argument", - ZSTR_VAL(ce->name), name); + ZSTR_VAL(ce->name), ZSTR_VAL(fptr->common.function_name)); } else { zend_error(error_type, "Method %s::%s() must take exactly %" PRIu32 " arguments", - ZSTR_VAL(ce->name), name, num_args); + ZSTR_VAL(ce->name), ZSTR_VAL(fptr->common.function_name), num_args); } return; } for (uint32_t i = 0; i < num_args; i++) { if (QUICK_ARG_SHOULD_BE_SENT_BY_REF(fptr, i + 1)) { zend_error(error_type, "Method %s::%s() cannot take arguments by reference", - ZSTR_VAL(ce->name), name); + ZSTR_VAL(ce->name), ZSTR_VAL(fptr->common.function_name)); return; } } } static void zend_check_magic_method_non_static( - const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) + const zend_class_entry *ce, const zend_function *fptr, int error_type) { if (fptr->common.fn_flags & ZEND_ACC_STATIC) { - zend_error(error_type, "Method %s::%s() cannot be static", ZSTR_VAL(ce->name), name); + zend_error(error_type, "Method %s::%s() cannot be static", + ZSTR_VAL(ce->name), ZSTR_VAL(fptr->common.function_name)); } } static void zend_check_magic_method_static( - const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) + const zend_class_entry *ce, const zend_function *fptr, int error_type) { if (!(fptr->common.fn_flags & ZEND_ACC_STATIC)) { - zend_error(error_type, "Method %s::%s() must be static", ZSTR_VAL(ce->name), name); + zend_error(error_type, "Method %s::%s() must be static", + ZSTR_VAL(ce->name), ZSTR_VAL(fptr->common.function_name)); } } static void zend_check_magic_method_public( - const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) + const zend_class_entry *ce, const zend_function *fptr, int error_type) { // TODO: Remove this warning after adding proper visibility handling. if (!(fptr->common.fn_flags & ZEND_ACC_PUBLIC)) { zend_error(E_WARNING, "The magic method %s::%s() must have public visibility", - ZSTR_VAL(ce->name), name); + ZSTR_VAL(ce->name), ZSTR_VAL(fptr->common.function_name)); } } static void zend_check_magic_method_no_return_type( - const char *name, const zend_class_entry *ce, const zend_function *fptr, int error_type) + const zend_class_entry *ce, const zend_function *fptr, int error_type) { if (fptr->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { zend_error_noreturn(error_type, "Method %s::%s() cannot declare a return type", - ZSTR_VAL(ce->name), name); + ZSTR_VAL(ce->name), ZSTR_VAL(fptr->common.function_name)); } } @@ -2076,63 +2077,63 @@ ZEND_API void zend_check_magic_method_implementation(const zend_class_entry *ce, } if (zend_string_equals_literal(lcname, ZEND_CONSTRUCTOR_FUNC_NAME)) { - zend_check_magic_method_non_static("__construct", ce, fptr, error_type); - zend_check_magic_method_no_return_type("__construct", ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_no_return_type(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_DESTRUCTOR_FUNC_NAME)) { - zend_check_magic_method_args(0, "__destruct", ce, fptr, error_type); - zend_check_magic_method_non_static("__destruct", ce, fptr, error_type); - zend_check_magic_method_no_return_type("__destruct", ce, fptr, error_type); + zend_check_magic_method_args(0, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_no_return_type(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CLONE_FUNC_NAME)) { - zend_check_magic_method_args(0, "__clone", ce, fptr, error_type); - zend_check_magic_method_non_static("__clone", ce, fptr, error_type); - zend_check_magic_method_no_return_type("__clone", ce, fptr, error_type); + zend_check_magic_method_args(0, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_no_return_type(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_GET_FUNC_NAME)) { - zend_check_magic_method_args(1, "__get", ce, fptr, error_type); - zend_check_magic_method_non_static("__get", ce, fptr, error_type); - zend_check_magic_method_public("__get", ce, fptr, error_type); + zend_check_magic_method_args(1, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_SET_FUNC_NAME)) { - zend_check_magic_method_args(2, "__set", ce, fptr, error_type); - zend_check_magic_method_non_static("__set", ce, fptr, error_type); - zend_check_magic_method_public("__set", ce, fptr, error_type); + zend_check_magic_method_args(2, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_UNSET_FUNC_NAME)) { - zend_check_magic_method_args(1, "__unset", ce, fptr, error_type); - zend_check_magic_method_non_static("__unset", ce, fptr, error_type); - zend_check_magic_method_public("__unset", ce, fptr, error_type); + zend_check_magic_method_args(1, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_ISSET_FUNC_NAME)) { - zend_check_magic_method_args(1, "__isset", ce, fptr, error_type); - zend_check_magic_method_non_static("__isset", ce, fptr, error_type); - zend_check_magic_method_public("__isset", ce, fptr, error_type); + zend_check_magic_method_args(1, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CALL_FUNC_NAME)) { - zend_check_magic_method_args(2, "__call", ce, fptr, error_type); - zend_check_magic_method_non_static("__call", ce, fptr, error_type); - zend_check_magic_method_public("__call", ce, fptr, error_type); + zend_check_magic_method_args(2, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_CALLSTATIC_FUNC_NAME)) { - zend_check_magic_method_args(2, "__callStatic", ce, fptr, error_type); - zend_check_magic_method_static("__callStatic", ce, fptr, error_type); - zend_check_magic_method_public("__callStatic", ce, fptr, error_type); + zend_check_magic_method_args(2, ce, fptr, error_type); + zend_check_magic_method_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_TOSTRING_FUNC_NAME)) { - zend_check_magic_method_args(0, "__toString", ce, fptr, error_type); - zend_check_magic_method_non_static("__toString", ce, fptr, error_type); - zend_check_magic_method_public("__toString", ce, fptr, error_type); + zend_check_magic_method_args(0, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) { - zend_check_magic_method_args(0, "__debugInfo", ce, fptr, error_type); - zend_check_magic_method_non_static("__debugInfo", ce, fptr, error_type); - zend_check_magic_method_public("__debugInfo", ce, fptr, error_type); + zend_check_magic_method_args(0, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__serialize")) { - zend_check_magic_method_args(0, "__serialize", ce, fptr, error_type); - zend_check_magic_method_non_static("__serialize", ce, fptr, error_type); - zend_check_magic_method_public("__serialize", ce, fptr, error_type); + zend_check_magic_method_args(0, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__unserialize")) { - zend_check_magic_method_args(1, "__unserialize", ce, fptr, error_type); - zend_check_magic_method_non_static("__unserialize", ce, fptr, error_type); - zend_check_magic_method_public("__unserialize", ce, fptr, error_type); + zend_check_magic_method_args(1, ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__set_state")) { - zend_check_magic_method_args(1, "__set_state", ce, fptr, error_type); - zend_check_magic_method_static("__set_state", ce, fptr, error_type); - zend_check_magic_method_public("__set_state", ce, fptr, error_type); + zend_check_magic_method_args(1, ce, fptr, error_type); + zend_check_magic_method_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } else if (zend_string_equals_literal(lcname, "__invoke")) { - zend_check_magic_method_non_static("__invoke", ce, fptr, error_type); - zend_check_magic_method_public("__invoke", ce, fptr, error_type); + zend_check_magic_method_non_static(ce, fptr, error_type); + zend_check_magic_method_public(ce, fptr, error_type); } } /* }}} */ From 91e5452b956628b4ea7d825e20264a82a8fa551d Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 20 Jul 2020 11:15:58 +0200 Subject: [PATCH 8/9] Remove unused lc_class_name variable This is probably a leftover from "old style constructor" support. --- Zend/zend_API.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 99b9100f61e04..accf98c0beb5c 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2150,8 +2150,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio zend_function *ctor = NULL, *dtor = NULL, *clone = NULL, *__get = NULL, *__set = NULL, *__unset = NULL, *__isset = NULL, *__call = NULL, *__callstatic = NULL, *__tostring = NULL, *__debugInfo = NULL, *__serialize = NULL, *__unserialize = NULL; zend_string *lowercase_name; size_t fname_len; - const char *lc_class_name = NULL; - size_t class_name_len = 0; if (type==MODULE_PERSISTENT) { error_type = E_CORE_WARNING; @@ -2166,17 +2164,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio internal_function->module = EG(current_module); memset(internal_function->reserved, 0, ZEND_MAX_RESERVED_RESOURCES * sizeof(void*)); - if (scope) { - class_name_len = ZSTR_LEN(scope->name); - if ((lc_class_name = zend_memrchr(ZSTR_VAL(scope->name), '\\', class_name_len))) { - ++lc_class_name; - class_name_len -= (lc_class_name - ZSTR_VAL(scope->name)); - lc_class_name = zend_str_tolower_dup(lc_class_name, class_name_len); - } else { - lc_class_name = zend_str_tolower_dup(ZSTR_VAL(scope->name), class_name_len); - } - } - while (ptr->fname) { fname_len = strlen(ptr->fname); internal_function->handler = ptr->handler; @@ -2250,14 +2237,10 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio } } else { if (scope && (scope->ce_flags & ZEND_ACC_INTERFACE)) { - efree((char*)lc_class_name); zend_error(error_type, "Interface %s cannot contain non abstract method %s()", ZSTR_VAL(scope->name), ptr->fname); return FAILURE; } if (!internal_function->handler) { - if (scope) { - efree((char*)lc_class_name); - } zend_error(error_type, "Method %s%s%s() cannot be a NULL function", scope ? ZSTR_VAL(scope->name) : "", scope ? "::" : "", ptr->fname); zend_unregister_functions(functions, count, target_function_table); return FAILURE; @@ -2359,9 +2342,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio zend_string_release(lowercase_name); } if (unload) { /* before unloading, display all remaining bad function in the module */ - if (scope) { - efree((char*)lc_class_name); - } while (ptr->fname) { fname_len = strlen(ptr->fname); lowercase_name = zend_string_alloc(fname_len, 0); @@ -2389,7 +2369,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio scope->__debugInfo = __debugInfo; scope->__serialize = __serialize; scope->__unserialize = __unserialize; - efree((char*)lc_class_name); } return SUCCESS; } From efce3694b7cea12e3640a47250544fb241d67611 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 20 Jul 2020 11:22:47 +0200 Subject: [PATCH 9/9] Directly assign magic methods Instead of going through intermediary variables. --- Zend/zend_API.c | 44 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index accf98c0beb5c..b1d3c26fd8e11 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2147,7 +2147,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio int count=0, unload=0; HashTable *target_function_table = function_table; int error_type; - zend_function *ctor = NULL, *dtor = NULL, *clone = NULL, *__get = NULL, *__set = NULL, *__unset = NULL, *__isset = NULL, *__call = NULL, *__callstatic = NULL, *__tostring = NULL, *__debugInfo = NULL, *__serialize = NULL, *__unserialize = NULL; zend_string *lowercase_name; size_t fname_len; @@ -2299,36 +2298,36 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio if (ZSTR_VAL(lowercase_name)[0] != '_' || ZSTR_VAL(lowercase_name)[1] != '_') { reg_function = NULL; } else if (zend_string_equals_literal(lowercase_name, ZEND_CONSTRUCTOR_FUNC_NAME)) { - ctor = reg_function; - ctor->common.fn_flags |= ZEND_ACC_CTOR; + scope->constructor = reg_function; + scope->constructor->common.fn_flags |= ZEND_ACC_CTOR; } else if (zend_string_equals_literal(lowercase_name, ZEND_DESTRUCTOR_FUNC_NAME)) { - dtor = reg_function; + scope->destructor = reg_function; } else if (zend_string_equals_literal(lowercase_name, ZEND_CLONE_FUNC_NAME)) { - clone = reg_function; + scope->clone = reg_function; } else if (zend_string_equals_literal(lowercase_name, ZEND_CALL_FUNC_NAME)) { - __call = reg_function; + scope->__call = reg_function; } else if (zend_string_equals_literal(lowercase_name, ZEND_CALLSTATIC_FUNC_NAME)) { - __callstatic = reg_function; + scope->__callstatic = reg_function; } else if (zend_string_equals_literal(lowercase_name, ZEND_TOSTRING_FUNC_NAME)) { - __tostring = reg_function; + scope->__tostring = reg_function; } else if (zend_string_equals_literal(lowercase_name, ZEND_GET_FUNC_NAME)) { - __get = reg_function; + scope->__get = reg_function; scope->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lowercase_name, ZEND_SET_FUNC_NAME)) { - __set = reg_function; + scope->__set = reg_function; scope->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lowercase_name, ZEND_UNSET_FUNC_NAME)) { - __unset = reg_function; + scope->__unset = reg_function; scope->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lowercase_name, ZEND_ISSET_FUNC_NAME)) { - __isset = reg_function; + scope->__isset = reg_function; scope->ce_flags |= ZEND_ACC_USE_GUARDS; } else if (zend_string_equals_literal(lowercase_name, ZEND_DEBUGINFO_FUNC_NAME)) { - __debugInfo = reg_function; + scope->__debugInfo = reg_function; } else if (zend_string_equals_literal(lowercase_name, "__serialize")) { - __serialize = reg_function; + scope->__serialize = reg_function; } else if (zend_string_equals_literal(lowercase_name, "__unserialize")) { - __unserialize = reg_function; + scope->__unserialize = reg_function; } else { reg_function = NULL; } @@ -2355,21 +2354,6 @@ ZEND_API int zend_register_functions(zend_class_entry *scope, const zend_functio zend_unregister_functions(functions, count, target_function_table); return FAILURE; } - if (scope) { - scope->constructor = ctor; - scope->destructor = dtor; - scope->clone = clone; - scope->__call = __call; - scope->__callstatic = __callstatic; - scope->__tostring = __tostring; - scope->__get = __get; - scope->__set = __set; - scope->__unset = __unset; - scope->__isset = __isset; - scope->__debugInfo = __debugInfo; - scope->__serialize = __serialize; - scope->__unserialize = __unserialize; - } return SUCCESS; } /* }}} */