Change segfault method! (#97)

Use php_error_docref instead of php_log_err to emit warnings and notices

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, &params[2], &params[3]);
+            php_error_docref(NULL, E_WARNING,
+                             "OpenTelemetry: pre hook invalid signature,"
+                             " class=%s function=%s",
+                             (Z_TYPE_P(&params[2]) == IS_NULL)
+                                 ? "null"
+                                 : Z_STRVAL_P(&params[2]),
+                             Z_STRVAL_P(&params[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, &params[4], &params[5]);
+            php_error_docref(NULL, E_WARNING,
+                             "OpenTelemetry: post hook invalid signature, "
+                             "class=%s function=%s",
+                             (Z_TYPE_P(&params[4]) == IS_NULL)
+                                 ? "null"
+                                 : Z_STRVAL_P(&params[4]),
+                             Z_STRVAL_P(&params[5]));
             continue;
         }
 

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--
+<?php if (PHP_VERSION_ID < 80200) die('skip requires PHP >= 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--
 <?php
 OpenTelemetry\Instrumentation\hook(
@@ -22,8 +22,11 @@ OpenTelemetry\Instrumentation\hook(
 
 var_dump(array_slice([1,2,3], 1));
 ?>
---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)
 }

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--
 <?php
 OpenTelemetry\Instrumentation\hook(
@@ -33,4 +31,5 @@ TestClass::test();
 --EXPECTF--
 string(3) "pre"
 string(4) "test"
-OpenTelemetry: post hook invalid signature, class=TestClass function=test
+
+Warning: TestClass::test(): OpenTelemetry: post hook invalid signature, class=TestClass function=test in %s on line %d

ext/tests/invalid_pre_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 and a message will be written to error_log.
 --EXTENSIONS--
 opentelemetry
---XFAIL--
-Providing a pre invalid callback signature causes segfault. The behaviour is currently disabled, so instead of a segfault a message is logged to error_log.
 --FILE--
 <?php
 OpenTelemetry\Instrumentation\hook(
@@ -31,6 +29,6 @@ class TestClass {
 TestClass::test();
 ?>
 --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"

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--
 <?php
 OpenTelemetry\Instrumentation\hook(
@@ -30,6 +28,6 @@ function hello(): void
 hello();
 ?>
 --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"

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--
+<?php if (PHP_VERSION_ID < 80200) die('skip requires PHP >= 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"
 }