fix(tracing): sandbox should save/restore jit_trace_num#3964
Conversation
|
✨ Fix all issues with BitsAI or with Cursor
|
bwoebi
left a comment
There was a problem hiding this comment.
LGTM assuming CI passes.
Benchmarks [ tracer ]Benchmark execution time: 2026-06-08 21:51:07 Comparing candidate commit a175a4a in PR branch Found 0 performance improvements and 6 performance regressions! Performance is the same for 188 metrics, 0 unstable metrics.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a175a4a857
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if ($bailoutEnabled && !$bailed) { | ||
| $bailed = true; | ||
| trigger_error('force sandbox bailout after entering a JIT trace', E_USER_ERROR); |
There was a problem hiding this comment.
Avoid using E_USER_ERROR for the bailout regression
On PHP 8.4+ this trigger_error(..., E_USER_ERROR) first emits the new deprecation for using E_USER_ERROR; because the sandbox installs EH_THROW, that deprecation is converted into an exception before the fatal error bailout is reached. In those environments the test can still print ok, but it no longer exercises the zend_bailout() path or the restored EG(jit_trace_num), so the regression this test is meant to catch would slip through.
Useful? React with 👍 / 👎.
Description
Fixes a PHP 8 tracing-JIT crash after sandboxed hook bailouts.
AppSec/RASP can trigger a
zend_bailout()when it blocks a request during a hooked call. That bailout skips PHP’s normalEG(jit_trace_num)restore inzend_call_function(). ddtrace’s sandbox restoredcurrent_execute_data, but notjit_trace_num, leaving the JIT with a stale trace id.A later JIT side exit could then read exit metadata from the wrong trace and crash.
Reviewer checklist