fix(pathfinder): use exact dynamic library names - #2689
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test ab26c96 |
Restrict CUPTI filesystem discovery to descriptor-declared DLL names and remove the now-unused fallback-glob metadata and search machinery. This keeps filesystem discovery consistent with already-loaded detection and native loading, avoiding undeclared wildcard matches.
Use one descriptor-defined newest-first candidate list for filesystem discovery, RTLD_NOLOAD checks, and native loading. Remove the generic unversioned and glob fallbacks so undeclared ABI names cannot be found on disk while remaining invisible to resident checks. Correct the cufftMp catalog ordering so ABI 12 is preferred over ABI 11. Layouts must provide an exact declared SONAME alias; physical patch filenames without that alias are intentionally not discovered.
ab26c96 to
f20ad66
Compare
CUDA 12.9 NVVM wheels expose libnvvm.so without a libnvvm.so.4 alias. Declare that exact artifact name so strict descriptor-driven discovery supports the wheel without restoring an implicit fallback.
|
/ok to test 6eb600b |
This comment has been minimized.
This comment has been minimized.
|
/ok to test aae8753 |
|
/ok to test |
isVoid
left a comment
isVoid
left a comment
There was a problem hiding this comment.
It generally lgtm pending one / 2 question(s): isn't such change in library discovery behavior considered a breaking change in cuda-pathfinder?
|
|
||
| def linux_soname_candidates(desc: LibDescriptor) -> tuple[str, ...]: | ||
| """Return declared Linux SONAMEs in runtime preference order.""" | ||
| # The catalog is authored oldest -> newest; loading prefers newest -> oldest. |
There was a problem hiding this comment.
Why do we have to keep two orders? If all we need is newest -> oldest, should we enforce it when we author it, then just use as-is? Also, I'm not sure if the "oldest -> newest" description is 100% accurate, for instance, nvvm soname:
linux_sonames=("libnvvm.so", "libnvvm.so.4"),
There isn't a chronological order between these two names.
There was a problem hiding this comment.
Good point. The two orders are purely historical. When #921 introduced newest-first lookup, the tables remained in ascending, old-to-new order and the runtime reversed them. Keeping the descriptor catalog in the old order creates an opportunity for ordering mistakes.
You are also right that "oldest to newest" is not accurate for aliases such as libnvvm.so and libnvvm.so.4; those names have a preference relationship, not a chronological one. The intended NVVM runtime order is libnvvm.so.4 first, followed by the explicitly declared unversioned wheel filename.
I agree that the cleaner model is to author candidate names directly in most-preferred-first runtime order and consume them as-is. However, that's a very noisy change. I think it's best to do that in a follow-up PR, but before releasing 1.8.0.
The rationale for deferring the reordering to a follow-on PR is:
- Reordering the full catalog is a large, noisy mechanical diff that deserves a focused review rather than obscuring the wildcard-removal change here.
- I want to use the same follow-up to review ordering and preference semantics in the other Pathfinder components - binary utilities, headers, and static libraries - so we establish one coherent model rather than changing only the dynamic-library catalog in isolation.
Strictly speaking yes, you're right, in the behavioral sense, this is a breaking change for an installation that relies exclusively on an implicit or wildcard-only library filename. A case that succeeded previously can now fail with To lay out my initial thinking:
However, none of this changes that the fallback removal is not fully backward-compatible. The Linux fallback was documented in the 1.5.4 release notes, and the CUPTI wildcard fallback was documented in the 1.7.0 release notes. In light of this compatibility distinction, I agree that the conservative treatment is to call it out as a subtle breaking discovery change and target |
|
@isVoid could you please take another look? Only the release notes changed, therefore I'm not re-triggering the CI now, pending your feedback. |
|
2 participants
Description
This PR removes
cuda-pathfinder's remaining wildcard and implicit filename fallbacks for dynamic-library discovery. Filesystem discovery, already-loaded detection, and native loading now use the same descriptor-declared library names, in the same newest-first runtime preference order.Background and motivation
The wildcard fallbacks date to the earliest Pathfinder implementation. At that time, the library metadata and our model of the relevant package and installation layouts were still incomplete. Looking for
lib<name>.so*on Linux and, historically,<name>*.dllon Windows was a pragmatic way to cover filenames that had not been modeled explicitly.Pathfinder is now production infrastructure for multiple projects, and the descriptor catalog has become the precise source of truth for supported library names, layouts, platforms, architectures, and dependencies. The old fallbacks have therefore become both unnecessary and inconsistent with the rest of the loader.
In particular, wildcard matching applied only to filesystem discovery.
check_if_already_loaded_from_elsewhere()and native system loading have always needed concrete names. A wildcard-only filename could consequently be found and loaded by absolute path but remain invisible to an uncached already-loaded check. The same problem applies when such a library was loaded by another component before Pathfinder runs. Pathfinder could then attempt to load another candidate; if that candidate resolves to a different path or version, multiple versions of the library could end up in the process.The detailed work on #2680 required a cross-cutting audit of filesystem matching, native loading, and already-loaded detection. That work accumulated the context needed to revisit this older design. This cleanup is intentionally kept in a separate PR so it did not delay #2680's user-facing cuDNN and NCCL features or their release in
cuda-pathfinder1.7.0.Changes
This PR makes descriptor-declared filenames the single identity model used by the dynamic-library loader:
RTLD_NOLOADalready-loaded checks, and native loading now share one descriptor-declared candidate list, in newest-first runtime order..so*match and implicit genericlib<name>.socandidate are removed. An unversioned name remains supported when the descriptor declares it explicitly.libnvvm.sofilename used by the CUDA 12.9 NVVM wheel is declared explicitly, preserving that layout without restoring an implicit fallback.cufftMpSONAME order is corrected so ABI 12 is preferred over ABI 11.cuda-pathfinder1.8.0 release.Compatibility and maintenance model
The main compatibility risk is a valid installation whose library is exposed only under a filename that is missing from the descriptor catalog. Such a layout will now produce a clear not-found error instead of being accepted by a wildcard. On Linux, a physical patch-versioned file remains supported when the directory also exposes a descriptor-declared SONAME alias; a patch filename by itself is no longer treated as sufficient identity.
This is an intentional fail-closed tradeoff. The previous behavior has a known correctness flaw: it can load an undeclared filename that the other loader paths cannot subsequently identify. The bounded alternative risk is a visible catalog omission, which is deterministic and straightforward to repair.
The CUDA 12.9 NVVM wheel exercised exactly this failure mode during this PR's CI: it exposes
libnvvm.sowithout alibnvvm.so.4alias. Adding that exact artifact name to the descriptor restored support while preserving the strict model. If another supported.soor.dllfilename is found to be missing, we will treat it as a descriptor-catalog bug, add the exact name, and issue acuda-pathfinderbug-fix patch release.Focused tests cover exact-name discovery, newest-first ordering, explicitly declared unversioned names, and rejection of undeclared ABI, patch-version, backup-like, and Windows CUPTI filenames. The full CI matrix also exercises real NVIDIA wheels and local CUDA installations across Linux x86-64, Linux ARM64, and Windows.
Checklist