fix(appsec): load on musl ZTS; ci: disk envelope for helper-rust coverage - #4180
fix(appsec): load on musl ZTS; ci: disk envelope for helper-rust coverage#4180Leiyks wants to merge 3 commits into
Conversation
…n load
The TLS destructor registration in PHP_GINIT_FUNCTION(ddappsec) declared
__cxa_thread_atexit_impl as a strong symbol and called it unconditionally on
Linux. The symbol is glibc-private and musl exports it nowhere, and musl
resolves relocations eagerly, so dlopen("ddappsec.so") fails outright with
"Error relocating ...: __cxa_thread_atexit_impl: symbol not found". AppSec is
therefore dead on musl + ZTS (FrankenPHP-Alpine, Swoole/pthreads on Alpine) in
1.25.0 and 1.25.1. ddtrace-zts.so references the same symbol weakly, which is
why the tracer is unaffected.
Declare it weak and register the destructor only when it resolves.
registered_thread_local_dtor then stays false on musl, which is the case
PHP_GSHUTDOWN_FUNCTION(ddappsec) already handles ("a platform without a
thread-exit destructor mechanism"), so tshutdown still runs on the owning
thread. glibc is unchanged: the weak reference resolves there and the
destructor is registered as before.
Verified on dunglas/frankenphp:php8.3.12-alpine (PHP 8.3.12 ZTS, musl 1.2.5),
same tree, incremental rebuild of only this file:
before: NOTYPE GLOBAL DEFAULT UND __cxa_thread_atexit_impl
extension_loaded("ddappsec") => false, "Error relocating" warning
after: NOTYPE WEAK DEFAULT UND __cxa_thread_atexit_impl
extension_loaded("ddappsec") => true, no relocation error
…overage "helper-rust integration coverage" never set DOCKER_LOOPBACK_SIZE, so it ran on the DinD default of ~20G while its sibling .appsec_integration_tests sets 30G and "push appsec images" sets 100G. 17 consecutive master runs died in :buildPortableLibdatadogPhp with "failed to build archive at .../libdatadog_php.a: No space left on device (os error 28)", and the 10 runs after that with "Bus error" from musl-clang at the final cdylib link. A same-pipeline A/B against the sibling holds everything else equal - same image, runner tag, CPU and memory - and differs only by -PuseHelperRustCoverage and the loopback size. Raise it to 50G: the coverage build is strictly larger than the sibling's 30G. The existing rm -rf /vol/cargo-target mitigation sits after the build, so it never runs when the build itself is what exhausts the volume. Also report df -h /, docker system df and the php-portable-libdatadog-php volume before and after the build. The ENOSPC is measured; the "Bus error" being the same exhaustion one step later is still an inference, and these numbers settle it in one cycle. The gradle exit status is preserved so a failing build still fails the job, and the report is emitted on that path too, which is the case the numbers are for.
|
Benchmarks [ appsec ]Benchmark execution time: 2026-09-10 23:38:36 Comparing candidate commit 13be1d2 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.
|
2 participants
Two independent fixes, one per commit. They share no code and can be reviewed separately.
1.
fix(appsec): AppSec cannot load at all on musl + ZTSPHP_GINIT_FUNCTION(ddappsec)declared__cxa_thread_atexit_implas a strong symbol and called it unconditionally on Linux. That symbol is glibc-private, musl exports it nowhere, and musl resolves relocations eagerly — sodlopen("ddappsec.so")fails outright withError relocating ...: __cxa_thread_atexit_impl: symbol not found.ddtrace-zts.soreferences the same symbol weakly, which is why the tracer is unaffected.This is a shipped, user-facing regression, not a CI artefact.
readelf -sWon the published bundles: absent in 1.24.1,GLOBAL UNDin both 1.25.0 and 1.25.1 (introduced by #3725). Every musl+ZTS deployment — FrankenPHP-Alpine is the canonical case, plus Swoole/pthreads on Alpine — silently loses AppSec with a startup warning. The blast radius is preciselymusl ∩ ZTS: NTS musl builds emit no reference at all, and glibc ZTS emits the sameGLOBAL UNDbut is saved by glibc ≥ 2.18 exporting the symbol.The fix declares the symbol
__attribute__((weak))and registers the destructor only when it resolves.registered_thread_local_dtorthen staysfalseon musl, which is exactly the casePHP_GSHUTDOWN_FUNCTION(ddappsec)already documents and handles ("a platform without a thread-exit destructor mechanism"), sotshutdownstill runs on the owning thread. glibc behaviour is unchanged — the weak reference resolves there and the destructor is registered as before.Verified by building
ddappsec.sofor musl+ZTS indunglas/frankenphp:php8.3.12-alpine(PHP 8.3.12 ZTS, musl 1.2.5, API 20230831 — the exact image the failing CI sub-test uses), A/B on the same build tree with an incremental rebuild of only this file:extension_loaded("ddappsec")NOTYPE GLOBAL DEFAULT UNDfalse, with the verbatim CIError relocatingwarningNOTYPE WEAK DEFAULT UNDtrue, no relocation errorinstaller testswill not go green when this merges. That job installs the published GitHub release for$(cat VERSION)— itsneedssupply only*-unknown-linux-gnubundles, and no musl-ZTS bundle exists as an artifact input — so it keeps downloading the immutable 1.25.1 tarball regardless of what master contains. It stays red until a release containing this fix is published andVERSIONpoints at it, or the sub-test is re-pointed at a locally built musl-ZTS bundle. In other words this fix is release-blocking, not CI-blocking, and the gating decision for that job is separate from this code change. Its single failing sub-testtest_alpine_zts_no_zend_signals.shis also the only failing one: becausefindordering varies per run, the union across recent master runs covers 40/40 sub-tests with 39/40 observed passing.2.
ci: disk envelope forhelper-rust integration coverageThe job never set
DOCKER_LOOPBACK_SIZE, so it ran on the DinD default of ~20G, while its sibling.appsec_integration_testssets30Gandpush appsec imagessets100G. On master it has failed 33/33 runs over the last three days in two consecutive signatures, both in:buildPortableLibdatadogPhp: 17 runs withfailed to build archive at .../libdatadog_php.a: No space left on device (os error 28), then 10 runs withBus errorfrommusl-clangat the final cdylib link. A same-pipeline A/B against the sibling holds image, runner tag, CPU and memory equal and differs only by-PuseHelperRustCoverageand the loopback size; OOM was positively excluded (noOOMKilled/eviction/DiskPressurefor the exact pods, and SIGBUS is not what a SIGKILL looks like).Raised to
50G, since the coverage build is strictly larger than the sibling's 30G. Note that the existingrm -rf /vol/cargo-targetmitigation sits after the build, so it never runs when the build itself is what exhausts the volume.The second half of the change is measurement rather than mitigation:
df -h /,docker system dfanddf -hon thephp-portable-libdatadog-phpvolume, before and after the build. The ENOSPC is measured; the claim that theBus erroris the same exhaustion one step later is currently an inference, and these numbers settle it in a single cycle. The gradle exit status is preserved so a failing build still fails the job, and the report is emitted on the failure path too — which is the case the numbers exist for. Nothing is|| true'd and noallow_failureor flaky-jobs entry is added.Follow-up, deliberately not in this PR
Scoping
-C instrument-coverageto thehelper-rustpackage (-Zprofile-rustflags+profile.tracer-release.package.helper-rust.rustflags) instead of applying it globally on top of-Z build-stdwould attack the cause of the size rather than the envelope, and would cut build time. It is left out because it is unverified and risks silently emptying the LCOV report under fat LTO; it should land on its own once thedfnumbers above are in and the report can be diffed against a known-good one.