From 33452a65e97ebbfbd0dd5ede7907baae7383ba5a Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 13 Jan 2026 07:03:27 +0100 Subject: [PATCH 1/3] Revert "test(profiling): fix linking by removing recently added test (#3570)" This reverts commit 4a26042074e6d7b7970192ac32b49dafcf825da1. --- profiling/src/php_ffi.c | 30 ++++++++++- profiling/src/profiling/stack_walking.rs | 65 ++++++++++++++++++++++-- 2 files changed, 90 insertions(+), 5 deletions(-) diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index fd5d07a1755..ad01ae2b41a 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -390,7 +390,7 @@ uintptr_t *ddog_test_php_prof_function_run_time_cache(zend_function const *func) } #endif -#if CFG_STACK_WALKING_TESTS +#if CFG_STACK_WALKING_TESTS || defined(CFG_TEST) static int (*og_snprintf)(char *, size_t, const char *, ...); // "weak" let's us polyfill, needed by zend_string_init(..., persistent: 1). @@ -463,7 +463,33 @@ void ddog_php_test_free_fake_zend_execute_data(zend_execute_data *execute_data) free(execute_data); } -#endif + +zend_function *ddog_php_test_create_fake_zend_function_with_name_len(size_t len) { + zend_op_array *op_array = calloc(1, sizeof(zend_function)); + if (!op_array) return NULL; + + op_array->type = ZEND_USER_FUNCTION; + + if (len > 0) { + op_array->function_name = zend_string_alloc(len, true); + if (!op_array->function_name) { + free(op_array); + return NULL; + } + memset(ZSTR_VAL(op_array->function_name), 'x', len); + ZSTR_VAL(op_array->function_name)[len] = '\0'; + } + + return (zend_function *)op_array; +} + +void ddog_php_test_free_fake_zend_function(zend_function *func) { + if (!func) return; + + free(func->common.function_name); + free(func); +} +#endif // CFG_STACK_WALKING_TESTS || CFG_TEST void *opcache_handle = NULL; diff --git a/profiling/src/profiling/stack_walking.rs b/profiling/src/profiling/stack_walking.rs index 04a8babfca0..6f38b74dbb1 100644 --- a/profiling/src/profiling/stack_walking.rs +++ b/profiling/src/profiling/stack_walking.rs @@ -528,14 +528,20 @@ mod detail { pub use detail::*; -// todo: this should be feature = "stack_walking_tests" but it seemed to -// cause a failure in CI to migrate it. -#[cfg(all(test, stack_walking_tests))] +#[cfg(test)] mod tests { use super::*; use crate::bindings as zend; + extern "C" { + fn ddog_php_test_create_fake_zend_function_with_name_len( + len: libc::size_t, + ) -> *mut zend::zend_function; + fn ddog_php_test_free_fake_zend_function(func: *mut zend::zend_function); + } + #[test] + #[cfg(stack_walking_tests)] fn test_collect_stack_sample() { unsafe { let fake_execute_data = zend::ddog_php_test_create_fake_zend_execute_data(3); @@ -560,4 +566,57 @@ mod tests { zend::ddog_php_test_free_fake_zend_execute_data(fake_execute_data); } } + + #[test] + fn test_extract_function_name_short_string() { + unsafe { + let func = ddog_php_test_create_fake_zend_function_with_name_len(10); + assert!(!func.is_null()); + + let name = extract_function_name(&*func).expect("should extract name"); + assert_eq!(name, "xxxxxxxxxx"); + + ddog_php_test_free_fake_zend_function(func); + } + } + + #[test] + fn test_extract_function_name_at_limit_minus_one() { + unsafe { + let func = ddog_php_test_create_fake_zend_function_with_name_len(STR_LEN_LIMIT - 1); + assert!(!func.is_null()); + + let name = extract_function_name(&*func).expect("should extract name"); + assert_eq!(name.len(), STR_LEN_LIMIT - 1); + assert_ne!(name, COW_LARGE_STRING); + + ddog_php_test_free_fake_zend_function(func); + } + } + + #[test] + fn test_extract_function_name_at_limit() { + unsafe { + let func = ddog_php_test_create_fake_zend_function_with_name_len(STR_LEN_LIMIT); + assert!(!func.is_null()); + + let name = extract_function_name(&*func).expect("should return large string marker"); + assert_eq!(name, COW_LARGE_STRING); + + ddog_php_test_free_fake_zend_function(func); + } + } + + #[test] + fn test_extract_function_name_over_limit() { + unsafe { + let func = ddog_php_test_create_fake_zend_function_with_name_len(STR_LEN_LIMIT + 1000); + assert!(!func.is_null()); + + let name = extract_function_name(&*func).expect("should return large string marker"); + assert_eq!(name, COW_LARGE_STRING); + + ddog_php_test_free_fake_zend_function(func); + } + } } From 3fd5e8bf8e2fae0cc96d4c879115fe7cd402b9c4 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Tue, 13 Jan 2026 07:43:54 +0100 Subject: [PATCH 2/3] Introduce `test_zend_string_create()` for tests Co-authored-by: Claude Opus 4.5 --- profiling/src/php_ffi.c | 52 +++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index ad01ae2b41a..4a594221247 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -393,14 +393,36 @@ uintptr_t *ddog_test_php_prof_function_run_time_cache(zend_function const *func) #if CFG_STACK_WALKING_TESTS || defined(CFG_TEST) static int (*og_snprintf)(char *, size_t, const char *, ...); -// "weak" let's us polyfill, needed by zend_string_init(..., persistent: 1). -void *__attribute__((weak)) __zend_malloc(size_t len) { - void *tmp = malloc(len); - if (EXPECTED(tmp || !len)) { - return tmp; +// Manually create a zend_string without using zend_string_init(), since we +// do not link against PHP at test time +static zend_string *test_zend_string_create(const char *str, size_t len) { + // zend_string has a flexible array member val[1], so we allocate + // sizeof(zend_string) - 1 (for the val[1]) + len + 1 (for null terminator) + zend_string* zs = calloc(1, sizeof(zend_string) + len); + if (!zs) { + return NULL; } - fprintf(stderr, "Out of memory\n"); - exit(1); + + // Initialize the refcounted header +#if PHP_VERSION_ID < 70299 + GC_REFCOUNT(zs) = 1; +#else + GC_SET_REFCOUNT(zs, 1); +#endif +#if PHP_VERSION_ID < 70499 + GC_TYPE_INFO(zs) = IS_STRING; +#else + GC_TYPE_INFO(zs) = GC_STRING; +#endif + + zs->h = 0; + zs->len = len; + if (len > 0) { + memcpy(ZSTR_VAL(zs), str, len); + } + ZSTR_VAL(zs)[len] = '\0'; + + return zs; } static zend_execute_data *create_fake_frame(int depth) { @@ -412,11 +434,11 @@ static zend_execute_data *create_fake_frame(int depth) { char buffer[64] = {0}; int len = og_snprintf(buffer, sizeof buffer, "function name %03d", depth) + 1; ZEND_ASSERT(len >= 0 && sizeof buffer > (size_t)len); - op_array->function_name = zend_string_init(buffer, len - 1, true); + op_array->function_name = test_zend_string_create(buffer, len - 1); len = og_snprintf(buffer, sizeof buffer, "filename-%03d.php", depth) + 1; ZEND_ASSERT(len >= 0 && sizeof buffer > (size_t)len); - op_array->filename = zend_string_init(buffer, len - 1, true); + op_array->filename = test_zend_string_create(buffer, len - 1); return execute_data; } @@ -471,13 +493,19 @@ zend_function *ddog_php_test_create_fake_zend_function_with_name_len(size_t len) op_array->type = ZEND_USER_FUNCTION; if (len > 0) { - op_array->function_name = zend_string_alloc(len, true); + char *buffer = malloc(len + 1); + if (!buffer) { + free(op_array); + return NULL; + } + memset(buffer, 'x', len); + buffer[len] = '\0'; + op_array->function_name = test_zend_string_create(buffer, len); + free(buffer); if (!op_array->function_name) { free(op_array); return NULL; } - memset(ZSTR_VAL(op_array->function_name), 'x', len); - ZSTR_VAL(op_array->function_name)[len] = '\0'; } return (zend_function *)op_array; From 63decfdccb38304b2282eb4ea0beba4e58dd02f5 Mon Sep 17 00:00:00 2001 From: Levi Morrison Date: Fri, 16 Jan 2026 11:55:31 -0700 Subject: [PATCH 3/3] fix: test_zend_string_create and extract test_zend_string_alloc --- profiling/src/php_ffi.c | 71 +++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/profiling/src/php_ffi.c b/profiling/src/php_ffi.c index 4a594221247..698d8775080 100644 --- a/profiling/src/php_ffi.c +++ b/profiling/src/php_ffi.c @@ -393,14 +393,28 @@ uintptr_t *ddog_test_php_prof_function_run_time_cache(zend_function const *func) #if CFG_STACK_WALKING_TESTS || defined(CFG_TEST) static int (*og_snprintf)(char *, size_t, const char *, ...); -// Manually create a zend_string without using zend_string_init(), since we -// do not link against PHP at test time -static zend_string *test_zend_string_create(const char *str, size_t len) { - // zend_string has a flexible array member val[1], so we allocate - // sizeof(zend_string) - 1 (for the val[1]) + len + 1 (for null terminator) - zend_string* zs = calloc(1, sizeof(zend_string) + len); +static ZEND_COLD ZEND_NORETURN void out_of_memory(void) { + fprintf(stderr, "Out of memory\n"); + exit(1); +} + +static zend_string *test_zend_string_alloc(size_t len) { + // We don't need to handle large strings. + if (len > INT32_MAX) { + out_of_memory(); + } + + // zend_string logically has a flexible array member val, but it uses the + // so-called "struct hack" instead. So we calculate the number of bytes + // needed to get to `val`, then add the `len` plus 1 (for a null byte). + uint32_t alloc_len = offsetof(zend_string, val) + len + 1; + + // Need to round up to 8 bytes on 64-bit platforms, won't overflow because + // of above large string check. + uint32_t rounded = (alloc_len + UINT32_C(7)) & ~UINT32_C(7); + zend_string *zs = calloc(rounded, 1); if (!zs) { - return NULL; + out_of_memory(); } // Initialize the refcounted header @@ -409,22 +423,35 @@ static zend_string *test_zend_string_create(const char *str, size_t len) { #else GC_SET_REFCOUNT(zs, 1); #endif -#if PHP_VERSION_ID < 70499 - GC_TYPE_INFO(zs) = IS_STRING; -#else - GC_TYPE_INFO(zs) = GC_STRING; + +#if PHP_VERSION_ID < 70200 +#undef GC_FLAGS_SHIFT +#define GC_FLAGS_SHIFT 8 +#endif + +#if PHP_VERSION_ID < 80000 +#undef GC_STRING +#define GC_STRING IS_STRING #endif + // IS_STR_PERSISTENT means it's allocated with malloc, not emalloc. + GC_TYPE_INFO(zs) = GC_STRING | (IS_STR_PERSISTENT << GC_FLAGS_SHIFT); + zs->h = 0; zs->len = len; - if (len > 0) { - memcpy(ZSTR_VAL(zs), str, len); - } ZSTR_VAL(zs)[len] = '\0'; return zs; } +// Manually create a zend_string without using zend_string_init(), since we +// do not link against PHP at test time +static zend_string *test_zend_string_create(const char *str, size_t len) { + zend_string *zstr = test_zend_string_alloc(len); + memcpy(ZSTR_VAL(zstr), str, len); + return zstr; +} + static zend_execute_data *create_fake_frame(int depth) { zend_execute_data *execute_data = calloc(1, sizeof(zend_execute_data)); zend_op_array *op_array = calloc(1, sizeof(zend_function)); @@ -493,19 +520,9 @@ zend_function *ddog_php_test_create_fake_zend_function_with_name_len(size_t len) op_array->type = ZEND_USER_FUNCTION; if (len > 0) { - char *buffer = malloc(len + 1); - if (!buffer) { - free(op_array); - return NULL; - } - memset(buffer, 'x', len); - buffer[len] = '\0'; - op_array->function_name = test_zend_string_create(buffer, len); - free(buffer); - if (!op_array->function_name) { - free(op_array); - return NULL; - } + zend_string *zstr = test_zend_string_alloc(len); + memset(ZSTR_VAL(zstr), 'x', len); + op_array->function_name = zstr; } return (zend_function *)op_array;