fusion: don't let a broken llvm-nm silently drop the symbol prefix#137
Open
atassis wants to merge 1 commit into
Open
fusion: don't let a broken llvm-nm silently drop the symbol prefix#137atassis wants to merge 1 commit into
atassis wants to merge 1 commit into
Conversation
KernelCompilationRule._prefix_symbols renames each fused kernel object's symbols with `nm --defined-only --extern-only obj | awk '...' > map` and then `objcopy --redefine-syms=map`. Two things combine into a silent failure. First, _find_tool returns the first existing llvm-nm/llvm-objcopy even if that binary cannot actually run, for example one with an unsatisfied shared library dependency. Second, in `nm | awk > map` the pipeline's exit status is awk's, so a failure of nm is masked: the map comes out empty, objcopy renames nothing, and every fused object keeps its bare symbol, which surfaces only much later as `undefined symbol: <prefix><sym>` at the per-core link. This fixes both. It adds _find_working_tool, which probes each candidate with `--version` and skips a present-but-unrunnable binary, falling through to the system PATH copy (and finally to _find_tool, so the existing not-found error is preserved). And it runs nm to a file joined to awk with `&&`, so a failing nm aborts the build loudly instead of yielding an empty map. Both changes are no-ops when the tools work.
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.
Summary
KernelCompilationRule._prefix_symbolsbuilds a redefine-syms map withnm --defined-only --extern-only obj | awk '...' > mapand applies it withobjcopy --redefine-syms=map. Ifnmfails, the map comes out empty,objcopyrenames nothing, and everyfused kernel object keeps its bare symbol, which surfaces only much later as
undefined symbol: <prefix><sym>at the per-core link. Two things make that failure silent:
_find_toolreturns the first existingllvm-nm/llvm-objcopyeven if that binary cannot actuallyrun (for example one with an unsatisfied shared-library dependency).
nm | awk > mapthe pipeline's exit status isawk's, so a failure ofnmis masked.Fix
_find_working_tool, which probes each candidate with--versionand skips a present-but-unrunnablebinary, falling through to the system PATH copy and finally to
_find_tool(so the existing not-founderror is preserved).
_prefix_symbolsand_rename_symbolsuse it to selectllvm-nm/llvm-objcopy.nmto a file joined toawkwith&&, so a failingnmaborts the build loudly instead ofyielding an empty map.
Both changes are no-ops when the tools work: the same binary is selected and the same map is produced.
Testing
Reproduced the original silent failure by selecting an
llvm-nmthat exits nonzero; before the change thefused link failed with
undefined symbol: <prefix><sym>, and after it the build either succeeds (a workingnmis found on PATH) or aborts at the map step withnm's own error, instead of failing opaquely at linktime. No behavior change when
nm/objcopyrun normally.