Skip to content

refactor(sampling): replace std::random_device with the existing xorshift64 - #794

Merged
rkennke merged 4 commits into
mainfrom
refactor/drop-random-device
Sep 16, 2026
Merged

rkennke merged 4 commits into
mainfrom
refactor/drop-random-device

Conversation

@rkennke

@rkennke rkennke commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?:

Gives the profiler one random source instead of two. xorshift.h holds the
generator and the draws built on it; the four places that had their own copy of
the recurrence now call it, and the three that used std::mt19937 seeded from
std::random_device are converted.

before after
PoissonSampler xorshift64, inline xorshift::
ProfiledThread::_fi_rng xorshift64, inline xorshift::
faultInjection fallback xorshift64, inline xorshift::
LivenessTracker mt19937 + uniform_real_distribution, per thread, heap ThreadLocal<u64>
ReservoirSampler mt19937 + two distributions xorshift::
BaseWallClock mt19937 + normal_distribution xorshift::nextNormal

<random> is gone from the library.

Motivation:

poissonSampler.h already argued the case, and the argument was never specific
to that one sampler:

  • Allocation and exceptions. std::random_device and the distribution
    wrappers may allocate and may throw. LivenessTracker::track runs on
    allocation callbacks on arbitrary application threads, where neither is safe.
  • Seeding. std::random_device may block or return low-entropy values
    depending on the host, and which it does is decided by the libstdc++ the
    library was compiled against, not by anything in this source. That made a
    sampling-relevant behaviour a property of the build image.
  • Size. std::mt19937 carries ~2.5 KB of state. LivenessTracker held one
    per thread, on the heap, plus a distribution object — and the lazy-create /
    explicit-free machinery that existed only because they were heap-allocated.

Side effect that matters elsewhere. std::random_device is also why the
shipped library referenced getentropy@GLIBC_2.25. That reference is not in our
code: libstdc++ decides at its build time how to seed std::random_device, and
from GCC 12 on it calls getentropy where the glibc it was configured against
has it — so -static-libstdc++ copies random.o and that undefined reference
into libjavaProfiler.so. glibc has only one version of getentropy, so there
is no older symbol to pin it to. Removing the three std::random_device uses
keeps random.o out of the link entirely, which is what lets #790 hold the
glibc 2.17 floor while building on a supported EL8 toolchain — the remaining
above-2.17 reference there is expf, which is in our own code and can be
.symver-pinned. This PR stands on the reasons above regardless of that
decision; the floor is a beneficiary, not the motivation.

Additional Notes:

Seeding is no longer OS-backed. A stream is an identity mixed with a sequence
number — an object address with an epoch, a thread id with a tick. That yields
independent streams per thread and per recording without an entropy source.
Sampling needs independence, not unpredictability, so this is a deliberate
trade rather than an oversight: two processes started on the same tick with
ASLR disabled would correlate. Nothing here is used for anything security-
relevant.

ReservoirSampler now takes its stream as a constructor argument. Seeding
from the instance address alone meant every sampler built at one call site got
the same stream, so consecutive recordings sampled identically. Making it a
required argument means a caller cannot silently inherit a fixed stream; the
wall-clock loop passes TSC::ticks().

The wall-clock dither keeps its shape. It was normal_distribution(interval, interval/10) and still is, drawn by Box-Muller. The alternatives (a flat
dither, sum-of-uniforms) would have changed observable behaviour for no reason.

LivenessTracker's subsample decision is now an integer compare against a
threshold computed once in initialize(), the same device faultInjection
already used for its probability tiers.

How to test the change?:

ddprof-lib/src/test/cpp/xorshift_ut.cpp — 23 assertions, picked up
automatically by the gtest plugin:

  • seed: never returns 0 (a fixed point that would freeze the stream),
    including when the two inputs cancel exactly; distinct identities give
    distinct streams; small sequence numbers (0, 1, 2 …) are spread rather than
    differing in a few low bits
  • next: advances, never returns 0, no repeats over a short run
  • toUnitDouble: strictly inside (0, 1) at both extremes of the input range
    and over 10k draws, with log() finite throughout; roughly uniform by decile
    and mean
  • threshold: saturates at both ends rather than casting an out-of-range
    double; monotonic; fires at the requested rate (0.01 … 0.9)
  • boundedIndex: stays in range for every n up to 64; handles n == 0; covers
    and balances the range; does not favour low indices for a non-power-of-two n,
    which a modulo mapping would
  • nextNormal: matches the requested mean and stddev over 100k draws;
    returns the mean for stddev 0
  • ReservoirSampler: keeps everything when the input fits; fills to capacity
    when it does not; writes every slot; distinct streams give distinct samples
    and the same stream gives the same samples; empty input; reuse across calls

Two defects in the first draft were found by these tests and fixed:
toUnitDouble(UINT64_MAX) returned exactly 1.0 (1 - 2^-65 rounds up), which
would make log(1 - u) infinite for a later caller; and every ReservoirSampler
at one call site produced byte-identical output. Both have tests pinned to the
specific failure.

Also run: the full gtestDebug suite, and gtestDebug_faultInjection_ut under
-PenableFaultInjection — the three tier-rate tests there exercise the
consolidated generator statistically, and the fault-injection paths are not
compiled by the default build.

For Datadog employees:

  • This PR doesn't touch any of that.

🤖 Generated with Claude Code

…hift64

Sampling code carried two random sources: xorshift64, written out in
PoissonSampler together with the rationale for preferring it, and std::mt19937
seeded from std::random_device in three other places. The reasons given for
the first apply to all of them.

std::random_device and the distribution wrappers may allocate and may throw,
and LivenessTracker::track runs on allocation callbacks on arbitrary
application threads, where neither is safe. std::random_device may also block
or return low-entropy values depending on the host, and which it does is
settled by the libstdc++ the library is compiled against rather than by
anything in this source.

xorshift.h now holds the generator and the draws built on it, and every copy
of the recurrence calls it: PoissonSampler, ProfiledThread's fault-injection
state, faultInjection's global fallback, and the three converted sites. The
Knuth constant references common.h instead of being spelled out a fifth time.

LivenessTracker loses the most: two thread-local heap allocations per thread
(~2.5 KB of mt19937 among them), four create/free functions, and a subsample
decision that is now an integer compare against a threshold computed once in
initialize().

Seeding no longer draws on the OS. A stream comes from an identity mixed with
a sequence number -- an object address with an epoch, a thread id with a tick
-- giving independent streams per thread and per recording with no entropy
source. Sampling requires independence rather than unpredictability, so this
is a deliberate trade: two processes started on the same tick with ASLR
disabled would correlate.

ReservoirSampler takes its stream as a constructor argument. Seeding from the
instance address alone made every sampler built at one call site replay a
single stream, so consecutive recordings sampled identically.

The wall-clock interval dither keeps its normal distribution, now drawn by
Box-Muller rather than std::normal_distribution.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@rkennke
rkennke requested a review from a team as a code owner September 16, 2026 09:06
@datadog-prod-us1-5

datadog-prod-us1-5 Bot commented Sep 16, 2026

Copy link
Copy Markdown

Pipelines

❌ Errors

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 4 Pipeline jobs failed

DataDog/java-profiler | gtest-asan-amd64

View more details · View in GitLab

DataDog/java-profiler | gtest-asan-arm64

View more details · View in GitLab

DataDog/java-profiler | gtest-tsan-amd64

View more details · View in GitLab

View all 4 failed jobs.

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 7237b42 | Docs | View more details | Give us feedback!

@dd-octo-sts

dd-octo-sts Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Scan-Build Report

User:runner@runnervmlun5p
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Wed Sep 16 14:17:17 2026

Bug Summary

Bug TypeQuantityDisplay?
All Bugs1
Logic error
Dereference of null pointer1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Logic errorDereference of null pointerfaultInjection.cppcrashNow242

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5364e99f9a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ddprof-lib/src/main/cpp/xorshift.h

@datadog-prod-us1-5 datadog-prod-us1-5 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: FAIL

The new threshold helper converts NaN to u64. The liveness option parser accepts NaN, so profiler setup has undefined behavior.

Open Bits AI session

🤖 Datadog Autotest · Commit 5364e99 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

Comment thread ddprof-lib/src/main/cpp/xorshift.h
@rkennke rkennke added the sphinx:critical Sphinx: critical — human review required label Sep 16, 2026
@dd-octo-sts

dd-octo-sts Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #35107293522 | Commit: 25bec40 | Duration: 3h 0m 44s (longest job)

All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - - -
8-ibm - 🚫 - -
8-j9 - -
8-librca - -
8-orcl - - -
11 - - -
11-j9 - -
11-librca - -
17 - -
17-graal - -
17-j9 - -
17-librca - -
21 - -
21-graal - -
21-librca - -
25 - -
25-graal - -
25-librca - -

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 31 | Failed: 0 | Cancelled: 1


Updated: 2026-09-16 17:19:04 UTC

…sholds

Pre-review follow-ups, three of them comments that claimed properties the
code does not have.

reservoirSampler.h lost its transitive <cassert> when <random> went, so the
header no longer compiled standalone; it built only because wallClock.h
includes <cassert> first and the gtest gets assert from gtest.

MIN_UNIFORM's comment said the floor was not about finiteness. At _size == 1,
reachable with wall_threads_per_tick=1, weight equals the draw, and
toUnitDouble's smallest draw makes 1 - weight round to exactly 1.0,
log(1 - weight) exactly 0.0 and the division -inf, whose cast to int is
undefined. The floor is what prevents that, so the comment was inviting a
change that would introduce undefined behaviour. It now says so, along with
the ~1e-16 engagement probability that makes the case untestable.

xorshift::threshold() compared p against 0 in a way NaN passes, leaving an
unordered value to reach the cast -- undefined, and a trap under
-fsanitize=undefined. A "nan" sampling ratio survives the min/max clamp in
arguments.cpp and reaches it. The guard is inverted and covered by a test.

The two LivenessTracker comments described the pre-change design: one argued
about pthread-key destructors leaking heap allocations that no longer exist
(both ThreadLocals now hold plain values and register no destructor), the
other promised a per-recording reseed that does not happen, since the slot is
cleared only at thread end or JNI detach and not by stop/start.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Comment thread ddprof-lib/src/main/cpp/livenessTracker.cpp Outdated
…e type

The ratio and the threshold derived from it were two independent fields kept in
step by convention. Nothing stopped a future writer of the ratio from leaving
the threshold behind, and the result would have been silent: allocations
sampled at the old rate while Recording reports the new ratio into the JFR
chunk, with no assertion or counter disagreeing.

SubsampleRate holds both, and its only constructor derives the threshold from
the ratio, so one cannot be set without the other; initialize() replaces the
pair in a single assignment. The invariant is now enforced by construction
rather than remembered.

Tested against observed behaviour instead of against xorshift::threshold, so
the tests still fail if the derivation stops honouring the advertised rate: the
keep-rate over 200k draws matches the ratio for four rates, reassignment
carries both halves, and the degenerate ratios (1, 0, NaN) all yield defined
thresholds.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

@zhengyu123 zhengyu123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few nits.

Comment thread ddprof-lib/src/main/cpp/xorshift.h
Comment thread ddprof-lib/src/main/cpp/xorshift.h
Comment thread ddprof-lib/src/main/cpp/threadLocalData.h Outdated
…ariant

Review follow-ups, all three of them places the first pass left behind.

ProfiledThread had two _fi_rng seeding sites -- claimAcquire and the
constructor -- and only the first was converted, so a hand-rolled copy of the
address-XOR-Knuth-hashed-tid seed survived in threadLocalData.h. Both now call
xorshift::seed, which is what the earlier commit claimed of every copy.

faultInjection.cpp kept its own KNUTH constant after xorshift.h started
referencing common.h's, leaving two declarations of one value; it now uses
xorshift::KNUTH at both of its sites.

next() documented requiring a non-zero state without checking it. It now
asserts on entry, and again after stepping: the recurrence is a bijection on
the non-zero states, so a valid state never reaches the fixed point, and
saying so in an assertion documents the property rather than only guarding the
input. Release builds compile with -DNDEBUG, so neither check reaches the
sampling path.

The non-zero rule itself had three implementations -- inside seed(), in
setFiRng's `seed ? seed : 1`, and in faultinj::nextRandom's `if (x == 0)`.
xorshift::nonZero() is now the only one, and seed() is expressed in terms of
it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@rkennke
rkennke merged commit 8160a6a into main Sep 16, 2026
95 of 102 checks passed
@rkennke
rkennke deleted the refactor/drop-random-device branch September 16, 2026 14:48
@github-actions github-actions Bot added this to the 1.51.0 milestone Sep 16, 2026
rkennke added a commit that referenced this pull request Sep 16, 2026
#794 removed the std::random_device uses, so libstdc++'s getentropy reference
no longer enters the link and the last symbol above 2.17 is gone. With expf
already pinned per architecture, the artifact requires nothing newer than the
glibc that EL7 ships.

Measured on release builds in the AlmaLinux 8 image on both architectures: no
references above 2.17 at all, and the floor check passes at 2.17 where it
previously named getentropy@GLIBC_2.25. Both artifacts also load on Oracle
Linux 7.9 (glibc 2.17) with no unresolved symbol versions, which tests the
dynamic linker rather than the symbol table.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
rkennke added a commit that referenced this pull request Sep 17, 2026
… EOL Debian (#790)

* ci: drop the archived buster base default, bump the amd64 build image

The x64-2.17 image's Dockerfile defaulted to openjdk:11-slim-buster. Debian 10
buster left even LTS support in June 2024 and its repositories are archived, so
`apt-get update` against them now fails with "does not have a Release file" --
and rebuild-images.sh builds without --build-arg BASE_IMAGE whenever
BASE_IMAGE_LIBC_2_17 is unset, which reaches that default. The body of the file
is yum-based, so a Debian base cannot work there at all; the default now names
what CI actually passes (centos:7), with the glibc 2.17 rationale for staying
on an EOL distro recorded next to the vault mirror URLs it already needs.

The amd64 build image moves to Debian 13 trixie, the current stable release.
It compiles no shipped native code -- build:x64 uses the glibc 2.17 image -- so
its glibc is not a customer-facing floor. The arm64 image stays on bullseye: it
is where build:arm64 compiles the shipped linux-arm64 libjavaProfiler.so, so
that glibc (2.31) is the runtime floor for arm64 customers and trixie would
raise it to 2.41. Both variables now say which of the two they are and why.

Also drop the `|| true` from the base image's package install, and with it the
dead `apk` branch -- Alpine has its own Dockerfile.musl. As written, a total
install failure produced a "successful" image whose tools were simply missing,
surfacing later as an unrelated job failure.

Verified against the real archives: buster's apt repositories fail as
described, bullseye's still resolve, and the full package list installs on
trixie with hexdump present (bsdmainutils is still a real package there).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* ci: build the shipped binaries on AlmaLinux 8 with gcc-toolset-14

Release builds link -static-libstdc++ -static-libgcc, so the C++ runtime and
unwinder in the shipped libjavaProfiler.so come out of the build image's
toolchain. That makes the build image's support status a property of the
artifact, not just of CI: on CentOS 7 those objects can never receive another
security rebuild, and no gcc newer than devtoolset-10 was ever built for
aarch64 there (the dts-11 collection ships binutils, elfutils and its own
metapackage for aarch64, but no compiler).

AlmaLinux 8 with gcc-toolset-14 is supported until 2029 and publishes the
toolset for x86_64 and aarch64 alike, so one image definition now covers both
architectures with one compiler version, replacing the hand-rolled CentOS 7
image and its vault/Rocky mirror repositories. almalinux:8 is a multi-arch
manifest, so a single BASE_IMAGE_GLIBC serves both.

Measured on the real artifact, built in this image for both architectures:

  max required symbol version   GLIBC_2.27  (both x86_64 and aarch64)
  symbols above 2.17            expf (2.27), getentropy (2.25)
  dynamic dependencies          libc, libdl, libm, libpthread, librt
  GLIBCXX/CXXABI references     none

So the runtime floor moves 2.17 -> 2.27, not to the base image's 2.28: symbol
versions are per-function. 2.27 keeps Ubuntu 18.04 (glibc 2.27) in support and
drops RHEL 7 (2.17), Amazon Linux 2 (2.26) and SLES 15 SP1 (2.26). Both
symbols are addressable with a .symver pin if a lower floor is needed later.

With the shipped arm64 build moved off the Debian image, that image no longer
sets a customer-facing floor, so it moves to trixie alongside amd64 -- which
also restores apt on the arm64 sanitizer jobs.

Names now describe the purpose rather than a version that has changed twice:
BUILD_IMAGE_{X64,ARM64}_GLIBC and BASE_IMAGE_GLIBC, with tag suffixes
{x64,arm64}-glibc-base. The variables still hold the current images until
rebuild-images.sh publishes the new tags.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* ci: move the C++ test toolchain into the build image

The sanitizer jobs installed cmake, libgtest-dev, libgmock-dev, binutils,
libc6-dbg and llvm in a before_script, which made every run depend on the
distro archive still serving them, and which does not survive the move to
trixie: sanitizer/asan_interface.h is no longer part of the llvm package
there, so compiling linearAllocator.cpp fails with

  fatal error: 'sanitizer/asan_interface.h' file not found

The header comes from libclang-rt-<n>-dev. The packages now live in the image
instead, using the unversioned libclang-rt-dev metapackage so the right clang
runtime is picked up on whatever release the image is built from.

Dropping the before_script override also stops these jobs bypassing the
pipeline default, so they get the CANCELLED guard and the maven proxy export
that every other job already had.

Verified on a locally built trixie image (clang 19.1.7, gtest 1.16.0),
aarch64: buildGtestAsan and buildGtestTsan both compile, and all 62 asan and
62 tsan binaries pass.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* ci: repin the build images onto the rebuilt bases

Pins from the rebuild in pipeline 137281788, which built every image from the
definitions on this branch. Verified by pulling each digest:

  x64-glibc-base    AlmaLinux 8.10, glibc 2.28, gcc 14.2.1
  arm64-glibc-base  AlmaLinux 8.10, glibc 2.28, gcc 14.2.1
  x64-base          Debian 13 trixie, glibc 2.41, clang 19.1.7
  arm64-base        Debian 13 trixie

The musl, datadog-ci and benchmark images come from the same rebuild; their
definitions did not change on this branch.

These pins belong here rather than in the image-update PR the rebuild opened
against main: main has no BUILD_IMAGE_*_GLIBC variables, so the two
shipped-binary pins silently did nothing there, while BUILD_IMAGE_ARM64 -- which
main still uses for build:arm64 and stresstest:arm64 -- would have moved the
shipped arm64 binary onto a trixie image and its glibc 2.41 floor.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* ci: point the base image's default at the release CI actually uses

The ARG default was still the bullseye digest while both OPENJDK_BASE_IMAGE
variables moved to trixie, and the comment justified it as "the more
conservative of the two" for an arm64 glibc floor that no longer exists here:
the shipped binaries come from .gitlab/base/el8/Dockerfile, so neither arch of
this image sets a customer floor.

It also stopped being a harmless default. With the package install no longer
ending in `|| true`, a direct `docker build` of this file -- or
rebuild-images.sh reaching its documented unset-variable fallback -- hit
bullseye's pruned security pool and failed on exactly the 404s this PR exists
to fix. Verified: building with no BASE_IMAGE argument now produces a trixie
image.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* ci: assert the shipped library's ABI floor in the build job

The oldest host the shipped library can load on is decided entirely by the
build image: the highest versioned glibc symbol it references sets the glibc
floor, and the static-link flags decide whether it needs a C++ runtime from
the host at all. Nothing checked either, so both could move without a source
change and without a failing job.

check-abi-floor.sh asserts both against SHIPPED_GLIBC_FLOOR, declared in
config.env next to the image pin that determines it, and build.sh runs it on
each glibc artifact before the job ends. musl targets carry no glibc symbol
versions and are skipped. Failures name the offending symbols or libraries,
since the point of the check is to say what needs fixing.

Three cases fail loudly rather than reporting a pass: an artifact with no
versioned glibc symbols, one with no NEEDED entries, and a sort that cannot
order versions. Each would otherwise leave the check quietly inoperative,
which is worse than not having it.

The checker reads the artifact through $OBJDUMP, so its tests drive it with
canned output and need no compiler or real shared object; they run in a new
ci-script-tests job alongside shellcheck. Every guard was mutation-checked:
disabling it individually turns a named assertion red.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* ci: pin expf to its GLIBC_2.17 interface and tighten the floor to 2.25

glibc 2.27 added a faster expf under a new symbol version and made it the
default, so building on AlmaLinux 8 made the shipped library require
expf@GLIBC_2.27 from source that has not changed. glibcCompat.h selects the
GLIBC_2.17 interface explicitly, which is still exported and still maintained;
.symver picks which of the versions already in the host's libc is called and
bundles nothing, so a host on a newer glibc still runs its own implementation.

Measured on the AlmaLinux 8 + gcc-toolset-14 build: expf moves from
GLIBC_2.27 to GLIBC_2.17 and the artifact's requirement drops from 2.27 to
2.25, so the floor follows to 2.25. The remaining above-2.17 symbol is
getentropy, which libstdc++ references from its own std::random_device and
which glibc exports in only one version -- no pin can reach it, so 2.17 waits
on removing the std::random_device uses.

The pin is guarded to glibc: musl has no symbol versioning and a .symver
naming a GLIBC_* version fails to link there, which would have broken both
musl targets. Verified by linking and running a TU that reaches expf through
poissonSampler.h under musl, where it emits no GLIBC_ references at all.

The floor check could not read the artifact the pin produces. objdump
parenthesises a binding to a non-default version, which is exactly what
.symver creates, and the parser matched only the bare form -- so it found no
versioned references on a pinned library. The empty-result guard turned that
into a loud failure rather than the silent pass it would otherwise have been,
which is what it exists for. The parser now accepts both forms, with fixtures
for a pinned artifact at and above the floor.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* ci: pin expf per architecture and run both script suites through bash

The expf pin named a version that does not exist on x86_64, so every link
against it failed there:

  nativeSocketSampler.o: undefined reference to `expf@GLIBC_2.17'
  no symbol version section for versioned symbol `expf@GLIBC_2.17'

A symbol's oldest version is the baseline of the glibc release that introduced
the port, which differs per architecture: x86_64 has expf@GLIBC_2.2.5 and
aarch64 expf@GLIBC_2.17, both alongside the GLIBC_2.27 default added in 2.27.
Naming a version the architecture does not export is a link error rather than a
fallback, so the pin now selects per architecture and an unlisted one gets no
pin at all, leaving the ABI floor check to report what it requires.

This took out every x86_64 job that links the library: the two sanitizer gtest
builds, and the CodeQL and Analyze jobs, which build linkRelease.

Verified by linking and running a translation unit that reaches expf through
poissonSampler.h under clang on Debian trixie -- the toolchain the sanitizer
jobs use -- on both architectures, and by building the release artifact on
AlmaLinux 8 for both: expf binds to each architecture's oldest version,
getentropy@GLIBC_2.25 is the only remaining reference above 2.17, and the floor
check passes at the declared 2.25 on both.

Separately, ci-script-tests invoked the test suites directly. includes_test.sh
is not executable and documents being run through bash, so the job failed with
exit 126 after its own assertions had all passed. Both suites now run through
bash, which leaves the existing file's mode alone.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* ci: lower the shipped floor to 2.17 now that getentropy is gone

#794 removed the std::random_device uses, so libstdc++'s getentropy reference
no longer enters the link and the last symbol above 2.17 is gone. With expf
already pinned per architecture, the artifact requires nothing newer than the
glibc that EL7 ships.

Measured on release builds in the AlmaLinux 8 image on both architectures: no
references above 2.17 at all, and the floor check passes at 2.17 where it
previously named getentropy@GLIBC_2.25. Both artifacts also load on Oracle
Linux 7.9 (glibc 2.17) with no unresolved symbol versions, which tests the
dynamic linker rather than the symbol table.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sphinx:critical Sphinx: critical — human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants