-ffile-prefix-map=<staging>=. and --build-id injection in compiler-wrapper - #19
SebastianPaucar wants to merge 8 commits into
Conversation
…pack/compiler-wrapper/pull/19) Signed-off-by: SebastianPaucar <paucar.sebastian@hotmail.com>
| fi | ||
|
|
||
| # -ffile-prefix-map=<staging>=. injection for build reproducibility | ||
| if [ -n "${SPACK_DEBUG_PREFIX_MAP:-}" ]; then |
There was a problem hiding this comment.
This is a compiler flag, it should not be passed to the linker (see mode).
There was a problem hiding this comment.
Just updated. Thanks for pointing it out! Added mode guard on it, so the compiler flag is injected during compilation, not linking.
…tests added in test/run.sh (test_debug_prefix_map) Signed-off-by: SebastianPaucar <paucar.sebastian@hotmail.com>
7edd455 to
215a209
Compare
…cted-args ordering (all tests pass successfully) Signed-off-by: SebastianPaucar <paucar.sebastian@hotmail.com>
Signed-off-by: SebastianPaucar <paucar.sebastian@hotmail.com>
aa6dd20 to
2c72cf6
Compare
|
@SebastianPaucar Can you fix the shellcheck checks that are failing here? |
|
I think this is also ready for review, right? |
…st fixtures accordingly
Done, just fixed the shellchecks failures |
Prototype version pulling in --build-id/-Wl,--build-id and -ffile-prefix-map injection from spack/compiler-wrapper#19 ahead of an official release.
Prototype version pulling in --build-id/-Wl,--build-id and -ffile-prefix-map injection from spack/compiler-wrapper#19 ahead of an official release. Fix shellcheck failures.
|
@haampie Please review this. |
| # -ffile-prefix-map=<staging>=. injection for build reproducibility | ||
| case "$mode" in | ||
| cpp|as|cc|ccld) | ||
| append flags_list "-ffile-prefix-map=${SPACK_PREFIX_MAP}=." |
There was a problem hiding this comment.
This argument is not supported by all compilers that Spack supports. This needs to be refactored in such a way that Spack is still usable with older compilers and compilers that do not support this option.
Reference table generated by Gemini:
Compiler Suite | Supports -ffile-prefix-map? | Notes / Engine
________________________________________________
GCC (8+) | Yes | Native implementation.
Clang / LLVM (10+) | Yes | Native implementation.
Intel oneAPI (icx) | Yes | Inherited from Clang frontend.
Intel Classic (icc) | No | Proprietary engine.
IBM Open XL | Yes | Modern Clang-based frontend.
IBM Classic XL | No | Proprietary engine.
Fujitsu (fcc) | Yes / No | Supported only in Clang Mode (-Nclang).
NVHPC (nvc/nvfortran) | No | Uses LLVM backend but rejects this frontend flag.
NAG Fortran (nagfor) | No | Completely standalone, proprietary architecture.
MSVC | No | Uses native Microsoft engine.
There is no need to support MSVC, since that doesn't use the same compiler wrapper. But this needs to support:
intel-oneapi-compilers-classicnvhpcgcc@:7clang@:9xl@:15andxl_r@:15fujitsuin non-clang modenag
Most likely, the best option is to include the option name in the variable we emit from Spack. Then individual compilers can either override it with their own option for the same capability, or set it to empty if they lack the support entirely.
There was a problem hiding this comment.
Thanks @becker33 for reviewing this. Just updated. Checks pass successfully.
Yes, -ffile-prefix-map is GCC/Clang specific and would break against other compilers that don't support it.
I redesigned this so cc.sh no longer needs to know the compiler-specific flag name for the prefix remapping. It is wired in the new changes I made in spack/spack-packages#5353. Now each compiler package declares its own support (right now available for gcc, llvm, and intel-oneapi-compilers), but other compilers like nvhpc, nag, xl, etc. could also be plugged. The complete flag string is passed from compiler_wrapper/package.py via the SPACK_PREFIX_MAP_ARGS and SPACK_BUILD_PREFIX_MAP_ARGS shell variables, and cc.sh just appends them. cc.sh is agnostic to the specific flag throughout this process.
| append flags_list "--build-id" ;; | ||
| ccld) | ||
| append flags_list "-Wl,--build-id" ;; |
There was a problem hiding this comment.
This needs to be abstracted into a variable (which can be set empty or unset on macos)
There was a problem hiding this comment.
@becker33 to clarify, should --build-id injection be controlled by platform detection only, or let it be user-configurable (via config, or even a variant)? The purpose of adding --build-id is to let binaries built with this compiler wrapper be identified by an ID that GDB can automatically look up for debugging.
Indeed, the combination of -ffile-prefix-map + --build-id is meant to make debug info GDB-findable automatically, with no machine-specific build paths embedded in DWARF. I was thinking of this PR as a proposal for a new compiler-wrapper release (e.g. compiler-wrapper v1.1.1) to get reproducible (and redistributable) debug info and debuggable installations.
There was a problem hiding this comment.
I was just thinking use SPACK_BUILD_ID_ARGS and then the compilers can set it appropriately based on the platform, I don't think we want to make it user configurable.
So gcc could have:
setup_dependent_build_environment(self, env, dependent_spec):
if dependent_spec.satisfies("platform=linux"):
env.set("SPACK_BUILD_ID_ARGS", "--build-id")
And then the wrapper could use:
case "$mode" in
ld)
append flags_list "$SPACK_BUILD_ID_ARGS" ;;
ccld)
append flags_list "$linker_arg$SPACK_BUILD_ID_ARGS" ;;
with a guard above to only do this if SPACK_BUILD_ID_ARGS is set and non-empty.
There was a problem hiding this comment.
@becker33 Thanks for this direction. Updated. SPACK_BUILD_ID_ARGS is now set per-compiler-package per-platform (can see those new commits for --build-id in spack/spack-packages#5353), so cc.sh just appends it via $linker_arg when non-empty. Verified with spack build-env that SPACK_BUILD_ID_ARGS reaches the real build environment correctly. Added a test to cover the unset case. All tests pass.
|
@becker33 PR description updated, suggested changes made, and I've requested review again. This PR is ready for another look. As mentioned before, this is the foundation for debuggable installations and debug info redistribution in Spack, worked on with @wdconinc and previously discussed with @haampie. Happy to dig into any observations you can point out. |
This PR makes compiler-wrapper's debug-reproducibility flags compiler-agnostic, per recent @becker33 review that hardcoding
-ffile-prefix-mapand--build-idwould break unsupported compilers and platforms.Changes:
cc.shno longer hardcodes flag names or does platform detection. It appendsSPACK_PREFIX_MAP_ARGS/SPACK_BUILD_PREFIX_MAP_ARGS(file-prefix remapping) andSPACK_BUILD_ID_ARGS(--build-id) only when set and supported (build-idwrapped in$linker_arg).SPACK_PREFIX_MAP_ARGSfor-ffile-prefix-map=<stage>=.andSPACK_BUILD_ID_ARGSfor--build-idinjection at compiler-wrapper level (companion to spack/compiler-wrapper#19) spack-packages#5353).test/run.sh: renamed vars and added tests for the unsupported/unset case. All checks pass.Companion PR: spack/spack-packages#5353
@haampie @becker33 could you take another look when you have a chance?
cc @wdconinc @tgamblin