Improve test coverage

php_perfidious.h

@@ -81,7 +81,6 @@ enum perfidious_error_mode
 {
     PERFIDIOUS_ERROR_MODE_THROW = 0,
     PERFIDIOUS_ERROR_MODE_WARNING = 1,
-    PERFIDIOUS_ERROR_MODE_SILENT = 2,
 };
 
 PERFIDIOUS_PUBLIC extern zend_class_entry *perfidious_exception_interface_ce;

src/extension.c

@@ -99,8 +99,6 @@ static struct perfidious_handle *split_and_open(zend_string *restrict metrics, b
     zval z_metrics = {0};
     struct perfidious_handle *handle = NULL;
 
-    PERFIDIOUS_G(error_mode) = PERFIDIOUS_ERROR_MODE_WARNING;
-
     do {
         zend_string *delim = zend_string_init_fast(ZEND_STRL(","));
         array_init(&z_metrics);
@@ -138,14 +136,15 @@ static struct perfidious_handle *split_and_open(zend_string *restrict metrics, b
 
 done:
     zval_dtor(&z_metrics);
-    PERFIDIOUS_G(error_mode) = PERFIDIOUS_ERROR_MODE_THROW;
     return handle;
 }
 
 static PHP_MINIT_FUNCTION(perfidious)
 {
     const int flags = CONST_CS | CONST_PERSISTENT;
 
+    PERFIDIOUS_G(error_mode) = PERFIDIOUS_ERROR_MODE_WARNING;
+
     // Initialize pfm
     int pfm_ret = pfm_initialize();
     if (pfm_ret != PFM_SUCCESS) {
@@ -185,11 +184,15 @@ static PHP_MINIT_FUNCTION(perfidious)
         }
     }
 
+    PERFIDIOUS_G(error_mode) = PERFIDIOUS_ERROR_MODE_THROW;
+
     return SUCCESS;
 }
 
 static PHP_MSHUTDOWN_FUNCTION(perfidious)
 {
+    PERFIDIOUS_G(error_mode) = PERFIDIOUS_ERROR_MODE_WARNING;
+
     if (PERFIDIOUS_G(request_handle)) {
         perfidious_handle_close(PERFIDIOUS_G(request_handle));
         PERFIDIOUS_G(request_handle) = NULL;
@@ -210,6 +213,7 @@ static zend_always_inline void minfo_handle_metrics(struct perfidious_handle *re
     zval z_metrics = {0};
 
     if (UNEXPECTED(FAILURE == perfidious_handle_read_to_array(handle, &z_metrics))) {
+        php_info_print_table_colspan_header(2, "READ ERROR");
         return;
     }
 

src/handle.c

@@ -35,7 +35,8 @@
 #include <Zend/zend_API.h>
 #include <Zend/zend_exceptions.h>
 #include <Zend/zend_portability.h>
-#include "main/php.h"
+#include <main/php.h>
+#include <ext/spl/spl_exceptions.h>
 
 #include "php_perfidious.h"
 #include "handle.h"
@@ -141,7 +142,7 @@ zend_result perfidious_handle_close(struct perfidious_handle *restrict handle)
 
     for (ssize_t i = (ssize_t) handle->metrics_count - 1; i >= 0; i--) {
         if (UNEXPECTED(-1 == close(handle->metrics[i].fd))) {
-            zend_throw_exception_ex(perfidious_io_exception_ce, errno, "close failed: %s", strerror(errno));
+            perfidious_error_helper(perfidious_io_exception_ce, errno, "close failed: %s", strerror(errno));
             rv = FAILURE;
             // continue even if it fails
         }
@@ -207,6 +208,13 @@ zend_result perfidious_handle_read_to_array_with_times(
         struct perfidious_read_format_value *value = &data->values[i];
 
         if (UNEXPECTED(metric->id != value->id)) {
+            perfidious_error_helper(
+                spl_ce_DomainException, 0, "ID mismatch: %" PRIu64 " != %" PRIu64, metric->id, value->id
+            );
+            zval_ptr_dtor(return_value);
+            ZVAL_UNDEF(return_value);
+            return FAILURE;
+            /*
             metric = NULL;
 
             // skip the first entry - it should be the dummy
@@ -220,6 +228,7 @@ zend_result perfidious_handle_read_to_array_with_times(
             if (UNEXPECTED(metric == NULL)) {
                 continue;
             }
+            */
         } else if (i == 0) {
             // skip the first entry - it should be the dummy
             continue;
@@ -317,7 +326,6 @@ struct perfidious_handle *perfidious_handle_open_ex(
     struct perfidious_handle *handle = pecalloc(
         sizeof(struct perfidious_handle) + sizeof(struct perfidious_metric) * (event_names_length + 1), 1, persist
     );
-    handle->marker = PERFIDIOUS_HANDLE_MARKER;
     handle->metrics_size = event_names_length + 1;
 
     // Open a dummy event to hold the group
@@ -372,7 +380,7 @@ struct perfidious_handle *perfidious_handle_open_ex(
 
         err = pfm_get_os_event_encoding(ZSTR_VAL(event_name), PFM_PLM3, PFM_OS_PERF_EVENT, &arg);
         if (UNEXPECTED(err != PFM_SUCCESS)) {
-            zend_throw_exception_ex(
+            perfidious_error_helper(
                 perfidious_pmu_event_not_found_exception_ce,
                 err,
                 "failed to get libpfm event encoding for %.*s: %s",
@@ -393,7 +401,7 @@ struct perfidious_handle *perfidious_handle_open_ex(
         fd = (int) perf_event_open(&attr, pid, cpu, group_fd, 0);
 
         if (UNEXPECTED(fd == -1)) {
-            zend_throw_exception_ex(
+            perfidious_error_helper(
                 perfidious_io_exception_ce,
                 errno,
                 "perf_event_open() failed for %.*s: %s",
@@ -442,10 +450,6 @@ static PHP_METHOD(PerfidousHandle, disable)
 
     struct perfidious_handle_obj *obj = perfidious_fetch_handle_object(Z_OBJ_P(ZEND_THIS));
 
-    if (UNEXPECTED(FAILURE == perfidious_handle_marker_assert(obj->handle))) {
-        RETURN_NULL();
-    }
-
     perfidious_handle_disable(obj->handle);
 
     RETURN_ZVAL(ZEND_THIS, 1, 0);
@@ -458,10 +462,6 @@ static PHP_METHOD(PerfidousHandle, enable)
 
     struct perfidious_handle_obj *obj = perfidious_fetch_handle_object(Z_OBJ_P(ZEND_THIS));
 
-    if (UNEXPECTED(FAILURE == perfidious_handle_marker_assert(obj->handle))) {
-        RETURN_NULL();
-    }
-
     perfidious_handle_enable(obj->handle);
 
     RETURN_ZVAL(ZEND_THIS, 1, 0);
@@ -470,15 +470,20 @@ static PHP_METHOD(PerfidousHandle, enable)
 ZEND_COLD
 static PHP_METHOD(PerfidousHandle, rawStream)
 {
-    ZEND_PARSE_PARAMETERS_NONE();
+    zend_long idx = 0;
+
+    ZEND_PARSE_PARAMETERS_START(0, 1)
+        Z_PARAM_OPTIONAL
+        Z_PARAM_LONG(idx)
+    ZEND_PARSE_PARAMETERS_END();
 
     struct perfidious_handle_obj *obj = perfidious_fetch_handle_object(Z_OBJ_P(ZEND_THIS));
 
-    if (UNEXPECTED(FAILURE == perfidious_handle_marker_assert(obj->handle))) {
+    if (UNEXPECTED((size_t) idx >= obj->handle->metrics_count)) {
         RETURN_NULL();
     }
 
-    php_stream *stream = php_stream_fopen_from_fd(obj->handle->metrics[0].fd, "r", NULL);
+    php_stream *stream = php_stream_fopen_from_fd(obj->handle->metrics[idx].fd, "r", NULL);
     php_stream_to_zval(stream, return_value);
 }
 
@@ -489,10 +494,6 @@ static PHP_METHOD(PerfidousHandle, read)
 
     struct perfidious_handle_obj *obj = perfidious_fetch_handle_object(Z_OBJ_P(ZEND_THIS));
 
-    if (UNEXPECTED(FAILURE == perfidious_handle_marker_assert(obj->handle))) {
-        RETURN_NULL();
-    }
-
     bool orig_enabled = obj->handle->enabled;
 
     if (orig_enabled) {
@@ -513,10 +514,6 @@ static PHP_METHOD(PerfidousHandle, readArray)
 
     struct perfidious_handle_obj *obj = perfidious_fetch_handle_object(Z_OBJ_P(ZEND_THIS));
 
-    if (UNEXPECTED(FAILURE == perfidious_handle_marker_assert(obj->handle))) {
-        RETURN_NULL();
-    }
-
     bool orig_enabled = obj->handle->enabled;
 
     if (orig_enabled) {
@@ -539,10 +536,6 @@ static PHP_METHOD(PerfidousHandle, reset)
 
     struct perfidious_handle_obj *obj = perfidious_fetch_handle_object(Z_OBJ_P(ZEND_THIS));
 
-    if (UNEXPECTED(FAILURE == perfidious_handle_marker_assert(obj->handle))) {
-        RETURN_NULL();
-    }
-
     perfidious_handle_reset(obj->handle);
 
     RETURN_ZVAL(ZEND_THIS, 1, 0);

src/handle.h

@@ -27,8 +27,6 @@
 #include <Zend/zend_portability.h>
 #include "php_perfidious.h"
 
-static const uint64_t PERFIDIOUS_HANDLE_MARKER = 0x327b23c66b8b4567;
-
 struct perfidious_metric
 {
     int fd;
@@ -38,7 +36,6 @@ struct perfidious_metric
 
 struct perfidious_handle
 {
-    uint64_t marker;
     size_t metrics_size;
     size_t metrics_count;
     bool enabled;
@@ -82,6 +79,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(perfidious_handle_enable_arginfo, 0, 0, P
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO(perfidious_handle_raw_stream_arginfo, IS_RESOURCE, 0)
+    ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(false, idx, IS_LONG, true, "0")
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(perfidious_handle_read_arginfo, 0, 0, Perfidious\\ReadResult, 0)
@@ -109,23 +107,4 @@ zend_result perfidious_handle_read_to_array_with_times(
     uint64_t *restrict time_running
 );
 
-ZEND_HOT
-PERFIDIOUS_ATTR_NONNULL_ALL
-PERFIDIOUS_ATTR_WARN_UNUSED_RESULT
-static zend_always_inline zend_result perfidious_handle_marker_assert(struct perfidious_handle *restrict handle)
-{
-    if (UNEXPECTED(handle->marker != PERFIDIOUS_HANDLE_MARKER)) {
-        zend_throw_exception_ex(
-            perfidious_overflow_exception_ce,
-            0,
-            "Assertion failed: marker mismatch: %" PRIu64 " != %" PRIu64,
-            handle->marker,
-            PERFIDIOUS_HANDLE_MARKER
-        );
-        return FAILURE;
-    }
-
-    return SUCCESS;
-}
-
 #endif /* PERFIDIOUS_HANDLE_H */

src/private.h

@@ -82,7 +82,9 @@ static inline bool perfidious_zend_long_to_pid_t(zend_long from, pid_t *restrict
 #if SIZEOF_ZEND_LONG > SIZEOF_PID_T
     const zend_long PID_MAX = (((zend_long) 1) << ((SIZEOF_PID_T * 8) - 1)) - 1;
     if (UNEXPECTED(from > PID_MAX)) {
-        zend_throw_exception_ex(perfidious_overflow_exception_ce, 0, "pid too large: %ld > %ld", from, PID_MAX);
+        zend_throw_exception_ex(
+            perfidious_overflow_exception_ce, 0, "pid too large: %" ZEND_LONG_FMT_SPEC " > %ld", from, PID_MAX
+        );
         return false;
     }
 #endif
@@ -105,8 +107,6 @@ perfidious_error_helper(zend_class_entry *restrict exception_ce, zend_long code,
     va_end(args);
 
     switch (PERFIDIOUS_G(error_mode)) {
-        case PERFIDIOUS_ERROR_MODE_SILENT:
-            break;
         case PERFIDIOUS_ERROR_MODE_WARNING:
             php_error_docref(NULL, E_WARNING, "%.*s", bytes, buffer);
             break;

tests/handle/dirty-2.phpt

@@ -0,0 +1,19 @@
+--TEST--
+Perfidious\Handle::rawStream() - do dirty things part 2
+--EXTENSIONS--
+perfidious
+--FILE--
+<?php
+$handle = Perfidious\open([
+    "perf::PERF_COUNT_SW_CPU_CLOCK:u",
+    "perf::PERF_COUNT_SW_PAGE_FAULTS:u",
+    "perf::PERF_COUNT_SW_CONTEXT_SWITCHES:u",
+]);
+$stream = $handle->rawStream(2);
+var_dump(strlen(fread($stream, 32)));
+fclose($stream);
+var_dump($handle->read());
+--EXPECTF--
+int(32)
+%A Uncaught Perfidious\IOException: failed to read: Illegal seek %A
+%A Uncaught Perfidious\IOException: close failed: Bad file descriptor %A

tests/handle/dirty-3.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Perfidious\Handle::rawStream() - do dirty things part 3
+--EXTENSIONS--
+perfidious
+--FILE--
+<?php
+$handle = Perfidious\open([
+    "perf::PERF_COUNT_SW_CPU_CLOCK:u",
+]);
+$stream = $handle->rawStream();
+var_dump(strlen(fread($stream, 32)));
+fclose($stream);
+try {
+    var_dump($handle->enable());
+} catch (\Throwable $e) {
+    var_dump($e->getMessage());
+}
+try {
+    var_dump($handle->disable());
+} catch (\Throwable $e) {
+    var_dump($e->getMessage());
+}
+try {
+    var_dump($handle->reset());
+} catch (\Throwable $e) {
+    var_dump($e->getMessage());
+}
+--EXPECTF--
+int(32)
+string(%d) "ioctl failed: Bad file descriptor"
+string(%d) "ioctl failed: Bad file descriptor"
+string(%d) "ioctl failed: Bad file descriptor"
+%A Uncaught Perfidious\IOException: close failed: Bad file descriptor %A

tests/handle/invalid-event-name-2.phpt

@@ -0,0 +1,14 @@
+--TEST--
+Perfidious\Handle::open() - invalid event name 2
+--EXTENSIONS--
+perfidious
+--FILE--
+<?php
+$rv = Perfidious\open([
+    "perf::PERF_COUNT_SW_CPU_CLOCK:u",
+    "perf::PERF_COUNT_SW_PAGE_FAULTS:u",
+    "perf::PERF_COUNT_SW_CONTEXT_SWITCHES:u",
+    "blahblahblah",
+]);
+--EXPECTF--
+%A Uncaught Perfidious\PmuEventNotFoundException: failed to get libpfm event encoding for blahblahblah: event not found %A

tests/handle/invalid-event-name.phpt

@@ -1,5 +1,5 @@
 --TEST--
-Perfidious\Handle::enable()
+Perfidious\Handle::open() - invalid event name
 --EXTENSIONS--
 perfidious
 --FILE--

tests/info-both.phpt

@@ -1,5 +1,5 @@
 --TEST--
-info
+phpinfo global and per-request stats
 --EXTENSIONS--
 perfidious
 --INI--

tests/info-global-closed.phpt

@@ -0,0 +1,16 @@
+--TEST--
+phpinfo global stats - closed fd
+--EXTENSIONS--
+perfidious
+--INI--
+perfidious.global.enable=1
+perfidious.global.metrics=perf::PERF_COUNT_SW_CPU_CLOCK:u,perf::PERF_COUNT_SW_PAGE_FAULTS:u,perf::PERF_COUNT_SW_CONTEXT_SWITCHES:u
+--FILE--
+<?php
+$handle = Perfidious\global_handle();
+fclose($handle->rawStream());
+phpinfo(INFO_MODULES);
+// looks like the second error is happening during mshutdown and doesn't get printed?
+--EXPECTF--
+%A READ ERROR %A
+%A Uncaught Perfidious\IOException: failed to read: Bad file descriptor %A

tests/info-global-invalid.phpt

@@ -0,0 +1,12 @@
+--TEST--
+phpinfo global stats - invalid event name
+--EXTENSIONS--
+perfidious
+--INI--
+perfidious.global.enable=1
+perfidious.global.metrics=blahblahblah
+--FILE--
+<?php
+phpinfo(INFO_MODULES);
+--EXPECTF--
+%AWarning: PHP Startup: failed to get libpfm event encoding for blahblahblah: event not found%A

tests/info-global.phpt

@@ -1,5 +1,5 @@
 --TEST--
-info
+phpinfo global stats
 --EXTENSIONS--
 perfidious
 --INI--