Skip to content

fix: make zig cc pass -l/-L like Clang/GCC for ELF #19818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

a-khabarov
Copy link

This PR is my attempt to fix #19699 by making the way zig cc passes -l/-L flags for ELF linking consistent with Clang and GCC.
I've added a test case that mimics the example in the issue description. Without the changes this PR makes to src, the new test case fails as expected with

========= expected to find: ==========================
NEEDED libfoo.so
========= but parsed file does not contain it: =======
dynamic section
RUNPATH foo
NEEDED foo/libfoo.so
NEEDED libc.so.6

I am not sure if test/tests.zig is the best place for it - I couldn't find a good way to test this in test/link/elf.zig or test/standalone.

@a-khabarov
Copy link
Author

@motiejus @kubkon This PR builds on the functionality implemented in #15743 and should also fix cc_test not working correctly when using hermetic_cc_toolchain as mentioned in #15743 (comment).

@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from 5f810f0 to 1860048 Compare May 20, 2024 12:05
@a-khabarov a-khabarov requested a review from kubkon May 20, 2024 12:23
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from 9107b5a to 8d5fc36 Compare June 4, 2024 15:38
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from b4dcd83 to 094faf4 Compare June 17, 2024 10:02
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from 094faf4 to b3cfcab Compare July 5, 2024 16:08
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from b3cfcab to f211d2d Compare July 26, 2024 12:26
@alexrp alexrp added this to the 0.14.0 milestone Feb 19, 2025
@a-khabarov
Copy link
Author

I am in the process of rebasing this on current mainline

@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from f211d2d to 46752f5 Compare February 21, 2025 15:57
@a-khabarov a-khabarov changed the title fix: make zig cc pass -l/-L like Clang/GCC for ELF fix: make zig cc pass -l/-L like Clang/GCC for ELF Feb 21, 2025
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from 46752f5 to 2af0d55 Compare February 21, 2025 17:02
@a-khabarov
Copy link
Author

I've rebased this PR.

The fix itself is in src/link/Elf.zig.

The new test is in test/link/elf.zig. It follows the example from
#19699.

The rest of the changes are new functionality needed for the test:

  • omit_soname option for Build.Module, OverlayOptions.
    This passes -fno-soname to the linker when enabled.
  • addLibraryPathSpecial for Build.Module, Build.Step.Compile.
    Like addLibraryPath but for []const u8.
    addRPathSpecial is also added for completeness.
  • setCwd for Build.Step.Compile.
    Allows setting current directory for compile steps.
  • opt_cwd for evalZigProcess in Build.Step.
    Allows running evalZigProcess in a specific directory.
    Needed for setting current directory for compile steps.
  • --zig-lib-dir is now appended differently in Build.Step.Compile.
    This is needed for compatibility with setCwd.

@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from 2af0d55 to b8f46fa Compare February 24, 2025 19:28
@a-khabarov
Copy link
Author

I made some changes to avoid reconstructing the library name and directory in src/link/Elf.zig.
Everything is now passed via src/link.zig

a-khabarov added a commit to a-khabarov/zig that referenced this pull request Feb 25, 2025
This commit adds `addLibraryPathSpecial` for `Build.Module` and
`Build.Step.Compile`. Like `addLibraryPath` but for `[]const u8`.
`addRPathSpecial` is also added for `Build.Step.Compile` for
completeness.

This is needed to implement the new test in
ziglang#19818.
a-khabarov added a commit to a-khabarov/zig that referenced this pull request Feb 25, 2025
This commit adds the `soname` option for `Build.Module` and
`OverlayOptions`. This controls whether `-fsoname` or `-fno-soname` is
passed to the linker.

This is needed to implement the new test in
ziglang#19818
a-khabarov added a commit to a-khabarov/zig that referenced this pull request Feb 25, 2025
We need this to implement the new test in
ziglang#19818

This commits adds:

* `setCwd` for `Build.Step.Compile`.
  Allows setting current directory for compile steps.
* `opt_cwd` for `evalZigProcess` in `Build.Step`.
  Allows running `evalZigProcess` in a specific directory.
  Needed for setting current directory for compile steps.

`--zig-lib-dir` is now appended differently in `Build.Step.Compile`.
This is needed for compatibility with `setCwd`.
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from b8f46fa to 077f287 Compare February 25, 2025 15:37
a-khabarov added a commit to a-khabarov/zig that referenced this pull request Feb 25, 2025
This commit adds the `omit_soname` option for `Build.Module` and
`OverlayOptions`. This controls whether `-fsoname` or `-fno-soname` is
passed to the linker.

This is needed to implement the new test in
ziglang#19818
a-khabarov added a commit to a-khabarov/zig that referenced this pull request Feb 25, 2025
We need this to implement the new test in
ziglang#19818

This commits adds:

* `setCwd` for `Build.Step.Compile`.
  Allows setting current directory for compile steps.
* `opt_cwd` for `evalZigProcess` in `Build.Step`.
  Allows running `evalZigProcess` in a specific directory.
  Needed for setting current directory for compile steps.

`--zig-lib-dir` is now appended differently in `Build.Step.Compile`.
This is needed for compatibility with `setCwd`.
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from 077f287 to ac69db5 Compare February 25, 2025 15:44
a-khabarov added a commit to a-khabarov/zig that referenced this pull request Feb 25, 2025
This commit adds the `soname` option for `Build.Module` and
`OverlayOptions`. This controls whether `-fsoname` or `-fno-soname` is
passed to the linker.

This is needed to implement the new test in
ziglang#19818
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from ac69db5 to 0a646aa Compare February 25, 2025 20:51
a-khabarov added a commit to a-khabarov/zig that referenced this pull request Feb 25, 2025
This commit adds the `soname` option for `Build.Module` and
`OverlayOptions`. This controls whether `-fsoname` or `-fno-soname` is
passed to the linker.

This is needed to implement the new test in
ziglang#19818
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from 0a646aa to b0c5bbb Compare February 25, 2025 20:53
@a-khabarov
Copy link
Author

a-khabarov commented Feb 25, 2025

I've removed everything related to setCwd and addLibraryPathSpecial, it turns out that they are not necessary.

@andrewrk andrewrk removed this from the 0.14.0 milestone Mar 1, 2025
@alexrp alexrp self-requested a review April 15, 2025 19:25
@alexrp alexrp self-assigned this Apr 15, 2025
a-khabarov added a commit to a-khabarov/zig that referenced this pull request Apr 16, 2025
This commit adds the `soname` option for `Build.Module` and
`OverlayOptions`. This controls whether `-fsoname` or `-fno-soname` is
passed to the linker. The `SOName` type is moved to `std.zig` to make
it accessible in other modules.

This is needed to implement the new test in
ziglang#19818
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from b0c5bbb to 7f064ee Compare April 16, 2025 19:15
This commit adds the `soname` option for `Build.Module` and
`OverlayOptions`. This controls whether `-fsoname` or `-fno-soname` is
passed to the linker. The `SoName` type is moved to `std.zig` to make
it accessible in other modules.

This is needed to implement the new test in
ziglang#19818
@a-khabarov a-khabarov force-pushed the fix-make-zig-cc-pass-l-L-like-Clang-GCC-for-ELF branch from 7f064ee to 31be45e Compare April 17, 2025 11:27
@a-khabarov a-khabarov requested a review from alexrp April 17, 2025 11:37
Copy link
Member

@alexrp alexrp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems okay to me. But I'm also not deeply familiar with this stuff, so it's possible that there are subtle details that I'm missing.

@andrewrk do you want to have a look before this gets merged?

@andrewrk andrewrk self-requested a review April 18, 2025 19:48
@andrewrk
Copy link
Member

Yes, thank you.

@alexrp alexrp removed their assignment Apr 19, 2025
@birunts
Copy link

birunts commented May 16, 2025

Hi, any updates on this topic? Is this going to be merged as it's already approved?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zig cc -l example sets the DSO paths incorrectly
5 participants