feat(opcache): give the OPcache surface real behaviour - #968
Guikingone wants to merge 4 commits into
Conversation
daa76bd to
d3e5419
Compare
11db621 to
1146bf6
Compare
62ada12 to
f910a42
Compare
ab6cc20 to
5dd6e89
Compare
 |
| let bytes = std::fs::read(entry_path(&dir, canonical)).ok()?; | ||
| let cached: CacheFile = bincode::deserialize(&bytes).ok()?; |
There was a problem hiding this comment.
If another user can write to the configured shared cache directory, they can place a cache entry under the predictable path hash with matching public header fields. load() fully deserializes that entry before checking its magic, version, source identity, or provenance, and the returned ScriptSegment::Code is then executed as the victim script. The cache directory must be private and trusted, or entries must be authenticated before deserialization; decoding should also be bounded so malformed length fields cannot exhaust a worker.
How this was verified: Cache bytes are deserialized at line 106 before the checks at lines 107–115, and accepted segments flow directly into the include interpreter.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/elephc-magician/src/script_cache/file_store.rs
Line: 105-106
Comment:
**Untrusted cache code execution**
If another user can write to the configured shared cache directory, they can place a cache entry under the predictable path hash with matching public header fields. `load()` fully deserializes that entry before checking its magic, version, source identity, or provenance, and the returned `ScriptSegment::Code` is then executed as the victim script. The cache directory must be private and trusted, or entries must be authenticated before deserialization; decoding should also be bounded so malformed length fields cannot exhaust a worker.
**How this was verified:** Cache bytes are deserialized at line 106 before the checks at lines 107–115, and accepted segments flow directly into the include interpreter.
**Knowledge Base Used:**
- [Runtime preludes and core services](https://app.greptile.com/illegal-studio/-/custom-context/knowledge-base/illegalstudio/elephc/-/docs/runtime-preludes-and-core-services.md)
- [Tests, fixtures, and compatiblity coverage](https://app.greptile.com/illegal-studio/-/custom-context/knowledge-base/illegalstudio/elephc/-/docs/tests-fixtures-and-compatibility.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
The control flow is as described — I checked rather than taking it on trust. load() does:
let bytes = std::fs::read(entry_path(&dir, canonical)).ok()?;
let cached: CacheFile = bincode::deserialize(&bytes).ok()?; // decode
if cached.magic != MAGIC || cached.format_version != FORMAT_VERSION { // then check
return None;
}
if cached.path != canonical.to_string_lossy() || cached.mtime != mtime || cached.size != size {
return None;
}The magic, version, path, mtime and size checks are all real and unconditional — but they run
after the decode, so they cannot protect the decode. A hostile entry's length fields are
consumed before anything has been validated, which is the memory-exhaustion half of the finding,
and the identity checks only constrain what is returned, not what is parsed.
Worth separating the two halves, because they need different answers:
- Bounded decode is fixable here and unconditionally worth doing: a size cap on the file and
abincodeconfiguration with a byte limit, so a malformed length cannot make a worker
allocate without bound. That is cheap and independent of any threat model. - Authenticated entries is the larger question, and it turns on whether a shared cache
directory is in scope at all. If the directory is documented as trusted and private — which is
the usual answer, and whatopcache.file_cacheassumes — then the remaining exposure is a
deployment property, and the right fix is to say so and to refuse a world-writable directory
at startup rather than to authenticate every entry.
I would not call the threat model theoretical: shared hosting is exactly a case where another
user can write to a shared directory, and PHP's own opcache file cache has had CVEs of this
shape.
This is your branch rather than mine — I only rebased it onto main and settled the catalog counts
— so I have not changed the cache implementation. Happy to do either half if you want it.
There was a problem hiding this comment.
Agreed — the sequence confirms the finding. These are separate requirements:
- Bounded decoding is unconditional. Reject oversized cache files and configure bincode with a byte limit before accepting any decoded payload. This addresses malformed length fields regardless of deployment assumptions.
- The cache trust boundary must be explicit. If shared or user-writable cache directories are supported, entries need authentication. Otherwise, document the directory as private/trusted and refuse unsafe permissions at startup. Magic, version, path, mtime, and size are integrity checks only after deserialization, and another writer can forge them.
Until bounded decoding is implemented and the shared-directory policy is enforced or clearly excluded, this remains a valid blocking finding.
| let target = entry_path(&dir, canonical); | ||
| let temp = target.with_extension(format!("tmp{}", std::process::id())); | ||
| if std::fs::write(&temp, &bytes).is_err() { |
There was a problem hiding this comment.
If another user can write to the shared cache directory, they can pre-create the predictable <entry>.tmp<pid> path as a symlink to another file writable by the worker. std::fs::write follows that symlink, so the next cache fill truncates and overwrites the target with serialized cache bytes. Create temporary entries atomically with exclusive, no-follow semantics and verify their identity before publishing them.
How this was verified: The temporary filename is derived solely from the deterministic target and process ID, and line 155 opens it through the symlink-following std::fs::write API.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/elephc-magician/src/script_cache/file_store.rs
Line: 153-155
Comment:
**Predictable symlink-following cache write**
If another user can write to the shared cache directory, they can pre-create the predictable `<entry>.tmp<pid>` path as a symlink to another file writable by the worker. `std::fs::write` follows that symlink, so the next cache fill truncates and overwrites the target with serialized cache bytes. Create temporary entries atomically with exclusive, no-follow semantics and verify their identity before publishing them.
**How this was verified:** The temporary filename is derived solely from the deterministic target and process ID, and line 155 opens it through the symlink-following `std::fs::write` API.
**Knowledge Base Used:**
- [Runtime preludes and core services](https://app.greptile.com/illegal-studio/-/custom-context/knowledge-base/illegalstudio/elephc/-/docs/runtime-preludes-and-core-services.md)
- [Tests, fixtures, and compatiblity coverage](https://app.greptile.com/illegal-studio/-/custom-context/knowledge-base/illegalstudio/elephc/-/docs/tests-fixtures-and-compatibility.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Confirmed against the code:
let target = entry_path(&dir, canonical);
let temp = target.with_extension(format!("tmp{}", std::process::id()));
if std::fs::write(&temp, &bytes).is_err() { ... }std::fs::write is File::create, which is O_CREAT|O_WRONLY|O_TRUNC with no O_EXCL and no
O_NOFOLLOW, so it follows a symlink at temp and truncates whatever it points at. The path is
fully predictable: a deterministic entry path plus the PID.
One thing the finding does not say, which makes it slightly worse: the PID is not even a
freshness guarantee. PIDs are reused, and --web prefork means several workers share the
directory, so two processes can collide on the same temp name in the ordinary case — that is a
robustness bug independent of any attacker.
The write-through-temp-then-rename itself is doing its job — the comment is right that a reader
never sees a half-written entry, and rename is atomic. It just is not hardened against a
hostile directory.
The usual shape for this is to create the temp with OpenOptions::new().write(true).create_new(true)
(O_EXCL, so a pre-existing symlink or file fails instead of being followed) and a random suffix
rather than the PID, then rename as today. create_new alone fixes both the symlink follow and
the worker collision.
As on the sibling thread: this is your branch, I only rebased it, so I have not touched the cache
implementation. Say the word and I will.
There was a problem hiding this comment.
Please go ahead with the hardening. Use OpenOptions::create_new(true) for the temporary file and a per-write unique suffix; keep the atomic rename flow unchanged. This addresses both the symlink-following issue and ordinary worker/PID collisions.
5dd6e89 to
eb6f32f
Compare
|
Split this into the part that is a real gap, the part that is inherent, and the part I am not acting on unilaterally. The ordering observation is accurate. The trust conclusion is correct, and it was undocumented — fixed in The page documented the three checks in detail, which is exactly what made this easy to miss — it read as though the entry were fully validated. It now states the requirement plainly and gives the operational rule (own the directory, not group- or world-writable, treat it as a directory of executable PHP), and qualifies the "Authenticate entries before deserialization" is not achievable as stated. There is no key. Any in-band marker an attacker can read, they can also write. Authentication would need a keyed MAC with a secret the cache consumer holds and the attacker does not — a real design, with key management, not a check to add here. If that is wanted, it should be its own issue rather than a change slipped into this PR. "Bound the decoding" is fair and I have NOT done it. A malformed length prefix in a hostile entry can make bincode allocate before any field is inspected — a denial-of-service on a worker, distinct from the code-execution point and not fixed by documentation. It needs a decode limit, which means picking a bound and deciding whether it is configurable; I would rather that be a deliberate choice than something I set to a number of my own choosing on a security thread. Happy to implement it here if you name the policy, or to file it. Leaving this thread open on that last point. |
…hful reporting
Before this branch, elephc reported all 54 OPcache directives with byte-verified
values and answered all 8 API functions — and almost none of it did anything.
Fifteen directives now change what the program does, three shape a reported value,
and the other 36 remain honest reporting.
WHAT NOW EXISTS
A RUNTIME SCRIPT CACHE for the one tier an AOT binary can still grow: files pulled
in by dynamic include/require reached through eval(). The compile-time manifest
stays frozen — that is the "the binary IS the cache" premise — so this is where an
opcode-cache-shaped saving still exists. Measured on the eval-hosted include
benchmark: ~20.6 ns per source byte below the old 64 KiB fragment cap, and the whole
re-parse above it (a 128 KiB include cost 10.98 ms uncached against a ~0.26 ms floor).
AN ON-DISK FILE CACHE behind opcache.file_cache, holding elephc's PARSED form of
those scripts, so a cold process skips the read, the <?php scan and the parse. This
matters most under --web, where workers fork from a master that has executed no PHP
and are recycled after --max-requests: every worker otherwise pays the re-parse
continuously in production rather than once at boot. The format was chosen by
MEASUREMENT, which is what rejected serde_json — it decodes a small script SLOWER
than parsing it, while bincode is 3.5-5x faster. A format_guard hashes the eval IR
sources and fails the build if they change without a FORMAT_VERSION bump.
DIRECTIVES THAT ACT: validate_timestamps, revalidate_freq, max_file_size,
memory_consumption, max_accelerated_files, file_update_protection,
blacklist_filename, file_cache, file_cache_read_only, log_verbosity_level,
error_log, preload, restrict_api, enable, enable_cli. Three of them
(revalidate_freq, validate_timestamps, file_update_protection) are additionally
settable through ini_set(), which is the complete set php-src registers PHP_INI_ALL
that elephc genuinely acts on.
php-src's zend_accel_error CHANNEL — timestamped, pid-tagged, gated by
log_verbosity_level rather than error_reporting, with a FATAL exiting 254 — is
reproduced, and opcache.file_cache's startup refusal with it. A bad directory
refuses to run, exactly as reference does.
PRELOADING actually preloads: opcache.preload becomes an implicit require_once at
the top of the entry program, so its declarations compile in and its top-level code
runs first. opcache_get_status() reports the synthetic $PRELOAD$ entry, counted in
num_cached_scripts as reference counts it.
HOW IT WAS VERIFIED, and where that was not enough
Every rule was derived by probing reference PHP 8.5.10 rather than from memory, and
the matcher was additionally compared against reference over a generated corpus.
Twice that was still not enough, and both times a test passed while being wrong:
- the blacklist_miss_ratio formula was derived from a single run where
hits == blacklist_misses, so two candidate denominators gave the same number. The
right one is blacklist_misses * 100 / (hits + misses + blacklist_misses) —
php-src divides by its INTERNAL miss count, which includes blacklist misses, while
the misses it REPORTS has them subtracted back out;
- the differential corpus carried a doubled star only in TRAILING position, where
** and * behave identically, so it agreed while the matcher was wrong: php-src
compiles * to [^/]* but ** to .*, so a doubled star DOES cross a separator.
A four-way model review (Fable, Kimi K3, GLM 5.3, Deepseek) produced twenty findings,
six of which survived verification. The most serious was not in the matcher but in
its integration: load() is emitted inside ensure_eval_context, whose guard is a
FUNCTION-LOCAL stack slot zeroed in every prologue, so every call of a function
containing an eval() re-globbed and re-read every blacklist file. Enforced once on
the bridge side, since the bridge cannot assume how often generated code calls it.
DIVERGENCES, all documented in docs/php/opcache.md
Structural, and they follow from compiling ahead of time: no cache growth for
compiled code, no shared-memory segment, no tracing JIT, and no re-reading changed
code after opcache_invalidate() — the last is the only one that changes what a
program DOES rather than what it reports, and --strict-opcache turns it into a
RuntimeException.
opcache.preload_user is deliberately NOT honoured. Reference runs the preload file
in a privileged startup pass and uses the directive to drop out of root before
executing it; elephc inlines the file, so its code runs with the privileges of
whoever runs the binary and there is no boundary to drop from. The halves cannot be
split either: the uid-0 fatal WITHOUT the user switch would be worse than neither,
since setting the directive as root would let the binary keep running as root while
appearing guarded.
PAY-FOR-USE HELD throughout, which was the standing risk: the bridge calls fold away
at lowering time when eval_bridge is false. A program calling opcache_get_configuration()
and nothing else measures 0.13 MB against 0.05 MB for a bare program — the opcache
prelude's own baked literals, not the ~1.25 MB an eval-linked binary costs.
ALSO FIXED, unrelated to OPcache but found on the way
- libc::asctime does not exist on Linux; it broke two of the five supported targets
the moment accel_log.rs was first compiled. Replaced with a hand-written
format_asctime checked against the system asctime over 200,000 timestamps.
- opcache_get_status()['scripts'] reported its FIRST entry's last_used in local time
and every other entry in UTC: the helper resolved its zone from getenv('TZ') on
every call, and elephc's own date_default_timezone_set() WRITES TZ.
- A string returned by a runtime helper and stored into a STATIC property kept a
pointer into the shared concat scratch buffer.
- implode() over a mixed-typed array segfaulted for int elements.
- A run-time promoted array was read through the packed path by some readers.
A SECOND REVIEW ROUND, run against the code the first round had already fixed, found
nine more defects — SIX of them in those very fixes, which is the reason for running
it. Each was verified against reference PHP 8.5.10 before being treated as real.
THE MATCHER WAS WRONG IN ITS FIX, not in the original. Tracking a single most-recent
star is the classic glob trick, and it is correct only while every star is
interchangeable — which stopped being true the moment `*` and `**` were given
different crossing rules. A single star blocked at a separator then gave up instead
of falling back to an earlier `**`, so `/srv/**/*.php` missed `/srv/a/b/c.php`, which
reference refuses. One reviewer built a differential oracle against php-src's regexp
and found 23 mismatches in 200_000 random cases, every one containing `**`. The walk
is now a dynamic program over (token, position): it evaluates each pair once, so it
explores every star split by construction and cannot carry that class of bug, while
staying linear where a naive recursive fix would be exponential.
THE OTHER EIGHT:
- `fill_entry` bumped `misses` BEFORE the `max_file_size` refusal, so an oversized
include moved both counters — contradicting the reference figures quoted two lines
below it (`misses=0 blacklist_misses=2`). The size refusal now precedes the
accounting, like the blacklist refusal it mirrors. The three refusals differ and the
differences are measured, not assumed: blacklist and size move `blacklist_misses`
alone, age moves `misses` alone.
- The on-disk file cache was written BEFORE the size and age refusals, so a file those
rules reject was persisted anyway — including the part-written file
`opcache.file_update_protection` exists to keep out.
- A RELATIVE `opcache.blacklist_filename` left the entry base empty, which the path
folding turned into `/`, so every relative entry became `/name` and blocked nothing.
Resolved against the process cwd, as php-src's `expand_filepath` does.
- `__rt_str_persist` documents its x86_64 input as the string RESULT pair (`rax`/`rdx`),
not the SysV argument registers, and its first instruction is `cmp rax, r10`. The
empty-string fold wrote `rdi`, leaving `rax` holding whatever preceded it. The
bridge path wrote `rdi` too and worked only because `rax` already held the pointer.
- `opcache_get_status()` was quadratic in cached scripts: the generated loop asks for
five things per script and every reader rebuilt the whole snapshot, cloning each path
and re-sorting, with the cache mutex held. At the default ceiling of 10000 entries
that is 50000 snapshots of 10000 items for one status call. Now one snapshot per
thread, keyed by a generation the cache bumps on every mutation.
- POSIX `glob()` hides dotfiles: a leading `.` is matched only by a literal `.`, never
by `*`, `?` or a class. VERIFIED — `opcache.blacklist_filename=*.list` loads
`deny.list` and ignores `.secret.list`. Without this an editor backup beside the real
list would be read as a blacklist.
- `opcache_hit_rate` carried the SAME wrong denominator the blacklist ratio had, in the
sibling formula that was not reopened when that one was fixed. Both are over
`hits + misses + blacklist_misses`. VERIFIED: hits=5 misses=2 blacklist_misses=1
reports 62.5, which is 5*100/8 and not 5*100/7.
- An entry resolving to the filesystem root expanded to `//` rather than `/`, so the
most sweeping entry a list can carry matched nothing.
A THIRD ROUND, against the code the second had fixed, found six more — three of them
major, and all three in code written or changed the same day. Each was verified
against reference PHP 8.5.10 before being treated as real.
A SECOND-LEVEL HIT WAS COUNTED AS A MISS. `fill_entry` bumped `misses`
unconditionally, including when the script came back from the on-disk
`opcache.file_cache`. php-src reaches `ZCSG(misses)++` only when the file cache
produced nothing; a load from it takes the same branch as a shared-memory hit.
VERIFIED: two runs against one file-cache directory report `hits=0 misses=2` cold and
`hits=2 misses=0` warm — elephc reported the exact opposite in the recycled-worker
case the file cache exists for.
THE RESTART DID NOT ZERO THE COUNTERS. php-src's restart runs
`zend_reset_cache_vars()`, which clears `hits`, `misses` and `blacklist_misses` along
with the entries: it begins a fresh accounting period, not just a fresh cache.
Carrying them over left every later request, and both ratios, inflated for the life of
a worker. This was raised in the SECOND round and left alone because CLI cannot show
it — CLI has no second request, so a deferred restart never happens. `php -S` runs many
requests in one process and settles it: a request reporting `hits=4 misses=2` before
the reset is followed, after the restart, by `hits=0` plus only its own misses. A
finding that cannot be measured yet is not a finding that is wrong; it is waiting for
the right instrument.
`tests/web_session_tests.rs::opcache_reset_is_performed_at_the_next_request_boundary`
asserted the cumulative `m=2` and so PINNED that bug. It was written earlier the same
day, with a confident docblock, by the same hand as the code — which is exactly how a
defect survives a test suite. Corrected to `m=1`, with the reasoning and the
measurement written into the docblock rather than the number quietly adjusted. Its
discriminating field is now `h` (1 when the entry survived, 0 when the restart threw it
away), so it still separates reset from no-reset.
THE SNAPSHOT WENT STALE ON A WARM HIT. The per-script snapshot added hours earlier to
make `opcache_get_status()` linear is keyed on a generation counter, and the hit path
moved `entry.hits` and `entry.last_used` without bumping it — so `scripts[…]['hits']`
froze at whatever the first status call of the process saw while the aggregate `hits`
beside it kept counting. The struct's own comment promised "bumped on every mutation".
A fix for one defect introducing another is the reason this round existed.
THREE MINOR ONES, each measured before being believed:
- A line that is just two quotes expanded to the blacklist file's OWN DIRECTORY — a
prefix refusing everything beneath it, which for a list living beside the code it
names is the whole application. php-src strips the quotes, finds an empty entry and
skips the line. VERIFIED: reference blocks nothing and reports an empty list.
- `[^a]` in the directive glob was read as a negation. PHP bundles its own `php_glob`
(system glob is off by default), where `#define NOT '!'` and a leading `^` is an
ordinary member. VERIFIED: `bl_[^a].list` loads `bl_a.list`, which POSIX semantics
would have excluded — so the wrong file's entries were enforced.
- An entry resolving to the filesystem root produced `//` rather than `/`.
THE ONE DIVERGENCE NO TEST COVERED IS NOW PINNED. Reference reads the blacklist files
during startup, so `opcache_get_configuration()['blacklist']` carries them from the
program's first line; elephc loads them when the eval context is built, which is the
only moment a compiled binary has. The same call therefore answers `[]` before the
first eval and the patterns after it. Every existing test happened to read the
configuration AFTER its eval, so none could observe the pre-eval answer and the
divergence was free to change in either direction unnoticed. VERIFIED both ways:
reference reports `before=1 after=1`, elephc `before=0 after=1`.
The new test pins a DIFFERENCE rather than a value, so it discriminates by
construction: if the load ever moves to startup the `before` assertion fails (and the
test should simply be deleted), and if the list ever stops loading the `after` one
does. The refusal itself is unaffected either way — nothing can be included before the
eval that loads the list, because the include path runs through it.
THE MATCHER ITSELF NOW HAS INDEPENDENT EVIDENCE, which the previous rounds' hand-written
corpora did not provide. An oracle built from php-src's own translation rules (`*` →
`[^/]*`, `**` → `.*`, `?` → `[^/]`, anchored at the start only) was first VALIDATED
against real OPcache over 30 discriminating cases, one PHP process each because the
blacklist is read once at startup — 30 of 30 agree. The shipped matcher then agreed with
it on 40_010 generated pairs, 0 mismatches. The harness was also shown to DISCRIMINATE:
replaying the pre-rewrite matcher against the same oracle produces 6 mismatches. Worth
recording honestly — 3 of those 6 came from hand-built adversarial shapes rather than
the random draw, so volume alone would likely have missed the defect.
TWO TESTS ASSUMED `/` IS NOT WRITABLE, which holds for an ordinary user and not for
root — and CI runs as root on the Linux runners, where access("/", R_OK|W_OK)
succeeds and the read-write refusal stops happening. The behaviour there is correct
(reference PHP accepts the directory as root too), so both tests now MEASURE whether
this process can write to `/` rather than assuming it cannot. Measured, not inferred
from geteuid(): root is the common way to hold that access but not the only one, and
a CAP_DAC_OVERRIDE process would break a uid test in exactly the same way.
THE GENERATED BUILTIN PAGES ARE REGENERATED HERE TOO, and the reason is worth
stating because the diff carries no contract change. Each generated page and
`scripts/docs/builtin_registry.json` embed the LINE NUMBER of the builtin's lowering,
so the ten lines this change adds to `src/opcache_prelude/build.rs` move five
`opcache_*` entries by +10. That is drift with nothing behind it, and the
`builtins-docs-sync` job is right to refuse it anyway: the gate cannot tell a moved
line from a moved implementation, so the only honest answer is to regenerate.
Regenerated in the canonical `--features curl` configuration. `docs/php/compatibility.md`
was regenerated as well and comes back byte-identical, so the comparison page is
measurably unaffected rather than assumed to be.
Rebasing onto main left every hardcoded catalog count one short: main has added the `sizeof` contract since this branch's base, and each assertion here names an absolute total rather than a delta. The four numbers come from the failing assertions themselves, not from arithmetic on the comments -- contracts 1045 -> 1046, backend support records 583 -> 584, AOT registry 635 -> 636, interpreter adapters 562 -> 563. All 24 contract tests pass and `cargo check --all-targets` is clean. Generated builtin docs regenerated so `builtins-docs-sync` has nothing to report. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Raised in review as an untrusted-cache code-execution finding. The page already documents the three checks an entry must pass, which is what made the gap easy to miss: they authenticate the SOURCE — path, mtime, size, format — and say nothing about the WRITER. A file cache has no secret to authenticate with, so anyone who can create files in the directory can place an entry under a script's path hash with matching header fields and have its parsed form run in place of that script. Says so plainly, and gives the operational rule: treat the directory as a directory of executable PHP. Also qualifies the read-only note above it, which presented a shared directory as straightforwardly usable. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
`tests/implode_element_layout_tests.rs` asserts that `implode()` renders every element layout of a `mixed`-typed array. That is illegalstudio#1054's subject, not this branch's: the runtime layout dispatch it exercises is not here, so the test fails with `binary failed (exit None)` -- the SIGSEGV it was written to catch -- and reddened `Non-Codegen Tests` on both Linux arches. The same seven cases are now pinned in illegalstudio#1054's own `tests/codegen/strings/implode_boxed.rs`, beside the change that makes them pass, so nothing is lost by removing the copy here. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
a932491 to
005e84a
Compare
|
Rebased onto The failure was not the OPcache work.
I have pinned the same seven cases in #1054, in Verified locally after the rebase: Two things I did not change, because they look deliberate and are a repo-policy call rather than mine:
|
Before this branch, elephc reported all 54 OPcache directives with byte-verified values and answered all 8 API functions — and almost none of it did anything. Fifteen directives now change what the program does, three shape a reported value, and the other 36 remain honest reporting.
What now exists
A runtime script cache for the one tier an AOT binary can still grow: files pulled in by dynamic
include/requirereached througheval(). The compile-time manifest stays frozen — that is the "the binary IS the cache" premise — so this is where an opcode-cache-shaped saving still exists. Measured on the eval-hosted include benchmark: ~20.6 ns per source byte below the old 64 KiB fragment cap, and the whole re-parse above it (a 128 KiB include cost 10.98 ms uncached against a ~0.26 ms floor).An on-disk file cache behind
opcache.file_cache, holding elephc's parsed form of those scripts, so a cold process skips the read, the<?phpscan and the parse. This matters most under--web, where workers fork from a master that has executed no PHP and are recycled after--max-requests: every worker otherwise pays the re-parse continuously in production rather than once at boot.The format was chosen by measurement, which is what rejected
serde_json— it decodes a small script slower than parsing it, whilebincodeis 3.5–5× faster. Aformat_guardhashes the eval IR sources and fails the build if they change without aFORMAT_VERSIONbump.php-src's
zend_accel_errorchannel — timestamped, pid-tagged, gated byopcache.log_verbosity_levelrather thanerror_reporting, with a FATAL exiting 254 — andopcache.file_cache's startup refusal with it. A bad directory refuses to run, exactly as reference does.Preloading actually preloads:
opcache.preloadbecomes an implicitrequire_onceat the top of the entry program, so its declarations compile in and its top-level code runs first.opcache_get_status()reports the synthetic$PRELOAD$entry, counted innum_cached_scriptsas reference counts it.Directives that act
validate_timestamps·revalidate_freq·max_file_size·memory_consumption·max_accelerated_files·file_update_protection·blacklist_filename·file_cache·file_cache_read_only·log_verbosity_level·error_log·preload·restrict_api·enable·enable_cliThree of them —
revalidate_freq,validate_timestamps,file_update_protection— are additionally settable throughini_set(). That is the complete set php-src registersPHP_INI_ALLthat elephc genuinely acts on; succeeding for the other fifteen would move a reported value while nothing changed.How it was verified — and where that was not enough
Every rule was derived by probing reference PHP 8.5.10 rather than from memory, and the blacklist matcher was additionally compared against reference over a generated corpus of 34 paths × 26 patterns.
Twice that was still not enough, and both times a test passed while being wrong:
blacklist_miss_ratioformula came from a single run wherehits == blacklist_misses, so two candidate denominators gave the same number. The right one isblacklist_misses * 100 / (hits + misses + blacklist_misses): php-src divides by its internal miss count, which includes blacklist misses, while themissesit reports has them subtracted back out.**and*behave identically — so it agreed while the matcher was wrong. php-src compiles*to[^/]*but**to.*, so a doubled star does cross a separator.A four-way model review (Fable, Kimi K3, GLM 5.3, Deepseek) produced twenty findings, six of which survived verification. The most serious was not in the matcher but in its integration:
load()is emitted insideensure_eval_context, whose guard is a function-local stack slot zeroed in every prologue — so every call of a function containing aneval()re-globbed and re-read every blacklist file. Enforced once on the bridge side, since the bridge cannot assume how often generated code calls it.Divergences
All documented in
docs/php/opcache.md.Structural, following from compiling ahead of time: no cache growth for compiled code, no shared-memory segment, no tracing JIT, and no re-reading changed code after
opcache_invalidate(). The last is the only one that changes what a program does rather than what it reports, and--strict-opcacheturns it into aRuntimeException.opcache.preload_useris deliberately not honoured. Reference runs the preload file in a privileged startup pass and uses the directive to drop out of root before executing it; elephc inlines the file, so its code runs with the privileges of whoever runs the binary and there is no boundary to drop from. The halves cannot be split either: the uid-0 fatal without the user switch would be worse than neither, since setting the directive as root would let the binary keep running as root while appearing guarded.Pay-for-use held
This was the standing risk, since bridge calls sit inside functions every OPcache program calls. They fold away at lowering time when
eval_bridgeis false: a program callingopcache_get_configuration()and nothing else measures 0.13 MB against 0.05 MB for a bare program — the opcache prelude's own baked literals, not the ~1.25 MB an eval-linked binary costs.Also fixed, unrelated to OPcache but found on the way
libc::asctimedoes not exist on Linux. It broke two of the five supported targets the momentaccel_log.rswas first compiled, and took four apparently unrelated CI jobs down with it. Replaced with a hand-writtenformat_asctime, checked against the systemasctimeover 200,000 timestamps.opcache_get_status()['scripts']reported its first entry'slast_usedin local time and every other entry in UTC — the helper resolved its zone fromgetenv('TZ')on every call, and elephc's owndate_default_timezone_set()writesTZ.implode()over a mixed-typed array segfaulted for int elements.The pre-squash history is preserved at
backup/opcache-pre-squash-20260912if the per-commit reasoning is wanted for review; delete it once this merges.