fix(arpack): pass Fortran hidden character-length arguments in ARPACK calls#990
Conversation
… calls
The extern "C" prototypes for the ARPACK drivers omitted the hidden
length argument that Fortran compilers append for every CHARACTER dummy
(bmat, which, howmny). Calling through such prototypes is undefined
behavior: it happens to be benign for ARPACK's fixed-length dummies with
today's gfortran ABI, but is not guaranteed by any standard and is the
kind of mismatch that surfaces as platform-specific corruption.
Add trailing std::size_t lengths to all twelve x{s,n}aupd_/x{s,n}eupd_
prototypes (size_t matches gfortran >= 8 and LAPACK's FORTRAN_STRLEN
convention; on LP64 little-endian targets it is also register-compatible
with the int lengths of older compilers), extend the std::function
dispatch signatures in Lanczos.cpp/Arnoldi.cpp accordingly, and pass the
actual lengths (bmat: 1, which: 2, howmny: 1) at every call site.
Cleanup noted when closing #974 (whose segfault was environmental, not
caused by this — see the issue for the root cause).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the ARPACK C declarations in arpack_wrapper.hpp and their corresponding function signatures and calls in Arnoldi.cpp and Lanczos.cpp to include hidden string length arguments (std::size_t) for CHARACTER arguments. This aligns with Fortran's calling convention to prevent undefined behavior. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #990 +/- ##
=======================================
Coverage 30.88% 30.88%
=======================================
Files 229 229
Lines 34758 34758
Branches 14409 14409
=======================================
Hits 10734 10734
Misses 16720 16720
Partials 7304 7304
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Code ReviewAdds the Fortran hidden Verified correct
Notes (non-blocking)
No findings requiring changes — a well-reasoned, well-documented ABI-correctness fix. LGTM. Posted by Claude Code on behalf of @pcchen |
pcchen
left a comment
There was a problem hiding this comment.
Approving. Correct, clean Fortran/C ABI fix: the 12 ARPACK extern "C" prototypes and their call sites now carry the hidden CHARACTER-length arguments — right count and order per function (aupd: bmat,which → 1,2; eupd: howmny,bmat,which → 1,1,2), consistent std::function dispatch signatures, and std::size_t chosen for ABI-compatibility on LP64 little-endian with both the modern (gfortran ≥8) and legacy (int) hidden-length conventions. Strictly a UB-safety improvement with no behavior change, and CI is green on all platforms including the Arnoldi/Lanczos suites that exercise these calls.
Posted by Claude Code on behalf of @pcchen
Summary
The
extern "C"prototypes ininclude/backend/arpack_wrapper.hppomitted the hidden length argument that Fortran compilers append for everyCHARACTERdummy (bmat,which,howmny). Calling through such prototypes is undefined behavior. It happens to be benign for ARPACK's fixed-length dummies under today's gfortran ABI — and it was explicitly not the cause of the macOS segfaults investigated in #974 (those were environment-induced; see that issue) — but it is exactly the class of ABI mismatch that surfaces as platform-specific corruption, and was flagged as a cleanup follow-up when closing #974.Changes
x{s,n}aupd_/x{s,n}eupd_prototypes gain trailingstd::size_tlength parameters, one perCHARACTERargument, with a comment documenting the convention.std::size_tmatches gfortran ≥ 8 and LAPACK's modernFORTRAN_STRLENconvention; on LP64 little-endian targets it is also register/stack-slot compatible with theintlengths used by older compilers.std::functiondispatch signatures extended to match, and every call site now passes the actual lengths (bmat: 1,which: 2,howmny: 1).No functional change is expected on any platform where the code currently works: the callee already expected these arguments; we now actually pass them.
Verification
On macOS arm64 (AppleClang, conda-forge arpack-ng 2.1.0 + OpenBLAS), with the #972 test compile fixes applied locally (not part of this branch):
test_mainbuild succeeds;*Lanczos*+Arnoldi*pass (run withoutDYLD_LIBRARY_PATH, per the pitfall documented in macOS: ARPACK-backed linalg::Lanczos / Arnoldi tests segfault in dsaupd_ (null dereference) #974/fix(tests): make the test suite build on macOS/AppleClang #972) — covering the d/s symmetric, d/s non-symmetric, and z/c complex driver pairs.Linux/GCC CI exercises the same call paths and should show no change.
Related: #974 (root-cause investigation that flagged this), #626 (original ARPACK integration), #893 (previous wrapper cleanup, exonerated in #974).
🤖 Generated with Claude Code