From 5236169457b3945be397a8f8ab942c887eaa81f5 Mon Sep 17 00:00:00 2001 From: X3N0N Date: Tue, 10 Oct 2023 00:59:56 +0200 Subject: [PATCH] Change segfault method! (#97) Use php_error_docref instead of php_log_err to emit warnings and notices --- ext/otel_observer.c | 56 ++++++------------- ext/tests/expand_params_internal.phpt | 11 ++-- .../invalid_post_callback_signature.phpt | 5 +- ext/tests/invalid_pre_callback_signature.phpt | 4 +- ..._pre_callback_signature_with_function.phpt | 4 +- .../return_expanded_params_internal.phpt | 11 ++-- 6 files changed, 36 insertions(+), 55 deletions(-) diff --git a/ext/otel_observer.c b/ext/otel_observer.c index d0130c8..c3ca393 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -187,36 +187,6 @@ static inline bool is_valid_signature(zend_fcall_info fci, return true; } -static void log_invalid_message(char *msg, zval *scope, zval *function) { - char *s; - if (Z_TYPE_P(scope) == IS_NULL) { - s = "null"; - } else { - s = Z_STRVAL_P(scope); - } - char *f = Z_STRVAL_P(function); - - // Calculate the size of the formatted message. - int formatted_size = strlen(msg) + strlen(s) + strlen(f) + 1; - - // Allocate a buffer for the formatted message. - char *formatted = malloc(formatted_size); - if (formatted == NULL) { - php_log_err("OpenTelemetry: Failed to allocate " - "memory for formatted message."); - return; - } - - // Format the message. - snprintf(formatted, formatted_size, msg, s, f); - - // Log the message. - php_log_err(formatted); - - // Free the allocated memory. - free(formatted); -} - static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { if (!zend_llist_count(hooks)) { return; @@ -249,9 +219,13 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { fci.retval = &ret; if (!is_valid_signature(fci, fcc)) { - char *msg = "OpenTelemetry: pre hook invalid signature, class=%s " - "function=%s"; - log_invalid_message(msg, ¶ms[2], ¶ms[3]); + php_error_docref(NULL, E_WARNING, + "OpenTelemetry: pre hook invalid signature," + " class=%s function=%s", + (Z_TYPE_P(¶ms[2]) == IS_NULL) + ? "null" + : Z_STRVAL_P(¶ms[2]), + Z_STRVAL_P(¶ms[3])); continue; } @@ -276,8 +250,10 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { if (func->type == ZEND_INTERNAL_FUNCTION) { // TODO expanding args for internal functions causes // segfault - php_log_err("OpenTelemetry: expanding args of " - "internal functions not supported"); + php_error_docref( + NULL, E_NOTICE, + "OpenTelemetry: expanding args of " + "internal functions not supported"); continue; } // TODO Extend call frame? @@ -367,9 +343,13 @@ static void observer_end(zend_execute_data *execute_data, zval *retval, fci.retval = &ret; if (!is_valid_signature(fci, fcc)) { - char *msg = "OpenTelemetry: post hook invalid signature, class=%s " - "function=%s"; - log_invalid_message(msg, ¶ms[4], ¶ms[5]); + php_error_docref(NULL, E_WARNING, + "OpenTelemetry: post hook invalid signature, " + "class=%s function=%s", + (Z_TYPE_P(¶ms[4]) == IS_NULL) + ? "null" + : Z_STRVAL_P(¶ms[4]), + Z_STRVAL_P(¶ms[5])); continue; } diff --git a/ext/tests/expand_params_internal.phpt b/ext/tests/expand_params_internal.phpt index bfc0260..88faf62 100644 --- a/ext/tests/expand_params_internal.phpt +++ b/ext/tests/expand_params_internal.phpt @@ -5,10 +5,10 @@ Removing the `post` callback avoids the segfault. The segfault is actually durin from `zend_observer_fcall_end_all` (https://github.com/php/php-src/blob/php-8.2.8/Zend/zend_observer.c#L291). When it traverses back through zend_execute_data, the top-level frame appears corrupted, and any attempt to reference it is what causes the segfault. +--SKIPIF-- += 8.2'); ?> --EXTENSIONS-- opentelemetry ---XFAIL-- -Providing a post callback when expanding params of internal function causes segfault. The behaviour is currently disabled, so instead of a segfault a message is logged to error_log. --FILE-- ---EXPECT-- -array(1) { +--EXPECTF-- +Notice: array_slice(): OpenTelemetry: expanding args of internal functions not supported in %s on line %d +array(2) { [0]=> int(2) + [1]=> + int(3) } diff --git a/ext/tests/invalid_post_callback_signature.phpt b/ext/tests/invalid_post_callback_signature.phpt index acef320..536afb6 100644 --- a/ext/tests/invalid_post_callback_signature.phpt +++ b/ext/tests/invalid_post_callback_signature.phpt @@ -5,8 +5,6 @@ The invalid callback signature should not cause a fatal, so it is checked before is invalid, the callback will not be called. --EXTENSIONS-- opentelemetry ---XFAIL-- -Providing a post invalid callback signature causes segfault. The behaviour is currently disabled, so instead of a segfault a message is logged to error_log. --FILE-- --EXPECTF-- -OpenTelemetry: pre hook invalid signature, class=TestClass function=test +Warning: TestClass::test(): OpenTelemetry: pre hook invalid signature, class=TestClass function=test in %s on line %d string(4) "test" string(4) "post" diff --git a/ext/tests/invalid_pre_callback_signature_with_function.phpt b/ext/tests/invalid_pre_callback_signature_with_function.phpt index 1cfb335..81af076 100644 --- a/ext/tests/invalid_pre_callback_signature_with_function.phpt +++ b/ext/tests/invalid_pre_callback_signature_with_function.phpt @@ -6,8 +6,6 @@ is invalid, the callback will not be called and a message will be written to err null class. --EXTENSIONS-- opentelemetry ---XFAIL-- -Providing a invalid pre callback signature for function without class causes segfault. The behaviour is currently disabled, so instead of a segfault a message is logged to error_log. --FILE-- --EXPECTF-- -OpenTelemetry: pre hook invalid signature, class=null function=hello +Warning: hello(): OpenTelemetry: pre hook invalid signature, class=null function=hello in %s on line %d string(5) "hello" string(4) "post" diff --git a/ext/tests/return_expanded_params_internal.phpt b/ext/tests/return_expanded_params_internal.phpt index 385f847..85d64e1 100644 --- a/ext/tests/return_expanded_params_internal.phpt +++ b/ext/tests/return_expanded_params_internal.phpt @@ -2,6 +2,8 @@ Check if pre hook can expand and then return $params of internal function --DESCRIPTION-- The existence of a post callback is part of the failure preconditions. +--SKIPIF-- += 8.2'); ?> --EXTENSIONS-- opentelemetry --FILE-- @@ -18,10 +20,11 @@ opentelemetry var_dump(array_slice(['a', 'b', 'c'], 1)); ?> ---XFAIL-- -Core dump (same issue as in return_expanded_params.phpt) ---EXPECT-- -array(1) { +--EXPECTF-- +Notice: array_slice(): OpenTelemetry: expanding args of internal functions not supported in %s on line %d +array(2) { [0]=> string(1) "b" + [1]=> + string(1) "c" }