-
Notifications
You must be signed in to change notification settings - Fork 180
fix(prof): Add running PHP language tests for profiler and fix discovered bugs #3364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
realFlowControl
wants to merge
28
commits into
master
Choose a base branch
from
florian/run-language-tests-for-profiler
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
21a9ae0
run language tests for profiler
realFlowControl c5b0c18
fix profiler language test CI build
realFlowControl 27eeeed
stabilize profiler language test matrix
realFlowControl 689f6a0
profiling: xfail curl_setopt_ssl.phpt in PHP language tests
realFlowControl 95c505f
profiling: set O_CLOEXEC on duplicated stderr fd for logging
realFlowControl 458bd76
profiling: xfail allow_url_include deprecation-ordering tests
realFlowControl 2f6207a
profiling: add ZTS-only xfail list for PHP language tests
realFlowControl bf9ebc9
ci: remove rdkafka/memcached ini from active PHP scan dir in language…
realFlowControl e00f175
profiling: expand ZTS xfail list with all deprecation-ordering tests
realFlowControl 09096e1
profiling: xfail remaining ZTS startup-warning-ordering tests
realFlowControl 9074bec
profiling: document PHP language test xfail lists
realFlowControl 45f4119
profiling: fix SKIP_ONLINE_TESTS typo, drop online tests from xfail
realFlowControl 1f06966
profiling: trim tests README
realFlowControl 80e0128
profiling: drop bug48203_multi from language test xfail list
realFlowControl dfc13d3
profiling: drop parallel ext in language tests instead of xfailing ZTS
realFlowControl 291b95c
profiling: xfail opcache optimizer tests on PHP < 8.4
realFlowControl 11d2ea0
profiling: guard against NULL file in timeline error observer
realFlowControl 1d3fa3c
profiling: xfail bug60634 session test (parallel-run flake)
realFlowControl 9d25d22
ci(profiler): install fixed parallel 1.2.14 in UBSAN job (temporary)
realFlowControl 414c2e5
profiling: block all (non-fault) signals on helper threads
realFlowControl 4ee700f
profiling: drop INVESTIGATE-opcache-do_icall.md
realFlowControl d98ec11
profiling: use per-version xfail lists for PHP language tests
realFlowControl 44b3da8
Merge branch 'master' into florian/run-language-tests-for-profiler
morrisonlevi 21a9a8f
profiling: replace per-version xfail symlinks with a version check
realFlowControl 7d0a905
profiling: fail the language tests job if the profiler isn't loaded
realFlowControl c452896
fix k8n job requests
realFlowControl 34e9e26
fix comment
realFlowControl 08a3d7a
Merge branch 'master' into florian/run-language-tests-for-profiler
realFlowControl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # PHP language test xfail lists | ||
|
|
||
| The profiler's "PHP language tests" CI job runs the upstream PHP test suite | ||
| with the profiling extension loaded, for every supported PHP version and for | ||
| both the `nts` and `zts` builds. These lists exclude tests that cannot pass in | ||
| that environment for reasons unrelated to profiler correctness. | ||
|
|
||
| `.gitlab/run_php_language_tests.sh` **deletes** every `.phpt` named in | ||
| `XFAIL_LIST` before running, so listing a test means "do not run" it. | ||
|
|
||
| | File | Applies to | | ||
| |------|------------| | ||
| | `php-language-xfail.list` | all profiler runs (`nts` + `zts`, all versions) | | ||
| | `php-language-xfail-pre84.list` | PHP < 8.4 (appended by the job via a version check) | | ||
|
|
||
| ## `php-language-xfail.list` (all versions) | ||
|
|
||
| Fail with the profiler loaded regardless of version/flavour: | ||
|
|
||
| - `ext/ffi/tests/list.phpt` — aborts (`free(): invalid size`); allocation | ||
| profiler conflicts with the test's FFI memory management. | ||
| - `Zend/tests/concat_003.phpt` — perf-sensitive (2 s budget); allocation | ||
| profiling overhead can exceed it on CI runners. | ||
| - `ext/session/tests/bug60634.phpt` (also under `user_session_module/` on some | ||
| versions; both paths are listed) — `die()` inside a session save handler. | ||
| Fails intermittently in the parallel run with "Cannot call session save | ||
| handler in a recursive manner". Not a profiler issue: it passes in isolation | ||
| with the profiler enabled; it's a concurrency/session-save-path collision in | ||
| the 64-worker run. Listed because it is flaky under parallelism. | ||
|
|
||
| ## `php-language-xfail-pre84.list` (PHP < 8.4) | ||
|
|
||
| `php-language-xfail-pre84.list` contains opcache optimizer-output tests that | ||
| fail only with the profiler on PHP ≤ 8.3. On PHP < 8.4 the profiler overrides | ||
| `zend_execute_internal` (to handle VM interrupts while an internal function is | ||
| on the stack); on 8.4+ that hook is not installed (frameless calls), so these | ||
| pass. Internal calls therefore compile to `DO_FCALL` instead of `DO_ICALL`, | ||
| changing the optimized opcodes. | ||
|
|
||
| - `opt/prop_types.phpt`, `opt/gh11170.phpt`, `opt/nullsafe_002.phpt` — cosmetic | ||
| opcode-dump differences (`DO_ICALL` → `DO_FCALL`). | ||
| - `bug66251.phpt` — same `< 8.4` condition: with the execute hook installed, | ||
| opcache folds a same-file runtime constant that should stay dynamic. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| ext/opcache/tests/opt/prop_types.phpt | ||
| ext/opcache/tests/opt/gh11170.phpt | ||
| ext/opcache/tests/opt/nullsafe_002.phpt | ||
| ext/opcache/tests/bug66251.phpt |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Zend/tests/concat_003.phpt | ||
| ext/ffi/tests/list.phpt | ||
| ext/session/tests/bug60634.phpt | ||
| ext/session/tests/user_session_module/bug60634.phpt |
77 changes: 77 additions & 0 deletions
77
profiling/tests/phpt/pcntl_async_signal_helper_thread.phpt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| --TEST-- | ||
| [profiling] async PHP signal handlers must not run on profiler helper threads | ||
| --DESCRIPTION-- | ||
| Regression test for a crash when a PHP script installs an async signal handler | ||
| (pcntl_async_signals(true) + pcntl_signal()) for a signal the Zend Engine does | ||
| not itself register, e.g. SIGCHLD. | ||
|
|
||
| The profiler's helper threads (`ddprof_time`, `ddprof_upload`) only masked the | ||
| fixed set of signals the Zend Engine uses, so the kernel could deliver SIGCHLD | ||
| to one of them. pcntl's handler then ran on a thread with no valid PHP/TSRM | ||
| context and dereferenced the thread-local PCNTL_G, segfaulting. The fix is to | ||
| block every (non-fault) signal on the helper threads so async signals are | ||
| delivered to a PHP thread. | ||
|
|
||
| Only crashes on ZTS, where PCNTL_G is thread-local. Observed on PHP 8.4 ZTS, | ||
| reproduced by ext/pcntl/tests/waiting_on_sigchild_pcntl_wait.phpt: | ||
|
|
||
| Thread 2 "ddprof_time" received signal SIGSEGV, Segmentation fault. | ||
| #0 pcntl_signal_handler (signo=17, ...) at ext/pcntl/pcntl.c:1289 | ||
| 1289 struct php_pcntl_pending_signal *psig = PCNTL_G(spares); | ||
| #1 <signal handler called> | ||
| #2 syscall () | ||
| #3 std::sys::pal::unix::futex::futex_wait () | ||
| #4 std::sys::sync::thread_parking::futex::Parker::park_timeout () | ||
| ... | ||
| #15 run () at profiling/src/profiling/uploader.rs:157 | ||
| #16 {closure#3} () at profiling/src/profiling/mod.rs:898 | ||
| #17 ... at profiling/src/profiling/thread_utils.rs:45 | ||
| --SKIPIF-- | ||
| <?php | ||
| foreach (['datadog-profiling', 'pcntl'] as $extension) | ||
| if (!extension_loaded($extension)) | ||
| echo "skip: test requires {$extension}\n"; | ||
| if (!ZEND_THREAD_SAFE) | ||
| echo "skip: ZTS only (the crash is a thread-local PCNTL_G access from a helper thread)\n"; | ||
| if (PHP_OS_FAMILY !== 'Linux') | ||
| echo "skip: Linux only\n"; | ||
| if (getenv('SKIP_ASAN')) | ||
| die('skip: the profiler leaks on purpose in child of a fork'); | ||
| ?> | ||
| --ENV-- | ||
| DD_PROFILING_ENABLED=yes | ||
| DD_PROFILING_LOG_LEVEL=off | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| pcntl_async_signals(true); | ||
|
|
||
| $processes = []; | ||
| pcntl_signal(SIGCHLD, function () use (&$processes) { | ||
| while (($pid = pcntl_wait($status, WUNTRACED | WNOHANG)) > 0) { | ||
| unset($processes[$pid]); | ||
| } | ||
| }); | ||
|
|
||
| // Spawn bursts of short-lived children. They exit at roughly the same time, so | ||
| // a burst of SIGCHLD arrives while the main thread is idle in usleep(). If the | ||
| // profiler's helper threads do not block SIGCHLD the kernel may deliver it to | ||
| // one of them, where pcntl's handler runs without a PHP/TSRM context and | ||
| // crashes. Several rounds widen the (timing-dependent) race window. | ||
| for ($round = 0; $round < 4; $round++) { | ||
| for ($i = 0; $i < 8; $i++) { | ||
| $proc = proc_open('sleep 0.3', [], $pipes); | ||
| if ($proc !== false) { | ||
| $processes[proc_get_status($proc)['pid']] = $proc; | ||
| } | ||
| } | ||
| $iters = 50; | ||
| while (!empty($processes) && $iters-- > 0) { | ||
| usleep(100000); | ||
| } | ||
| } | ||
|
|
||
| echo empty($processes) ? "OK\n" : "leftover children\n"; | ||
| ?> | ||
| --EXPECT-- | ||
| OK |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.