From 2d0e2733c8a0010df9f45693d536531a7a725bdf Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 18 Mar 2021 15:40:48 +0100 Subject: [PATCH] Support prototypes in call graph Even if we don't know the exact method being called, include it in the call graph with the is_prototype flag. In particular, we can still make use of return types from prototype methods, as PHP 8 makes LSP violations a hard error. Most other places are adjusted to skip calls with !is_prototype. Maybe some of them would be fine, but ignoring them is conservative. --- Zend/Optimizer/dfa_pass.c | 1 + Zend/Optimizer/sccp.c | 2 +- Zend/Optimizer/zend_call_graph.c | 11 +++-- Zend/Optimizer/zend_call_graph.h | 1 + Zend/Optimizer/zend_func_info.c | 38 +++++--------- Zend/Optimizer/zend_inference.c | 34 ++++++++++--- Zend/Optimizer/zend_inference.h | 3 ++ Zend/Optimizer/zend_optimizer.c | 1 + ext/opcache/jit/zend_jit_trace.c | 3 +- ext/opcache/jit/zend_jit_x86.dasc | 8 +-- ext/opcache/tests/opt/verify_return_type.phpt | 49 +++++++++++++++++++ 11 files changed, 109 insertions(+), 42 deletions(-) diff --git a/Zend/Optimizer/dfa_pass.c b/Zend/Optimizer/dfa_pass.c index fe06de276b2..3ce28f63e3f 100644 --- a/Zend/Optimizer/dfa_pass.c +++ b/Zend/Optimizer/dfa_pass.c @@ -381,6 +381,7 @@ int zend_dfa_optimize_calls(zend_op_array *op_array, zend_ssa *ssa) zend_op *send_array; zend_op *send_needly; bool strict = 0; + ZEND_ASSERT(!call_info->is_prototype); if (call_info->caller_init_opline->extended_value == 2) { send_array = call_info->caller_call_opline - 1; diff --git a/Zend/Optimizer/sccp.c b/Zend/Optimizer/sccp.c index e4b7531f465..5bcf94d144b 100644 --- a/Zend/Optimizer/sccp.c +++ b/Zend/Optimizer/sccp.c @@ -1771,7 +1771,7 @@ static void sccp_visit_instr(scdf_ctx *scdf, zend_op *opline, zend_ssa_op *ssa_o } /* We're only interested in functions with up to three arguments right now */ - if (call->num_args > 3 || call->send_unpack) { + if (call->num_args > 3 || call->send_unpack || call->is_prototype) { SET_RESULT_BOT(result); break; } diff --git a/Zend/Optimizer/zend_call_graph.c b/Zend/Optimizer/zend_call_graph.c index b0f3247cd72..fb24a8d40fe 100644 --- a/Zend/Optimizer/zend_call_graph.c +++ b/Zend/Optimizer/zend_call_graph.c @@ -65,8 +65,7 @@ ZEND_API int zend_analyze_calls(zend_arena **arena, zend_script *script, uint32_ call_stack[call] = call_info; func = zend_optimizer_get_called_func( script, op_array, opline, &is_prototype); - /* TODO: Support prototypes? */ - if (func && !is_prototype) { + if (func) { call_info = zend_arena_calloc(arena, 1, sizeof(zend_call_info) + (sizeof(zend_send_arg_info) * ((int)opline->extended_value - 1))); call_info->caller_op_array = op_array; call_info->caller_init_opline = opline; @@ -74,6 +73,7 @@ ZEND_API int zend_analyze_calls(zend_arena **arena, zend_script *script, uint32_ call_info->callee_func = func; call_info->num_args = opline->extended_value; call_info->next_callee = func_info->callee_info; + call_info->is_prototype = is_prototype; func_info->callee_info = call_info; if (build_flags & ZEND_CALL_TREE) { @@ -194,7 +194,11 @@ static void zend_analyze_recursion(zend_call_graph *call_graph) op_array = call_graph->op_arrays[i]; func_info = call_graph->func_infos + i; call_info = func_info->caller_info; - while (call_info) { + for (; call_info; call_info = call_info->next_caller) { + if (call_info->is_prototype) { + /* Might be calling an overridden child method and not actually recursive. */ + continue; + } if (call_info->caller_op_array == op_array) { call_info->recursive = 1; func_info->flags |= ZEND_FUNC_RECURSIVE | ZEND_FUNC_RECURSIVE_DIRECTLY; @@ -205,7 +209,6 @@ static void zend_analyze_recursion(zend_call_graph *call_graph) func_info->flags |= ZEND_FUNC_RECURSIVE | ZEND_FUNC_RECURSIVE_INDIRECTLY; } } - call_info = call_info->next_caller; } } diff --git a/Zend/Optimizer/zend_call_graph.h b/Zend/Optimizer/zend_call_graph.h index a456dcfbb83..c0699dc0319 100644 --- a/Zend/Optimizer/zend_call_graph.h +++ b/Zend/Optimizer/zend_call_graph.h @@ -37,6 +37,7 @@ struct _zend_call_info { bool recursive; bool send_unpack; /* Parameters passed by SEND_UNPACK or SEND_ARRAY */ bool named_args; /* Function has named arguments */ + bool is_prototype; /* An overridden child method may be called */ int num_args; zend_send_arg_info arg_info[1]; }; diff --git a/Zend/Optimizer/zend_func_info.c b/Zend/Optimizer/zend_func_info.c index 533f9f85941..22f22f77a78 100644 --- a/Zend/Optimizer/zend_func_info.c +++ b/Zend/Optimizer/zend_func_info.c @@ -878,19 +878,8 @@ ZEND_API uint32_t zend_get_func_info( } #endif - if (callee_func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { - ret = zend_fetch_arg_info_type(NULL, callee_func->common.arg_info - 1, ce); - *ce_is_instanceof = 1; - } else { -#if 0 - fprintf(stderr, "Unknown internal function '%s'\n", func->common.function_name); -#endif - ret = MAY_BE_ANY | MAY_BE_ARRAY_KEY_ANY | MAY_BE_ARRAY_OF_ANY | MAY_BE_ARRAY_OF_REF - | MAY_BE_RC1 | MAY_BE_RCN; - } - if (callee_func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) { - ret |= MAY_BE_REF; - } + ret = zend_get_return_info_from_signature_only( + callee_func, /* script */ NULL, ce, ce_is_instanceof); #if ZEND_DEBUG if (internal_ret) { @@ -919,21 +908,18 @@ ZEND_API uint32_t zend_get_func_info( } #endif } else { - // FIXME: the order of functions matters!!! - zend_func_info *info = ZEND_FUNC_INFO((zend_op_array*)callee_func); - if (info) { - ret = info->return_info.type; - *ce = info->return_info.ce; - *ce_is_instanceof = info->return_info.is_instanceof; + if (!call_info->is_prototype) { + // FIXME: the order of functions matters!!! + zend_func_info *info = ZEND_FUNC_INFO((zend_op_array*)callee_func); + if (info) { + ret = info->return_info.type; + *ce = info->return_info.ce; + *ce_is_instanceof = info->return_info.is_instanceof; + } } if (!ret) { - ret = MAY_BE_ANY | MAY_BE_ARRAY_KEY_ANY | MAY_BE_ARRAY_OF_ANY | MAY_BE_ARRAY_OF_REF - | MAY_BE_RC1 | MAY_BE_RCN; - /* For generators RETURN_REFERENCE refers to the yielded values. */ - if ((callee_func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) - && !(callee_func->common.fn_flags & ZEND_ACC_GENERATOR)) { - ret |= MAY_BE_REF; - } + ret = zend_get_return_info_from_signature_only( + callee_func, /* TODO: script */ NULL, ce, ce_is_instanceof); } } return ret; diff --git a/Zend/Optimizer/zend_inference.c b/Zend/Optimizer/zend_inference.c index 8602e7cb039..8ba67a4daba 100644 --- a/Zend/Optimizer/zend_inference.c +++ b/Zend/Optimizer/zend_inference.c @@ -3979,18 +3979,38 @@ static int is_recursive_tail_call(const zend_op_array *op_array, return 0; } +uint32_t zend_get_return_info_from_signature_only( + const zend_function *func, const zend_script *script, + zend_class_entry **ce, bool *ce_is_instanceof) { + uint32_t type; + if (func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { + zend_arg_info *ret_info = func->common.arg_info - 1; + type = zend_fetch_arg_info_type(script, ret_info, ce); + *ce_is_instanceof = ce != NULL; + } else { + type = MAY_BE_ANY | MAY_BE_ARRAY_KEY_ANY | MAY_BE_ARRAY_OF_ANY | MAY_BE_ARRAY_OF_REF + | MAY_BE_RC1 | MAY_BE_RCN; + *ce = NULL; + *ce_is_instanceof = false; + } + + /* For generators RETURN_REFERENCE refers to the yielded values. */ + if ((func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE) + && !(func->common.fn_flags & ZEND_ACC_GENERATOR)) { + type |= MAY_BE_REF; + } + return type; +} + ZEND_API void zend_init_func_return_info( const zend_op_array *op_array, const zend_script *script, zend_ssa_var_info *ret) { if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) { - zend_arg_info *ret_info = op_array->arg_info - 1; zend_ssa_range tmp_range = {0, 0, 0, 0}; - - ret->type = zend_fetch_arg_info_type(script, ret_info, &ret->ce); - if (op_array->fn_flags & ZEND_ACC_RETURN_REFERENCE) { - ret->type |= MAY_BE_REF; - } - ret->is_instanceof = (ret->ce) ? 1 : 0; + bool is_instanceof = false; + ret->type = zend_get_return_info_from_signature_only( + (zend_function *) op_array, script, &ret->ce, &is_instanceof); + ret->is_instanceof = is_instanceof; ret->range = tmp_range; ret->has_range = 0; } diff --git a/Zend/Optimizer/zend_inference.h b/Zend/Optimizer/zend_inference.h index 83909460127..35af4d8823d 100644 --- a/Zend/Optimizer/zend_inference.h +++ b/Zend/Optimizer/zend_inference.h @@ -269,6 +269,9 @@ ZEND_API uint32_t zend_fetch_arg_info_type( const zend_script *script, zend_arg_info *arg_info, zend_class_entry **pce); ZEND_API void zend_init_func_return_info( const zend_op_array *op_array, const zend_script *script, zend_ssa_var_info *ret); +uint32_t zend_get_return_info_from_signature_only( + const zend_function *func, const zend_script *script, + zend_class_entry **ce, bool *ce_is_instanceof); void zend_func_return_info(const zend_op_array *op_array, const zend_script *script, int recursive, diff --git a/Zend/Optimizer/zend_optimizer.c b/Zend/Optimizer/zend_optimizer.c index cc9971a9f87..d6f27b19e68 100644 --- a/Zend/Optimizer/zend_optimizer.c +++ b/Zend/Optimizer/zend_optimizer.c @@ -1340,6 +1340,7 @@ static void zend_adjust_fcall_stack_size_graph(zend_op_array *op_array) zend_op *opline = call_info->caller_init_opline; if (opline && call_info->callee_func && opline->opcode == ZEND_INIT_FCALL) { + ZEND_ASSERT(!call_info->is_prototype); opline->op1.num = zend_vm_calc_used_stack(opline->extended_value, call_info->callee_func); } call_info = call_info->next_callee; diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index bc7038731a5..80ce45605e8 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -6207,7 +6207,8 @@ done: zend_call_info *call_info = jit_extension->func_info.callee_info; while (call_info) { - if (call_info->caller_init_opline == init_opline) { + if (call_info->caller_init_opline == init_opline + && !call_info->is_prototype) { if (op_array->fn_flags & ZEND_ACC_TRAIT_CLONE) { if (init_opline->opcode == ZEND_INIT_STATIC_METHOD_CALL && init_opline->op1_type != IS_CONST) { diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index ccf3a173dee..6aac9306e40 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -9186,7 +9186,7 @@ static int zend_jit_init_fcall(dasm_State **Dst, const zend_op *opline, uint32_t while (call_info && call_info->caller_init_opline != opline) { call_info = call_info->next_callee; } - if (call_info && call_info->callee_func) { + if (call_info && call_info->callee_func && !call_info->is_prototype) { func = call_info->callee_func; } } @@ -9353,7 +9353,7 @@ static int zend_jit_init_method_call(dasm_State **Dst, while (call_info && call_info->caller_init_opline != opline) { call_info = call_info->next_callee; } - if (call_info && call_info->callee_func) { + if (call_info && call_info->callee_func && !call_info->is_prototype) { func = call_info->callee_func; } } @@ -9678,6 +9678,8 @@ static uint32_t skip_valid_arguments(const zend_op_array *op_array, zend_ssa *ss uint32_t num_args = 0; zend_function *func = call_info->callee_func; + /* It's okay to handle prototypes here, because they can only increase the accepted arguments. + * Anything legal for the parent method is also legal for the parent method. */ while (num_args < call_info->num_args) { zend_arg_info *arg_info = func->op_array.arg_info + num_args; @@ -9731,7 +9733,7 @@ static int zend_jit_do_fcall(dasm_State **Dst, const zend_op *opline, const zend while (call_info && call_info->caller_call_opline != opline) { call_info = call_info->next_callee; } - if (call_info && call_info->callee_func) { + if (call_info && call_info->callee_func && !call_info->is_prototype) { func = call_info->callee_func; } } diff --git a/ext/opcache/tests/opt/verify_return_type.phpt b/ext/opcache/tests/opt/verify_return_type.phpt index c1384c7af88..3838cd8b0cf 100644 --- a/ext/opcache/tests/opt/verify_return_type.phpt +++ b/ext/opcache/tests/opt/verify_return_type.phpt @@ -20,6 +20,23 @@ class Test1 { } } +class Test2 { + public function getInt(): int { + return 42; + } + public function getInt2(): int { + return $this->getInt(); + } + public function getIntOrFloat(int $i): int|float { + return $i; + } + public function getInt3(int $i): int { + // Should not elide return type check. Test2::getIntOrFloat() returns only int, + // but a child method may return int|float. + return $this->getIntOrFloat($i); + } +} + ?> --EXPECTF-- $_main: @@ -42,3 +59,35 @@ Test1::getInt: 0000 INIT_METHOD_CALL 0 THIS string("getIntOrFloat") 0001 V0 = DO_UCALL 0002 RETURN V0 + +Test2::getInt: + ; (lines=1, args=0, vars=0, tmps=0) + ; (after optimizer) + ; %s +0000 RETURN int(42) + +Test2::getInt2: + ; (lines=3, args=0, vars=0, tmps=1) + ; (after optimizer) + ; %s +0000 INIT_METHOD_CALL 0 THIS string("getInt") +0001 V0 = DO_FCALL +0002 RETURN V0 + +Test2::getIntOrFloat: + ; (lines=2, args=1, vars=1, tmps=0) + ; (after optimizer) + ; %s +0000 CV0($i) = RECV 1 +0001 RETURN CV0($i) + +Test2::getInt3: + ; (lines=6, args=1, vars=1, tmps=1) + ; (after optimizer) + ; %s +0000 CV0($i) = RECV 1 +0001 INIT_METHOD_CALL 1 THIS string("getIntOrFloat") +0002 SEND_VAR CV0($i) 1 +0003 V1 = DO_FCALL +0004 VERIFY_RETURN_TYPE V1 +0005 RETURN V1