Skip to content

fix(geomodel): also hide std::__format vtables/typeinfo from exports#18

Merged
olantwin merged 1 commit into
mainfrom
fix/geomodel-hide-format-typeinfo
Jun 5, 2026
Merged

fix(geomodel): also hide std::__format vtables/typeinfo from exports#18
olantwin merged 1 commit into
mainfrom
fix/geomodel-hide-format-typeinfo

Conversation

@olantwin

@olantwin olantwin commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Broadens the linker version script from _ZNSt8__format* (functions only) to *NSt8__format* (functions + vtables + typeinfo + typeinfo names + any future std::__format mangling).
  • Bumps recipes/geomodel/recipe.yaml build.number to 4.

Why

PR #17 (_3) hid _ZN…-mangled std::__format functions and the local symbol audit went 4 → 0 weak _ZN exports per geomodel library. But ShipSoft/Geometry's PR #26 CI still crashes. PR #29 added a gdb --batch step to the failing workflow and captured this backtrace from run 27018773426:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7f90843 in std::__format::_Sink<char>::_M_write(...)
                    from /…/.pixi/envs/default/lib/libGeoModelWrite.so.6
#0  std::__format::_Sink<char>::_M_write(...)              from libGeoModelWrite.so.6
#1  std::__format::_Formatting_scanner<...>::_M_on_chars(...) from libGeoModelWrite.so.6
#2  std::__format::_Scanner<char>::_M_scan(this=0x7fffffffa2c0) at .../format:3942
#3  std::__format::__do_vformat_to<...>                    at .../format:4191
#4  std::vformat_to<std::__format::_Sink_iter<char>>(...)  at .../format:4221
#5  std::vformat[abi:cxx11](...)                           at .../format:4256
#6  std::format<int&, int&>(__fmt=…)                       at .../format:3764
#7  SHiPGeometry::CalorimeterFactory::buildStack(…)        at CalorimeterFactory.cpp:184

Why round 3 didn't fix it: nm -D libGeoModelWrite.so.6 | grep __format on the rebuilt _3 artifact still shows 15 V (vague-linkage) entries:

V _ZTINSt8__format5_SinkIcEE                                  typeinfo for _Sink<char>
V _ZTVNSt8__format14_Fixedbuf_sinkIcEE                        vtable for _Fixedbuf_sink<char>
V _ZTVNSt8__format9_Seq_sinkINSt7__cxx1112basic_string…       vtable for _Seq_sink<string>
V _ZTVNSt8__format19_Formatting_scannerINS_10_Sink_iterIcEEcEE  vtable for _Formatting_scanner
V _ZTSNSt8__format…                                            (6× typeinfo names)
V _ZTI…                                                        (6× typeinfo)

The _ZNSt8__format* glob doesn't match those — they start with _ZTV/_ZTI/_ZTS, not _ZN. So build_geometry's stack-allocated _Seq_sink<std::string> constructor still binds its vptr to libGeoModelWrite's exported vtable (libGeoModelWrite is first in DT_NEEDED), the virtual call dispatches into libGeoModelWrite's compiled-with-different-headers _M_write, and the class layout mismatch crashes.

*NSt8__format* matches the mangled namespace prefix wherever it appears in the symbol — functions (_ZN…NSt8__format…), vtables (_ZTVNSt8__format…), typeinfo (_ZTINSt8__format…), typeinfo names (_ZTSNSt8__format…). Each consumer .so keeps its own internally-emitted copies, ABI-consistent with the headers it was compiled against, and no cross-library vptr resolution happens.

Test plan

Summary by CodeRabbit

  • Chores
    • Updated build recipe and configuration for the geomodel package.

PR #17 added `-Wl,--version-script={local: _ZNSt8__format*;}` which
hid `_ZN…`-mangled function symbols. Local audit confirmed 0 weak
`_ZN` exports per geomodel lib. But CI debug PR (ShipSoft/Geometry#29)
captured a backtrace showing the crash is still in libGeoModelWrite:

  Program received signal SIGSEGV
  #0 std::__format::_Sink<char>::_M_write(…) from libGeoModelWrite.so.6
  #1 std::__format::_Formatting_scanner<…>::_M_on_chars(…) from libGeoModelWrite.so.6
  #2 std::__format::_Scanner<char>::_M_scan(…) at <format>:3942
  …
  #7 SHiPGeometry::CalorimeterFactory::buildStack(…) at CalorimeterFactory.cpp:184

`nm -D libGeoModelWrite.so.6 | grep __format` still shows 15 entries
(all `V` vague-linkage) for vtables (`_ZTV…`), typeinfo (`_ZTI…`), and
typeinfo names (`_ZTS…`) of _Sink<char>, _Buf_sink, _Fixedbuf_sink,
_Seq_sink<string>, _Scanner, _Formatting_scanner. Those are not matched
by `_ZNSt8__format*` (they start with `_ZTV`/`_ZTI`/`_ZTS`, not `_ZN`),
so build_geometry's `_Seq_sink<string>` constructor still binds its
vptr to libGeoModelWrite's vtable, and the virtual call lands in
libGeoModelWrite's compiled-with-different-headers `_M_write`.

Broaden the version script pattern to `*NSt8__format*` so vtables,
typeinfo, typeinfo names, and any future nested std::__format symbols
all get localized. Bump build.number to 4.
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3773027c-6f00-48f0-abf6-c2721416147a

📥 Commits

Reviewing files that changed from the base of the PR and between 5cc699a and 6a1481b.

📒 Files selected for processing (2)
  • recipes/geomodel/build.sh
  • recipes/geomodel/recipe.yaml

📝 Walkthrough

Walkthrough

The PR refines the geomodel recipe's linker version script to hide std::__format symbols using a more inclusive pattern matching strategy, and bumps the build metadata to version 4 to reflect this change.

Changes

Geomodel symbol hiding and build metadata

Layer / File(s) Summary
Linker version script and build metadata update
recipes/geomodel/build.sh, recipes/geomodel/recipe.yaml
The hide-std-format.ver generation updates its local: pattern from _ZNSt8__format* to *NSt8__format* to cover all std::__format mangling variants (functions, vtables, typeinfo/name symbols), with expanded inline documentation. The recipe build number is incremented from 3 to 4.

Possibly related PRs

  • ShipSoft/ship-conda-recipes#17: Both PRs modify recipes/geomodel/build.sh to generate/pass the hide-std-format.ver linker version script and both bump recipes/geomodel/recipe.yaml build metadata.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: broadening the linker version script pattern to hide std::__format vtables and typeinfo symbols in addition to functions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/geomodel-hide-format-typeinfo

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@olantwin olantwin merged commit 698bc78 into main Jun 5, 2026
2 checks passed
olantwin added a commit to ShipSoft/Geometry that referenced this pull request Jun 5, 2026
ShipSoft/ship-conda-recipes#18 broadened the linker version script to
`*NSt8__format*` so std::__format vtables, typeinfo, and typeinfo
names are localized along with the function symbols PR #17 already
hid. Local symbol audit now reports 0 V|W St8__format exports in
all four affected geomodel libs (was 0 W but 15 V on build 3), and
the full test suite passes 25/25 in this environment.
@olantwin olantwin deleted the fix/geomodel-hide-format-typeinfo branch June 5, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant