fix(profiling): disable wall/cpu on some PHP 8.5 builds

profiling/src/bindings/mod.rs

@@ -340,6 +340,10 @@ extern "C" {
     /// Must be called from a PHP thread during a request.
     pub fn datadog_php_profiling_vm_interrupt_addr() -> *const AtomicBool;
 
+    /// Returns Failure when this PHP build/runtime has the tailcall VM
+    /// interrupt crash that requires time sample collection to be disabled.
+    pub fn ddog_php_prof_check_tailcall_vm_interrupt() -> ZendResult;
+
     /// Registers the extension. Note that it's kept in a zend_llist and gets
     /// pemalloc'd + memcpy'd into place. The engine says this is a mutable
     /// pointer, but in practice it's const.

profiling/src/capi.rs

@@ -43,7 +43,10 @@ extern "C" fn ddog_php_prof_trigger_time_sample() {
     use std::sync::atomic::Ordering;
 
     let result = super::REQUEST_LOCALS.try_with_borrow(|locals| {
-        if locals.system_settings().profiling_enabled {
+        let system_settings = locals.system_settings();
+        if system_settings.profiling_wall_time_enabled
+            | system_settings.profiling_experimental_cpu_time_enabled
+        {
             // Safety: only vm interrupts are stored there, or possibly null (edges only).
             if let Some(vm_interrupt) = unsafe { locals.vm_interrupt_addr.as_ref() } {
                 locals.interrupt_count.fetch_add(1, Ordering::SeqCst);

profiling/src/config.rs

@@ -132,6 +132,21 @@ impl SystemSettings {
         let mut system_settings = SystemSettings::new();
 
         // Work around version-specific issues.
+        let tailcall_check = unsafe { bindings::ddog_php_prof_check_tailcall_vm_interrupt() };
+        if system_settings.profiling_enabled
+            && (system_settings.profiling_wall_time_enabled
+                | system_settings.profiling_experimental_cpu_time_enabled)
+            && tailcall_check != bindings::ZendResult::Success
+        {
+            error!(concat!(
+                "Wall- and CPU-time sample types are disabled because their VM interrupts can crash PHP 8.5.0-8.5.6 builds that have the tailcall VM.",
+                " To re-enable them, upgrade PHP to 8.5.7 or newer, or use a PHP build without the tailcall VM.",
+                " See https://github.com/php/php-src/pull/21922"
+            ));
+            system_settings.profiling_wall_time_enabled = false;
+            system_settings.profiling_experimental_cpu_time_enabled = false;
+        }
+
         #[cfg(not(php_zend_mm_set_custom_handlers_ex))]
         if allocation::allocation_le83::first_rinit_should_disable_due_to_jit() {
             if bindings::PHP_VERSION_ID >= 80400 {
@@ -1105,9 +1120,8 @@ pub(crate) fn minit(module_number: libc::c_int) {
                     displayer: None,
                     env_config_fallback: None,
                 },
-                // At the moment, wall-time cannot be fully disabled. This only
-                // controls automatic collection (manual collection is still
-                // possible).
+                // Controls wall-time collection and the wall-time related
+                // sample types.
                 zai_config_entry {
                     id: transmute::<ConfigId, u16>(ProfilingWallTimeEnabled),
                     name: ProfilingWallTimeEnabled.env_var_name(),

profiling/src/lib.rs

@@ -693,7 +693,6 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult {
         // Not logging, rinit could be quite spammy.
         _ = REQUEST_LOCALS.try_with_borrow(|locals| {
             let cpu_time_enabled = system_settings.profiling_experimental_cpu_time_enabled;
-            let wall_time_enabled = system_settings.profiling_wall_time_enabled;
             CLOCKS.with_borrow_mut(|clocks| clocks.initialize(cpu_time_enabled));
 
             TAGS.set({
@@ -717,8 +716,8 @@ extern "C" fn rinit(_type: c_int, _module_number: c_int) -> ZendResult {
                 Arc::new(tags)
             });
 
-            // Only add interrupt if cpu- or wall-time is enabled.
-            if !(cpu_time_enabled | wall_time_enabled) {
+            // Only add interrupt if cpu- or wall-time sample collection is enabled.
+            if !(cpu_time_enabled | system_settings.profiling_wall_time_enabled) {
                 return;
             }
 
@@ -775,10 +774,9 @@ extern "C" fn rshutdown(_type: c_int, _module_number: c_int) -> ZendResult {
     _ = REQUEST_LOCALS.try_with_borrow(|locals| {
         let system_settings = locals.system_settings();
 
-        // The interrupt is only added if CPU- or wall-time are enabled BUT
-        // wall-time is not expected to ever be disabled, except in testing,
-        // and we don't need to optimize for that.
-        if system_settings.profiling_enabled {
+        if system_settings.profiling_wall_time_enabled
+            | system_settings.profiling_experimental_cpu_time_enabled
+        {
             if let Some(profiler) = Profiler::get() {
                 let interrupt = VmInterrupt {
                     interrupt_count_ptr: &locals.interrupt_count,

profiling/src/php_ffi.c

@@ -5,6 +5,7 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
+#include <Zend/zend_vm.h>
 #include "SAPI.h"
 
 #if CFG_STACK_WALKING_TESTS
@@ -88,6 +89,18 @@ sapi_request_info datadog_sapi_globals_request_info() {
  */
 uint32_t ddog_php_prof_php_version_id(void) { return php_version_id(); }
 
+zend_result ddog_php_prof_check_tailcall_vm_interrupt(void) {
+#if PHP_VERSION_ID >= 80500 && PHP_VERSION_ID < 80600
+    uint32_t version_id = php_version_id();
+    // See this PR for more information on the tailcall VM crash:
+    // https://github.com/php/php-src/pull/21922
+    if (zend_vm_kind() == ZEND_VM_KIND_TAILCALL && version_id < 80507) {
+        return FAILURE;
+    }
+#endif
+    return SUCCESS;
+}
+
 /**
  * Returns the PHP_VERSION of the engine at run-time, not the version the
  * extension was built against at compile-time.

profiling/src/php_ffi.h

@@ -72,6 +72,12 @@ zend_module_entry *datadog_get_module_entry(const char *str, uintptr_t len);
  */
 void *datadog_php_profiling_vm_interrupt_addr(void);
 
+/**
+ * Detects tailcall VM interrupt bugs where time sample collection must be
+ * disabled.
+ */
+zend_result ddog_php_prof_check_tailcall_vm_interrupt(void);
+
 /**
  * For Code Hotspots, we need the tracer's local root span id and the current
  * span id. This is a cross-product struct, so keep it in sync with tracer's

profiling/src/profiling/sample_type_filter.rs

@@ -41,12 +41,17 @@ impl SampleTypeFilter {
         let mut sample_types_mask = [false; MAX_SAMPLE_TYPES];
 
         if system_settings.profiling_enabled {
-            // sample, wall-time, cpu-time
-            let len = 2 + system_settings.profiling_experimental_cpu_time_enabled as usize;
-            sample_types.extend_from_slice(&SAMPLE_TYPES[0..len]);
-            sample_types_mask[0] = true;
-            sample_types_mask[1] = true;
-            sample_types_mask[2] = system_settings.profiling_experimental_cpu_time_enabled;
+            if system_settings.profiling_wall_time_enabled {
+                // sample, wall-time
+                sample_types.extend_from_slice(&SAMPLE_TYPES[0..2]);
+                sample_types_mask[0] = true;
+                sample_types_mask[1] = true;
+            }
+
+            if system_settings.profiling_experimental_cpu_time_enabled {
+                sample_types.push(SAMPLE_TYPES[2]);
+                sample_types_mask[2] = true;
+            }
 
             // alloc-samples, alloc-size
             if system_settings.profiling_allocation_enabled {
@@ -219,6 +224,36 @@ mod tests {
         assert_eq!(values, vec![10, 20, 30]);
     }
 
+    #[test]
+    fn filter_without_wall_or_cpu_time() {
+        let mut settings = get_system_settings();
+        settings.profiling_enabled = true;
+        settings.profiling_wall_time_enabled = false;
+        settings.profiling_experimental_cpu_time_enabled = false;
+
+        let sample_type_filter = SampleTypeFilter::new(&settings);
+        let values = sample_type_filter.filter(get_samples());
+        let types = sample_type_filter.sample_types();
+
+        assert_eq!(types, Vec::<ValueType>::new());
+        assert_eq!(values, Vec::<i64>::new());
+    }
+
+    #[test]
+    fn filter_with_cpu_time_and_without_wall_time() {
+        let mut settings = get_system_settings();
+        settings.profiling_enabled = true;
+        settings.profiling_wall_time_enabled = false;
+        settings.profiling_experimental_cpu_time_enabled = true;
+
+        let sample_type_filter = SampleTypeFilter::new(&settings);
+        let values = sample_type_filter.filter(get_samples());
+        let types = sample_type_filter.sample_types();
+
+        assert_eq!(types, vec![ValueType::new("cpu-time", "nanoseconds")]);
+        assert_eq!(values, vec![30]);
+    }
+
     #[test]
     fn filter_with_allocations() {
         let mut settings = get_system_settings();
@@ -242,6 +277,28 @@ mod tests {
         assert_eq!(values, vec![10, 20, 40, 50]);
     }
 
+    #[test]
+    fn filter_with_allocations_without_wall_or_cpu_time() {
+        let mut settings = get_system_settings();
+        settings.profiling_enabled = true;
+        settings.profiling_allocation_enabled = true;
+        settings.profiling_wall_time_enabled = false;
+        settings.profiling_experimental_cpu_time_enabled = false;
+
+        let sample_type_filter = SampleTypeFilter::new(&settings);
+        let values = sample_type_filter.filter(get_samples());
+        let types = sample_type_filter.sample_types();
+
+        assert_eq!(
+            types,
+            vec![
+                ValueType::new("alloc-samples", "count"),
+                ValueType::new("alloc-size", "bytes"),
+            ]
+        );
+        assert_eq!(values, vec![40, 50]);
+    }
+
     #[test]
     fn filter_with_allocations_and_cpu_time() {
         let mut settings = get_system_settings();

profiling/src/wall_time.rs

@@ -110,7 +110,10 @@ static mut PREV_INTERRUPT_FUNCTION: Option<VmInterruptFn> = None;
 #[inline(never)]
 pub extern "C" fn ddog_php_prof_interrupt_function(execute_data: *mut zend_execute_data) {
     let result = REQUEST_LOCALS.try_with_borrow(|locals| {
-        if !locals.system_settings().profiling_enabled {
+        let system_settings = locals.system_settings();
+        if !(system_settings.profiling_wall_time_enabled
+            | system_settings.profiling_experimental_cpu_time_enabled)
+        {
             return;
         }
 

profiling/tests/phpt/phpinfo_07.phpt

@@ -40,26 +40,37 @@ foreach ($lines as $line) {
 // Check that Version exists, but not its value
 assert(isset($values["Version"]));
 
-// Check exact values for this set
+$tailcall_vm_interrupt_workaround_applies =
+    PHP_VERSION_ID >= 80500 &&
+    PHP_VERSION_ID < 80507 &&
+    defined('ZEND_VM_KIND') &&
+    ZEND_VM_KIND === 'ZEND_VM_KIND_TAILCALL';
+
+$cpu_time_expected = $tailcall_vm_interrupt_workaround_applies
+    ? "false"
+    : "true (all experimental features enabled)";
+
+// Check exact values for this set.
 $sections = [
     ["Profiling Enabled", "true"],
     ["Profiling Experimental Features Enabled", "true"],
-    ["Experimental CPU Time Profiling Enabled", "true (all experimental features enabled)"],
+    ["Experimental CPU Time Profiling Enabled", $cpu_time_expected],
     ["Allocation Profiling Enabled", "false"],
     ["Exception Profiling Enabled", "false"],
     ["Timeline Enabled", "false"],
     ["I/O Profiling Enabled", "true"],
 ];
 
 foreach ($sections as [$key, $expected]) {
+    $actual = $values[$key] ?? null;
     assert(
-        $values[$key] === $expected,
-        "Expected '{$expected}', found '{$values[$key]}', for key '{$key}'"
+        $actual === $expected,
+        "Expected '{$expected}', found '{$actual}', for key '{$key}'"
     );
 }
 
 echo "Done.";
 
 ?>
 --EXPECT--
-Done.
+Done.