fix: include mvec for compilation - #120
Open
botamochi0x12 wants to merge 1 commit into
Open
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Author
|
NOTE: Not a duplicate of ryoga-chan's fork. Diffed ryoga-chan/phashion's |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Abstract
This PR fixes a
symbol lookup error: ... undefined symbol: _ZGVbN2v_cos(see #90 and #101 ) that occurs at
runtime on Linux systems whose GCC/glibc auto-vectorize
cos()into alibmvecSIMD variant.
extconf.rbonly linked-lm, not-lmvec, so the symbol wasleft unresolved until the code path that calls it actually ran.
Background
The bundled
pHash-0.9.6/configure.acalways builds with-O3 -ffast-mathunless
--enable-debugis passed (ref:pHash-0.9.6/configure.ac), which lets GCC substitute
cos()calls with glibc's SIMD variant_ZGVbN2v_cosfromlibmvec(ref: vector math ABI).
extconf.rb's$LIBSonly listed-lm, so that symbol stayed unresolveduntil code that calls it actually ran.
Reproducing the crash turned out to be inconsistent by build path:
Debian/Ubuntu's
libm.sois itself a linker script that sometimes pulls inlibmvecautomatically viaAS_NEEDED, which is why localbundle exec rake compilebuilds don't always show the crash. Installingthe actual published gem (
gem install phashion, i.e. what every real userdoes) reproduces it reliably. See the Appendix for the experiments that
pinned this down.
Contribution
Add
-lmvecto$LIBSinext/phashion_ext/extconf.rbso the builtextension carries an explicit NEEDED entry for
libmvec.so.1and thesymbol resolves at load time regardless of build path or distro linker-script
behavior.
Before
After
Caveats
This PR may conflict with #105.
Environment & Appendix
Environment
Appendix
macOS (Darwin) should not trigger this error
_ZGVbN2v_cosis glibc's vector-math ABI: glibc's<math.h>annotatesfunctions like
cos()so that GCC's auto-vectorizer, under-O3 -ffast-math, can substitute calls to the SIMD variant implemented inlibmvec. This mechanism is specific to GNU/Linux:libSystem/libm, not glibc, and has nolibmvecorglibc-style
bits/math-vector.hdeclarations forcos()to hook into.gccbinary onPATHis normally a Clang shim), which doesn't perform this substitution.
extconf.rbdoesn't overrideCC/CXXon Darwin, so the default Clangtoolchain is what actually builds the extension.
system libm, not glibc, so it has nothing to vectorize into.
We have not reproduced this on real macOS hardware to confirm empirically —
this conclusion is based on the above reasoning about the toolchain, not a
direct test. Flagging as an assumption rather than a verified fact.
Experiments for completeness (Docker)
Verified in disposable containers, discarded after each run:
Debian 12 (
ruby:3.4-bookworm, GCC 12.2.0, glibc 2.36, binutils 2.40).Built from a pristine
git archive HEADcheckout (pre-fix,-lmonly)via both
bundle exec rake compileand a bareruby extconf.rb && make.Neither crashed —
readelf -dshowedlibmvec.so.1pulled in anyway viathe
AS_NEEDEDclause in/usr/lib/x86_64-linux-gnu/libm.so.Ubuntu 24.04 (
ubuntu:24.04+ distro-packagedruby-full3.2.3, GCC13.3.0, binutils 2.42). Same pristine pre-fix checkout, same two build
paths. Again did not crash for the same
AS_NEEDEDreason.Ubuntu 24.04 + RVM Ruby 3.4.7 (matches the reporter's environment
exactly, GCC 13.3.0, binutils 2.42). Pristine pre-fix checkout built via
bundle exec rake compileorruby extconf.rb && make— still did notcrash. But installing the published gem the normal way:
reproduced the crash reliably, every time.
readelf -don thatgem install-built.soshows nolibmvec.so.1NEEDEDentry at all —unlike the rake-compiler-built copies in the same container.
Rebuilding as a
.gemfrom the fixed source and installing that(
gem build phashion.gemspec && gem install ./phashion-1.2.0.gem) adds anexplicit
libmvec.so.1 NEEDEDentry and the same script now returnstruewithout crashing.Takeaway:
bundle exec rake compile(what this project's own test suiteand CI use) is not a reliable way to catch this class of bug — only building
through RubyGems' own extension builder (
gem install/gem build && gem install) reproduces it consistently. The explicit-lmvecfix removes the dependency on the implicit, build-path-sensitiveAS_NEEDEDbehavior entirely, so it holds regardless of which path is usedto build the extension.