diff --git a/agent/php_execute.c b/agent/php_execute.c index 18465ceb0..ff7297fd9 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -1934,6 +1934,7 @@ static void nr_php_observer_attempt_call_cufa_handler(NR_EXECUTE_PROTO) { nrl_verbosedebug(NRL_AGENT, "%s: cannot get previous execute data", __func__); return; } + if (NULL == execute_data->prev_execute_data->opline) { nrl_verbosedebug(NRL_AGENT, "%s: cannot get previous opline", __func__); return; @@ -1966,20 +1967,40 @@ static void nr_php_observer_attempt_call_cufa_handler(NR_EXECUTE_PROTO) { * cause additional performance overhead, this should be considered a last * resort. */ - const zend_op* prev_opline = execute_data->prev_execute_data->opline - 1; + + /* + * We cannot safely access the opline of internal functions, and we only + * want to instrument cufa calls from user functions anyway + */ + if (UNEXPECTED(NULL == execute_data->prev_execute_data->func)) { + nrl_verbosedebug(NRL_AGENT, "%s: cannot get previous function", __func__); + return; + } + if (!ZEND_USER_CODE(execute_data->prev_execute_data->func->type)) { + nrl_verbosedebug(NRL_AGENT, "%s: caller is php internal function", __func__); + return; + } + + const zend_op* prev_opline = execute_data->prev_execute_data->opline; + + /* + * Extra safety check. Previously, we instrumented by overwritting ZEND_DO_FCALL. + * Within OAPI, for consistency's sake, we will ensure the same + */ + if (ZEND_DO_FCALL != prev_opline->opcode) { + nrl_verbosedebug(NRL_AGENT, "%s: cannot get previous function name", __func__); + return; + } + prev_opline -= 1; // Checks previous opcode if (ZEND_CHECK_UNDEF_ARGS == prev_opline->opcode) { - prev_opline = execute_data->prev_execute_data->opline - 2; + prev_opline -= 1; // Checks previous opcode } if (ZEND_SEND_ARRAY == prev_opline->opcode) { + if (UNEXPECTED((NULL == execute_data->func))) { nrl_verbosedebug(NRL_AGENT, "%s: cannot get current function", __func__); return; } - if (UNEXPECTED(NULL == execute_data->prev_execute_data->func)) { - nrl_verbosedebug(NRL_AGENT, "%s: cannot get previous function", __func__); - return; - } - if (UNEXPECTED(NULL == execute_data->prev_execute_data->func->common.function_name)) { nrl_verbosedebug(NRL_AGENT, "%s: cannot get previous function name", __func__); return; diff --git a/tests/integration/frameworks/wordpress/test_wordpress_do_action.php8.php b/tests/integration/frameworks/wordpress/test_wordpress_do_action.php8.php index 872b87281..0483b2511 100644 --- a/tests/integration/frameworks/wordpress/test_wordpress_do_action.php8.php +++ b/tests/integration/frameworks/wordpress/test_wordpress_do_action.php8.php @@ -21,6 +21,7 @@ */ /*EXPECT +g f h g @@ -74,4 +75,13 @@ function f() { } } +/* + * Initiates a non-flattened call stack of internal->user_code + * to ensure that cufa instrumentation properly handles skipping + * opline lookups of internal functions + */ +$function = new ReflectionFunction('g'); +$function->invoke(); + + do_action("f");