fix(Calorimeter): avoid std::format to work around libstdc++ ABI clash#27
fix(Calorimeter): avoid std::format to work around libstdc++ ABI clash#27olantwin wants to merge 3 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 24 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
All dependencies are now available from the prefix.dev/ship channel (via ship-conda-recipes), so CI no longer needs CVMFS mounts, the CERN container, or a self-hosted runner. The new workflow runs on ubuntu-latest with prefix-dev/setup-pixi and a single `pixi run test` step that configures, builds, and runs ctest.
The OverlapCheck CI test (apps/build_geometry on the GitHub Actions ubuntu-latest runner) segfaulted on the first std::format call in CalorimeterFactory::buildStack. A gdb backtrace from CI showed the crash inside std::__format::_Sink<char>::_M_write — resolved at runtime into libGeoModelWrite.so.6 rather than libstdc++.so.6. The conda geomodel package ships libGeoModelWrite built against an older libstdc++ (GLIBCXX_3.4.31, GCC 12). It emits its own weak template instantiations for std::__format internals. build_geometry links libGeoModelWrite ahead of libstdc++ in DT_NEEDED order, so on the runner the dynamic linker picks the libGeoModelWrite copies of the format machinery, whose binary layout does not match what our GCC 14 headers expect — and the first call segfaults. The CalorimeterBuilds, TimingDetectorBarCount, and BuilderTest tests pass because they're not linked against libGeoModelWrite, so the bad symbols never appear in their address space. Replace std::format with std::to_string + string concatenation in the Calorimeter sources (CalorimeterFactory, CaloBarLayer, CaloFibreHPLayer). These call into well-established libstdc++ symbols (operator+, std::to_string) that are not duplicated by libGeoModelWrite, so they cannot be interposed. This can be reverted once libGeoModelWrite is rebuilt against a matching libstdc++.
9559425 to
ab3487f
Compare
The geomodel artifact currently on prefix.dev was built against an older libstdc++ (GLIBCXX_3.4.31, GCC 12) and emits weak std::__format template instantiations from libGeoModelWrite.so.6. Downstream consumers built with GCC 14 (current conda-forge cxx-compiler) hit those weak symbols ahead of libstdc++ in DT_NEEDED order, and the layout mismatch crashes the first std::format call (see ShipSoft/Geometry#27). Bumping build.number forces rattler-build to produce fresh artifacts under the current conda-forge pinning, restoring ABI compatibility. shipgeometry, shipgeometryservice, and aegir are bumped alongside so the channel ships a consistent set of binaries linked against the rebuilt geomodel.
Summary
Fixes the
OverlapChecksegfault that #26 surfaced on GitHub Actions ubuntu-latest.Root cause
A gdb backtrace captured on the runner showed
apps/build_geometrycrashing insidestd::__format::_Sink<char>::_M_writeresolved intolibGeoModelWrite.so.6rather than libstdc++.The conda
geomodelpackage ships libGeoModelWrite built against an older libstdc++ (GLIBCXX_3.4.31, GCC 12) and emits weakstd::__formattemplate instantiations in that library.build_geometrylinkslibGeoModelWrite.so.6ahead oflibstdc++.so.6in DT_NEEDED order, so the dynamic linker picks the libGeoModelWrite copies of the_Sink/_Scannerinternals — whose binary layout does not match what our GCC 14 headers expect. The firststd::formatcall (inCalorimeterFactory::buildStack, line 184 pre-fix) then crashes.The other tests pass on CI because they aren't linked against libGeoModelWrite (no clash).
Change
Replace
std::formatwithstd::to_string+ string concatenation in the three Calorimeter source files. The replacements call into well-established libstdc++ symbols that aren't shadowed by libGeoModelWrite.This can be reverted once libGeoModelWrite is rebuilt against a matching libstdc++.
Test plan