From 413a0d26123e4de30d2b107871ec04fda84c2426 Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Thu, 7 Dec 2023 12:52:01 +0100 Subject: [PATCH 1/2] Placing class and method name directly into AST insted of using magic constants --- agent/native/ext/AST_instrumentation.cpp | 12 +++++++----- agent/native/ext/AST_instrumentation.h | 2 +- agent/native/ext/Hooking.cpp | 4 ++-- agent/native/ext/WordPress_instrumentation.cpp | 8 ++++---- agent/native/ext/tracer_PHP_part.cpp | 1 - 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/agent/native/ext/AST_instrumentation.cpp b/agent/native/ext/AST_instrumentation.cpp index 45a096269..e908b3831 100644 --- a/agent/native/ext/AST_instrumentation.cpp +++ b/agent/native/ext/AST_instrumentation.cpp @@ -507,7 +507,7 @@ zend_ast* createAstStandaloneNotFqFunctionCall( StringView funcName, zend_ast* a return createAstStandaloneFunctionCall( funcName, /* isFullyQualified */ false, astArgList ); } -ResultCode createPreHookAstArgListByCaptureSpec( zend_ast_decl* astDecl, ArgCaptureSpecArrayView argCaptureSpecs, /* out */ zend_ast** pResult ) +ResultCode createPreHookAstArgListByCaptureSpec( zend_ast_decl *parentClass, zend_ast_decl* astDecl, ArgCaptureSpecArrayView argCaptureSpecs, /* out */ zend_ast** pResult ) { // AST for PHP code: // @@ -526,12 +526,14 @@ ResultCode createPreHookAstArgListByCaptureSpec( zend_ast_decl* astDecl, ArgCapt uint32_t lineNumber = astDecl->start_lineno; zend_ast* capturedArgsAstArray = NULL; + + ELASTIC_APM_CALL_IF_FAILED_GOTO( createCapturedArgsAstArray( astDecl, argCaptureSpecs, lineNumber, /* out */ &capturedArgsAstArray ) ); *pResult = createAstListWithThreeChildren( ZEND_AST_ARG_LIST - , astDecl->kind == ZEND_AST_METHOD ? createAstMagicConst__CLASS__( lineNumber ) : createAstConstNull( lineNumber ) - , createAstMagicConst__FUNCTION__( lineNumber ) + , astDecl->kind == ZEND_AST_METHOD ? createAstZValString(makeStringView(ZSTR_VAL(((zend_ast_decl *)parentClass)->name), ZSTR_LEN(((zend_ast_decl *)parentClass)->name)), lineNumber) : createAstConstNull( lineNumber ) + , createAstZValString(makeStringView(ZSTR_VAL(astDecl->name), ZSTR_LEN(astDecl->name)), lineNumber) , capturedArgsAstArray ); resultCode = resultSuccess; @@ -552,7 +554,7 @@ static StringView g_elastic_apm_ast_instrumentation_pre_hook_funcName = ELASTIC_ */ static const size_t g_funcDeclBodyChildIndex = 2; -ResultCode insertAstForFunctionPreHook( zend_ast_decl* funcAstDecl, ArgCaptureSpecArrayView argCaptureSpecs ) +ResultCode insertAstForFunctionPreHook( zend_ast_decl *parentClass, zend_ast_decl* funcAstDecl, ArgCaptureSpecArrayView argCaptureSpecs ) { // Before: // @@ -623,7 +625,7 @@ ResultCode insertAstForFunctionPreHook( zend_ast_decl* funcAstDecl, ArgCaptureSp ELASTIC_APM_SET_RESULT_CODE_AND_GOTO_FAILURE(); } - ELASTIC_APM_CALL_IF_FAILED_GOTO( createPreHookAstArgListByCaptureSpec( funcAstDecl, argCaptureSpecs, /* out */ &preHookCallAstArgList ) ); + ELASTIC_APM_CALL_IF_FAILED_GOTO( createPreHookAstArgListByCaptureSpec( parentClass, funcAstDecl, argCaptureSpecs, /* out */ &preHookCallAstArgList ) ); funcAstDecl->child[ g_funcDeclBodyChildIndex ] = createAstListWithTwoChildren( ZEND_AST_STMT_LIST diff --git a/agent/native/ext/AST_instrumentation.h b/agent/native/ext/AST_instrumentation.h index 2a6c0266c..29e939001 100644 --- a/agent/native/ext/AST_instrumentation.h +++ b/agent/native/ext/AST_instrumentation.h @@ -45,7 +45,7 @@ zend_ast_decl** findChildSlotForStandaloneFunctionAst( zend_ast* rootAst, String zend_ast_decl* findClassAst( zend_ast* rootAst, StringView nameSpace, StringView className ); zend_ast_decl** findChildSlotForMethodAst( zend_ast_decl* astClass, StringView methodName, size_t minParamsCount ); -ResultCode insertAstForFunctionPreHook( zend_ast_decl* funcAstDecl, ArgCaptureSpecArrayView argCaptureSpecs ); +ResultCode insertAstForFunctionPreHook( zend_ast_decl *parentClass, zend_ast_decl* funcAstDecl, ArgCaptureSpecArrayView argCaptureSpecs ); ResultCode appendDirectCallToInstrumentation( zend_ast_decl** pAstChildSlot, StringView constNameForMethodName ); ResultCode wrapStandaloneFunctionAstWithPrePostHooks( zend_ast_decl** pAstChildSlot ); diff --git a/agent/native/ext/Hooking.cpp b/agent/native/ext/Hooking.cpp index f3610bf66..f182ed0b1 100644 --- a/agent/native/ext/Hooking.cpp +++ b/agent/native/ext/Hooking.cpp @@ -58,7 +58,7 @@ void elastic_apm_error_cb(int type, zend_string *error_filename, const uint32_t } } -static void elastic_execute_internal(INTERNAL_FUNCTION_PARAMETERS) { +[[maybe_unused]] static void elastic_execute_internal(INTERNAL_FUNCTION_PARAMETERS) { zend_try { if (Hooking::getInstance().getOriginalExecuteInternal()) { @@ -89,7 +89,7 @@ static void elastic_interrupt_function(zend_execute_data *execute_data) { } void Hooking::replaceHooks() { - zend_execute_internal = elastic_execute_internal; + // zend_execute_internal = elastic_execute_internal; zend_interrupt_function = elastic_interrupt_function; zend_error_cb = elastic_apm_error_cb; } diff --git a/agent/native/ext/WordPress_instrumentation.cpp b/agent/native/ext/WordPress_instrumentation.cpp index b542f68da..ab266b8d2 100644 --- a/agent/native/ext/WordPress_instrumentation.cpp +++ b/agent/native/ext/WordPress_instrumentation.cpp @@ -87,14 +87,14 @@ void wordPressInstrumentationOnRequestShutdown() } static -ResultCode insertPreHookForFunctionWithHookNameCallbackParams( zend_ast_decl* astDecl ) +ResultCode insertPreHookForFunctionWithHookNameCallbackParams( zend_ast_decl *parentClass, zend_ast_decl* astDecl ) { // standalone function: // function _wp_filter_build_unique_id( $hook_name, $callback, $priority ) // class WP_Hook method // public function add_filter( $hook_name, $callback, $priority, $accepted_args ) ArgCaptureSpec argCaptureSpecArr[] = { /* capture $hook_name by value */ captureArgByValue, /* capture $callback by reference */ captureArgByRef }; - return insertAstForFunctionPreHook( astDecl, ELASTIC_APM_MAKE_ARRAY_VIEW_FROM_STATIC( ArgCaptureSpecArrayView, argCaptureSpecArr ) ); + return insertAstForFunctionPreHook(parentClass, astDecl, ELASTIC_APM_MAKE_ARRAY_VIEW_FROM_STATIC( ArgCaptureSpecArrayView, argCaptureSpecArr ) ); } static StringView g_globalNamespace = ELASTIC_APM_STRING_LITERAL_TO_VIEW( "" ); @@ -116,7 +116,7 @@ ResultCode wordPressInstrumentationTransformFile_plugin_php( zend_ast* ast ) ELASTIC_APM_SET_RESULT_CODE_AND_GOTO_FAILURE(); } - ELASTIC_APM_CALL_IF_FAILED_GOTO( insertPreHookForFunctionWithHookNameCallbackParams( *p_wp_filter_build_unique_id_astFuncDecl ) ); + ELASTIC_APM_CALL_IF_FAILED_GOTO( insertPreHookForFunctionWithHookNameCallbackParams( nullptr, *p_wp_filter_build_unique_id_astFuncDecl ) ); // It's important to record if we instrumented _wp_filter_build_unique_id successfully. // _wp_filter_build_unique_id instrumentation alone cannot make application work incorrectly // because it checks if $callback is an instance our WordPressFilterCallbackWrapper class before unwrapping it. @@ -159,7 +159,7 @@ ResultCode wordPressInstrumentationTransformFile_class_wp_hook_php( zend_ast* as ELASTIC_APM_SET_RESULT_CODE_AND_GOTO_FAILURE(); } - ELASTIC_APM_CALL_IF_FAILED_GOTO( insertPreHookForFunctionWithHookNameCallbackParams( *p_add_filter_astMethod ) ); + ELASTIC_APM_CALL_IF_FAILED_GOTO( insertPreHookForFunctionWithHookNameCallbackParams(WP_Hook_astClassDecl, *p_add_filter_astMethod ) ); resultCode = resultSuccess; finally: diff --git a/agent/native/ext/tracer_PHP_part.cpp b/agent/native/ext/tracer_PHP_part.cpp index c72ab4912..446716622 100644 --- a/agent/native/ext/tracer_PHP_part.cpp +++ b/agent/native/ext/tracer_PHP_part.cpp @@ -370,7 +370,6 @@ void tracerPhpPartForwardCall( StringView phpFuncName, zend_execute_data* execut } } - getArgsFromZendExecuteData( execute_data, g_maxInterceptedCallArgsCount, &( callArgs[ 0 ] ), &callArgsCount ); tracerPhpPartLogArguments( logLevel_trace, callArgsCount, callArgs ); From baf04c4e90c5681944173c641ca4af6096cbed19 Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Thu, 7 Dec 2023 13:15:50 +0100 Subject: [PATCH 2/2] reverted commented out code --- agent/native/ext/Hooking.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/native/ext/Hooking.cpp b/agent/native/ext/Hooking.cpp index f182ed0b1..f3610bf66 100644 --- a/agent/native/ext/Hooking.cpp +++ b/agent/native/ext/Hooking.cpp @@ -58,7 +58,7 @@ void elastic_apm_error_cb(int type, zend_string *error_filename, const uint32_t } } -[[maybe_unused]] static void elastic_execute_internal(INTERNAL_FUNCTION_PARAMETERS) { +static void elastic_execute_internal(INTERNAL_FUNCTION_PARAMETERS) { zend_try { if (Hooking::getInstance().getOriginalExecuteInternal()) { @@ -89,7 +89,7 @@ static void elastic_interrupt_function(zend_execute_data *execute_data) { } void Hooking::replaceHooks() { - // zend_execute_internal = elastic_execute_internal; + zend_execute_internal = elastic_execute_internal; zend_interrupt_function = elastic_interrupt_function; zend_error_cb = elastic_apm_error_cb; }