diff --git a/.github/workflows/sanitizers.yaml b/.github/workflows/sanitizers.yaml index 15fe013d5b..f66be45c93 100644 --- a/.github/workflows/sanitizers.yaml +++ b/.github/workflows/sanitizers.yaml @@ -108,7 +108,7 @@ jobs: - name: Set CGO flags run: | { - echo "CGO_CFLAGS=$CFLAGS -I${PWD}/watcher/target/include $(php-config --includes)" + echo "CGO_CFLAGS=$CFLAGS -I${PWD}/watcher/target/include -DFRANKENPHP_TEST_HOOKS $(php-config --includes)" echo "CGO_LDFLAGS=$LDFLAGS $(php-config --ldflags) $(php-config --libs)" } >> "$GITHUB_ENV" - name: Run tests diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 396d9ccf77..6798f7fb73 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -59,7 +59,7 @@ jobs: - name: Install e-dant/watcher uses: ./.github/actions/watcher - name: Set CGO flags - run: echo "CGO_CFLAGS=-I${PWD}/watcher/target/include $(php-config --includes)" >> "${GITHUB_ENV}" + run: echo "CGO_CFLAGS=-I${PWD}/watcher/target/include -DFRANKENPHP_TEST_HOOKS $(php-config --includes)" >> "${GITHUB_ENV}" - name: Build run: go build - name: Build testcli binary @@ -135,7 +135,7 @@ jobs: echo "GEN_STUB_SCRIPT=${PWD}/php-${PHP_VERSION}/build/gen_stub.php" >> "${GITHUB_ENV}" - name: Set CGO flags run: | - echo "CGO_CFLAGS=$(php-config --includes)" >> "${GITHUB_ENV}" + echo "CGO_CFLAGS=-DFRANKENPHP_TEST_HOOKS $(php-config --includes)" >> "${GITHUB_ENV}" echo "CGO_LDFLAGS=$(php-config --ldflags) $(php-config --libs)" >> "${GITHUB_ENV}" - name: Install gotestsum run: go install gotest.tools/gotestsum@latest @@ -172,7 +172,7 @@ jobs: - name: Set CGO flags run: | { - echo "CGO_CFLAGS=-I/opt/homebrew/include/ $(php-config --includes)" + echo "CGO_CFLAGS=-I/opt/homebrew/include/ -DFRANKENPHP_TEST_HOOKS $(php-config --includes)" echo "CGO_LDFLAGS=-L/opt/homebrew/lib/ $(php-config --ldflags) $(php-config --libs)" } >> "${GITHUB_ENV}" - name: Build diff --git a/frankenphp.c b/frankenphp.c index 0cc294e397..22e29b6cab 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -37,6 +37,12 @@ #include "_cgo_export.h" #include "frankenphp_arginfo.h" +#ifdef FRANKENPHP_TEST_HOOKS +/* The persistent_zval helpers are only compiled in when a consumer needs + * them. The step that lands the first real caller (background workers) + * will drop this guard. */ +#include "persistent_zval.h" +#endif #if defined(PHP_WIN32) && defined(ZTS) ZEND_TSRMLS_CACHE_DEFINE() @@ -708,12 +714,55 @@ PHP_FUNCTION(frankenphp_log) { } } +#ifdef FRANKENPHP_TEST_HOOKS +/* Test-only entry point that exercises persistent_zval.h end-to-end: + * validate -> persist (request -> persistent memory) -> + * to_request (persistent -> fresh request memory) -> free persistent copy. + * Compiled only when FRANKENPHP_TEST_HOOKS is defined; never registered + * in production builds. */ +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX( + arginfo_frankenphp_test_persist_roundtrip, 0, 1, IS_MIXED, 0) +ZEND_ARG_TYPE_INFO(0, value, IS_MIXED, 0) +ZEND_END_ARG_INFO() + +PHP_FUNCTION(frankenphp_test_persist_roundtrip) { + zval *input; + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_ZVAL(input) + ZEND_PARSE_PARAMETERS_END(); + + if (!persistent_zval_validate(input)) { + zend_throw_exception(spl_ce_LogicException, + "persistent_zval: value type not supported " + "(only scalars, arrays, and enums are allowed)", + 0); + RETURN_THROWS(); + } + + zval persistent; + persistent_zval_persist(&persistent, input); + persistent_zval_to_request(return_value, &persistent); + persistent_zval_free(&persistent); +} + +static const zend_function_entry frankenphp_test_hook_functions[] = { + PHP_FE(frankenphp_test_persist_roundtrip, + arginfo_frankenphp_test_persist_roundtrip) PHP_FE_END}; +#endif + PHP_MINIT_FUNCTION(frankenphp) { register_frankenphp_symbols(module_number); #ifndef PHP_WIN32 pthread_atfork(NULL, NULL, frankenphp_fork_child); #endif +#ifdef FRANKENPHP_TEST_HOOKS + if (zend_register_functions(NULL, frankenphp_test_hook_functions, NULL, + MODULE_PERSISTENT) == FAILURE) { + return FAILURE; + } +#endif + zend_function *func; // Override putenv diff --git a/persistent_zval.h b/persistent_zval.h new file mode 100644 index 0000000000..204430194a --- /dev/null +++ b/persistent_zval.h @@ -0,0 +1,277 @@ +/* persistent_zval.h - Deep-copy zval trees to and from persistent memory. + * + * Provides a small, self-contained toolkit for moving zval trees across + * thread boundaries. The supported shape is a whitelist: scalars, arrays, + * and enums. Everything else is rejected by persistent_zval_validate so + * callers can fail fast before allocating. + * + * Fast paths: + * - Interned strings: shared memory, no copy. + * - Opcache-immutable arrays: shared pointer, no copy, no free. + * + * Included by frankenphp.c; not a standalone compilation unit. */ + +#ifndef PERSISTENT_ZVAL_H +#define PERSISTENT_ZVAL_H + +#include + +/* Enum payload stored in persistent memory: the class name + case name + * are kept as persistent zend_strings and the case object is re-resolved + * via zend_lookup_class + zend_enum_get_case_cstr on each read. */ +typedef struct { + zend_string *class_name; + zend_string *case_name; +} persistent_zval_enum_t; + +/* Whitelist check: only scalars, arrays of allowed values, and enum + * instances pass. Returns false for objects other than enums, resources, + * closures, references, etc. */ +static bool persistent_zval_validate(zval *z) { + switch (Z_TYPE_P(z)) { + case IS_NULL: + case IS_FALSE: + case IS_TRUE: + case IS_LONG: + case IS_DOUBLE: + case IS_STRING: + return true; + case IS_OBJECT: + return (Z_OBJCE_P(z)->ce_flags & ZEND_ACC_ENUM) != 0; + case IS_ARRAY: { + /* Opcache-immutable arrays are compile-time constants in shared + * memory; their leaves are guaranteed scalars or further immutable + * arrays. The copy/free paths below already trust this flag, so a + * recursive walk here would just be cycles. */ + if ((GC_FLAGS(Z_ARRVAL_P(z)) & IS_ARRAY_IMMUTABLE) != 0) + return true; + zval *val; + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(z), val) { + if (!persistent_zval_validate(val)) + return false; + } + ZEND_HASH_FOREACH_END(); + return true; + } + default: + return false; + } +} + +/* Deep-copy a zval from request memory into persistent (pemalloc) memory. + * Callers must have already passed persistent_zval_validate on src. + * + * Storage convention for enums: dst becomes IS_PTR holding a + * persistent_zval_enum_t. This is an internal representation; the caller + * should never expose a persistent zval to PHP directly, only via + * persistent_zval_to_request. */ +static void persistent_zval_persist(zval *dst, zval *src) { + switch (Z_TYPE_P(src)) { + case IS_NULL: + case IS_FALSE: + case IS_TRUE: + ZVAL_COPY_VALUE(dst, src); + break; + case IS_LONG: + ZVAL_LONG(dst, Z_LVAL_P(src)); + break; + case IS_DOUBLE: + ZVAL_DOUBLE(dst, Z_DVAL_P(src)); + break; + case IS_STRING: { + zend_string *s = Z_STR_P(src); + if (ZSTR_IS_INTERNED(s)) { + ZVAL_STR(dst, s); /* interned strings live process-wide */ + } else { + ZVAL_NEW_STR(dst, zend_string_init(ZSTR_VAL(s), ZSTR_LEN(s), 1)); + } + break; + } + case IS_OBJECT: { + /* Must be an enum (validated earlier). */ + zend_class_entry *ce = Z_OBJCE_P(src); + persistent_zval_enum_t *e = pemalloc(sizeof(*e), 1); + e->class_name = + ZSTR_IS_INTERNED(ce->name) + ? ce->name + : zend_string_init(ZSTR_VAL(ce->name), ZSTR_LEN(ce->name), 1); + zval *case_name_zval = zend_enum_fetch_case_name(Z_OBJ_P(src)); + zend_string *case_str = Z_STR_P(case_name_zval); + e->case_name = + ZSTR_IS_INTERNED(case_str) + ? case_str + : zend_string_init(ZSTR_VAL(case_str), ZSTR_LEN(case_str), 1); + ZVAL_PTR(dst, e); + break; + } + case IS_ARRAY: { + HashTable *src_ht = Z_ARRVAL_P(src); + if ((GC_FLAGS(src_ht) & IS_ARRAY_IMMUTABLE) != 0) { + /* Opcache-immutable arrays live for the process lifetime and are + * safe to share across threads by pointer. Zero-copy, zero-free. */ + ZVAL_ARR(dst, src_ht); + break; + } + HashTable *dst_ht = pemalloc(sizeof(HashTable), 1); + zend_hash_init(dst_ht, zend_hash_num_elements(src_ht), NULL, NULL, 1); + ZVAL_ARR(dst, dst_ht); + + zend_string *key; + zend_ulong idx; + zval *val; + ZEND_HASH_FOREACH_KEY_VAL(src_ht, idx, key, val) { + zval pval; + persistent_zval_persist(&pval, val); + if (key) { + if (ZSTR_IS_INTERNED(key)) { + zend_hash_add_new(dst_ht, key, &pval); + } else { + zend_string *pkey = zend_string_init(ZSTR_VAL(key), ZSTR_LEN(key), 1); + /* Iteration guarantees the source key has its hash set. + * Propagating it lets zend_hash_add_new skip the re-hash. */ + ZSTR_H(pkey) = ZSTR_H(key); + zend_hash_add_new(dst_ht, pkey, &pval); + zend_string_release(pkey); + } + } else { + zend_hash_index_add_new(dst_ht, idx, &pval); + } + } + ZEND_HASH_FOREACH_END(); + break; + } + default: + /* Unreachable: persistent_zval_validate is the gatekeeper. */ + ZEND_UNREACHABLE(); + } +} + +/* Deep-free a persistent zval tree. Idempotent on scalars. Skips + * interned strings and immutable arrays (they are borrowed, not owned). */ +static void persistent_zval_free(zval *z) { + switch (Z_TYPE_P(z)) { + case IS_STRING: + if (!ZSTR_IS_INTERNED(Z_STR_P(z))) { + zend_string_free(Z_STR_P(z)); + } + break; + case IS_PTR: { + persistent_zval_enum_t *e = (persistent_zval_enum_t *)Z_PTR_P(z); + if (!ZSTR_IS_INTERNED(e->class_name)) + zend_string_free(e->class_name); + if (!ZSTR_IS_INTERNED(e->case_name)) + zend_string_free(e->case_name); + pefree(e, 1); + break; + } + case IS_ARRAY: { + HashTable *ht = Z_ARRVAL_P(z); + if ((GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) != 0) { + /* Borrowed from opcache, do not touch. */ + break; + } + zval *val; + ZEND_HASH_FOREACH_VAL(ht, val) { persistent_zval_free(val); } + ZEND_HASH_FOREACH_END(); + zend_hash_destroy(ht); + pefree(ht, 1); + break; + } + default: + break; + } +} + +/* Deep-copy a persistent zval tree back into request memory. Enums are + * resolved from their class+case names on each call. If the enum class + * or case can't be found in the current thread's class table, an + * exception is thrown and dst is set to IS_NULL. */ +static void persistent_zval_to_request(zval *dst, zval *src) { + switch (Z_TYPE_P(src)) { + case IS_NULL: + case IS_FALSE: + case IS_TRUE: + ZVAL_COPY_VALUE(dst, src); + break; + case IS_LONG: + ZVAL_LONG(dst, Z_LVAL_P(src)); + break; + case IS_DOUBLE: + ZVAL_DOUBLE(dst, Z_DVAL_P(src)); + break; + case IS_STRING: + if (ZSTR_IS_INTERNED(Z_STR_P(src))) { + ZVAL_STR(dst, Z_STR_P(src)); + } else { + ZVAL_STRINGL(dst, Z_STRVAL_P(src), Z_STRLEN_P(src)); + } + break; + case IS_PTR: { + persistent_zval_enum_t *e = (persistent_zval_enum_t *)Z_PTR_P(src); + zend_class_entry *ce = zend_lookup_class(e->class_name); + if (EG(exception)) { + /* Autoloader threw; let that exception propagate untouched. */ + ZVAL_NULL(dst); + break; + } + if (!ce || !(ce->ce_flags & ZEND_ACC_ENUM)) { + zend_throw_exception_ex(spl_ce_LogicException, 0, + "persistent_zval: enum class \"%s\" not found", + ZSTR_VAL(e->class_name)); + ZVAL_NULL(dst); + break; + } + zend_object *enum_obj = zend_enum_get_case_cstr(ce, ZSTR_VAL(e->case_name)); + if (!enum_obj) { + zend_throw_exception_ex(spl_ce_LogicException, 0, + "persistent_zval: enum case \"%s::%s\" not found", + ZSTR_VAL(e->class_name), ZSTR_VAL(e->case_name)); + ZVAL_NULL(dst); + break; + } + ZVAL_OBJ_COPY(dst, enum_obj); + break; + } + case IS_ARRAY: { + HashTable *src_ht = Z_ARRVAL_P(src); + if ((GC_FLAGS(src_ht) & IS_ARRAY_IMMUTABLE) != 0) { + /* Zero-copy: immutable arrays are safe to expose directly. */ + ZVAL_ARR(dst, src_ht); + break; + } + array_init_size(dst, zend_hash_num_elements(src_ht)); + HashTable *dst_ht = Z_ARRVAL_P(dst); + + zend_string *key; + zend_ulong idx; + zval *val; + ZEND_HASH_FOREACH_KEY_VAL(src_ht, idx, key, val) { + zval rval; + persistent_zval_to_request(&rval, val); + if (EG(exception)) { + zval_ptr_dtor(&rval); + break; + } + if (key) { + if (ZSTR_IS_INTERNED(key)) { + zend_hash_add_new(dst_ht, key, &rval); + } else { + zend_string *rkey = zend_string_init(ZSTR_VAL(key), ZSTR_LEN(key), 0); + ZSTR_H(rkey) = ZSTR_H(key); + zend_hash_add_new(dst_ht, rkey, &rval); + zend_string_release(rkey); + } + } else { + zend_hash_index_add_new(dst_ht, idx, &rval); + } + } + ZEND_HASH_FOREACH_END(); + break; + } + default: + /* Unreachable: only types produced by persistent_zval_persist land here. */ + ZEND_UNREACHABLE(); + } +} + +#endif /* PERSISTENT_ZVAL_H */ diff --git a/persistent_zval_test.go b/persistent_zval_test.go new file mode 100644 index 0000000000..97d24dacdd --- /dev/null +++ b/persistent_zval_test.go @@ -0,0 +1,50 @@ +// Tests for the persistent_zval.h helpers. The exercising PHP function +// frankenphp_test_persist_roundtrip is only registered when the +// FRANKENPHP_TEST_HOOKS preprocessor flag is set. Without the flag, the +// PHP fixture prints a SKIP line and the test skips so production builds +// stay clean. + +package frankenphp_test + +import ( + "errors" + "io" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/dunglas/frankenphp" + "github.com/stretchr/testify/require" +) + +func TestPersistentZvalRoundtrip(t *testing.T) { + cwd, _ := os.Getwd() + testDataDir := cwd + "/testdata/" + + require.NoError(t, frankenphp.Init()) + t.Cleanup(frankenphp.Shutdown) + + req := httptest.NewRequest("GET", "http://example.com/persist-roundtrip.php", nil) + fr, err := frankenphp.NewRequestWithContext(req, frankenphp.WithRequestDocumentRoot(testDataDir, false)) + require.NoError(t, err) + + w := httptest.NewRecorder() + if err := frankenphp.ServeHTTP(w, fr); err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { + t.Fatalf("serve: %v", err) + } + + body, _ := io.ReadAll(w.Result().Body) + out := string(body) + + if strings.Contains(out, "SKIP") { + t.Skip("FRANKENPHP_TEST_HOOKS not set; skipping persistent_zval roundtrip tests") + } + + require.NotContains(t, out, "FAIL", "persist-roundtrip.php reported a failure:\n"+out) + require.Contains(t, out, "OK null") + require.Contains(t, out, "OK enum active") + require.Contains(t, out, "OK stdClass rejected") + require.Contains(t, out, "OK resource rejected") + require.Contains(t, out, "OK nested stdClass rejected") +} diff --git a/testdata/persist-roundtrip.php b/testdata/persist-roundtrip.php new file mode 100644 index 0000000000..f2bcec0627 --- /dev/null +++ b/testdata/persist-roundtrip.php @@ -0,0 +1,88 @@ + 'alice', + 'age' => 30, + 'tags' => ['admin', 'editor'], + 'meta' => ['created' => 1234567890, 'flags' => [true, false, null]], + 0 => 'first', + 1 => 'second', +]; +same($rt($arr), $arr, 'nested array'); + +// Enum roundtrip: identity (===) must be preserved because the enum is +// re-resolved to the same singleton case on the read side. +same($rt(Status::Active), Status::Active, 'enum active'); +same($rt(Status::Paused), Status::Paused, 'enum paused'); + +// Array containing an enum. +same( + $rt(['status' => Status::Active, 'count' => 7]), + ['status' => Status::Active, 'count' => 7], + 'array with enum', +); + +// Invalid inputs throw LogicException. +try { + $rt(new stdClass()); + echo "FAIL stdClass should throw\n"; +} catch (\LogicException) { + echo "OK stdClass rejected\n"; +} + +try { + $rt(fopen('php://memory', 'r')); + echo "FAIL resource should throw\n"; +} catch (\LogicException) { + echo "OK resource rejected\n"; +} + +try { + $rt(['ok' => 1, 'bad' => new stdClass()]); + echo "FAIL nested stdClass should throw\n"; +} catch (\LogicException) { + echo "OK nested stdClass rejected\n"; +}