Skip to content
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

stage2: add --sysroot link option #9202

Merged
merged 2 commits into from
Jun 25, 2021
Merged

stage2: add --sysroot link option #9202

merged 2 commits into from
Jun 25, 2021

Conversation

ifreund
Copy link
Member

@ifreund ifreund commented Jun 22, 2021

This feature is necessary for cross-compiling code that dynamically
links system libraries, at least with the current feature set of lld.

Example usage to integrate with void linux's xbps-src cross compilation and packaging tool: void-linux/void-packages#29288

Comment on lines -1643 to +1652
if (cross_target.isNativeOs() and (system_libs.items.len != 0 or want_native_include_dirs)) {
if (sysroot == null and cross_target.isNativeOs() and
(system_libs.items.len != 0 or want_native_include_dirs))
{
Copy link
Member Author

Choose a reason for hiding this comment

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

@kubkon am I correct that skipping this logic makes sense if an explicit sysroot is provided?

Also, I've just realized that I should add support for this new flag in src/link/MachO.zig as well, or at least a error instead of silent failure. Does zig ld support this feature yet?

Copy link
Member

Choose a reason for hiding this comment

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

You should and zig ld supports syslibroot.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I believe you're correct here. If the user explicitly specifies the sysroot, we should just take it rather than infer it automatically.

Copy link
Member

Choose a reason for hiding this comment

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

We do have a concept of syslibroot in the linking stage as a flag, but with clang this usually is passed using -Wl,-syslibroot, and I'm not sure how sysroot should be remapped to it. Anyway, we use it with zig ld like so:

if (self.base.options.syslibroot) |syslibroot| {

Copy link
Member Author

Choose a reason for hiding this comment

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

Are the semantics of syslibroot the same as that of --sysroot for lld? lld and gnu ld understand --sysroot to be the root directory in which to look for both shared object and include files.

There's also -isysroot which only sets the root directory in which to look for include files, overriding --sysroot for them. I don't have a use-case for exposing a Zig flag to do that so I've left it off for now.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's also true for Apple's ld64:

❯ clang --sysroot $(xcrun --show-sdk-path) hello.cpp -lc++ -v
Apple clang version 12.0.5 (clang-1205.0.22.9)
Target: arm64-apple-darwin20.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
 "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang" -cc1 -triple arm64-apple-macosx11.0.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -Werror=implicit-function-declaration -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name hello.cpp -mrelocation-model pic -pic-level 2 -mframe-pointer=non-leaf -fno-strict-return -fno-rounding-math -munwind-tables -target-sdk-version=11.3 -fvisibility-inlines-hidden-static-local-var -target-cpu apple-a12 -target-feature +v8.3a -target-feature +fp-armv8 -target-feature +neon -target-feature +crc -target-feature +crypto -target-feature +fullfp16 -target-feature +ras -target-feature +lse -target-feature +rdm -target-feature +rcpc -target-feature +zcm -target-feature +zcz -target-feature +sha2 -target-feature +aes -target-abi darwinpcs -fallow-half-arguments-and-returns -debugger-tuning=lldb -target-linker-version 650.9 -v -resource-dir /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.5 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -stdlib=libc++ -internal-isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1 -internal-isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/local/include -internal-isystem /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.5/include -internal-externc-isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include -internal-externc-isystem /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include -Wno-reorder-init-list -Wno-implicit-int-float-conversion -Wno-c99-designator -Wno-final-dtor-non-final-class -Wno-extra-semi-stmt -Wno-misleading-indentation -Wno-quoted-include-in-framework-header -Wno-implicit-fallthrough -Wno-enum-enum-conversion -Wno-enum-float-conversion -Wno-elaborated-enum-base -fdeprecated-macro -fdebug-compilation-dir /Users/kubkon/dev/zig/examples -ferror-limit 19 -stack-protector 1 -fstack-check -mdarwin-stkchk-strong-link -fblocks -fencode-extended-block-signature -fregister-global-dtors-with-atexit -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fmax-type-align=16 -fcommon -fcolor-diagnostics -clang-vendor-feature=+disableNonDependentMemberExprInCurrentInstantiation -fno-odr-hash-protocols -mllvm -disable-aligned-alloc-awareness=1 -o /var/folders/ym/hxsqmxds3mqbxxrk4l3jl31h0000gn/T/hello-5c1855.o -x c++ hello.cpp
clang -cc1 version 12.0.5 (clang-1205.0.22.9) default target arm64-apple-darwin20.6.0
ignoring nonexistent directory "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/local/include"
ignoring nonexistent directory "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/Library/Frameworks"
#include "..." search starts here:
#include <...> search starts here:
 /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1
 /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.5/include
 /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include
 /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include
 /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks (framework directory)
End of search list.
 "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld" -demangle -lto_library /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/libLTO.dylib -no_deduplicate -dynamic -arch arm64 -platform_version macos 11.0.0 11.3 -syslibroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -o a.out /var/folders/ym/hxsqmxds3mqbxxrk4l3jl31h0000gn/T/hello-5c1855.o -lc++ -lSystem /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.5/lib/darwin/libclang_rt.osx.a

As you can see, -syslibroot was set to the sysroot value which is exactly what we want here as well.

@ifreund ifreund marked this pull request as draft June 22, 2021 23:53
@kubkon
Copy link
Member

kubkon commented Jun 25, 2021

@ifreund do you need any assistance with this PR? Turns out we need this to support linking frameworks when cross-compiling to macOS.

@ifreund
Copy link
Member Author

ifreund commented Jun 25, 2021

@ifreund do you need any assistance with this PR? Turns out we need this to support linking frameworks when cross-compiling to macOS.

@kubkon I just got busy with other things, though I would certainly appreciate another round of review wrt the darwin side of things. I opted to replace "syslibroot" with "sysroot" in non darwin-specific code as this matches the zig command line option and matches the term I expect more users to be familiar with from using C compilers.

Glad that this will solve problems for cross compiling to macOS as well!

@ifreund ifreund marked this pull request as ready for review June 25, 2021 12:08
const sysroot = blk: {
if (options.sysroot) |sysroot| {
break :blk sysroot;
} else if (build_options.have_llvm and comptime std.Target.current.isDarwin() and
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to either split the check into two so that build_options.have_llvm and comptime std.Target.current.isDarwin() is evaluated in compile-time, or remove the comptime altogether. use_lld and options.target.isDarwin() are only runtime-known.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see. On looking at this again I realize that as an unintended side-effect of my changes the system linker hack will not work if --sysroot is passed. I'll look for a way to fix both of these things without duplicating logic.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, given that we are close to removing system linker hack altogether, I'm fine with not having it on with explicit --sysroot. Thoughts? @andrewrk care to weigh in?

Copy link
Member

Choose a reason for hiding this comment

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

There is actually one major feature that's still missing, creation of dylibs, but apart from that, zig ld is more than capable.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've force-pushed again and revised this code to make the system linker hack compatible with --sysroot and hopefully fix the compile error.

This feature is necessary for cross-compiling code that dynamically
links system libraries, at least with the current feature set of lld.
@kubkon kubkon merged commit 7a85dc6 into ziglang:master Jun 25, 2021
@ifreund ifreund deleted the sysroot branch June 26, 2021 11:59
@kubkon kubkon mentioned this pull request Jun 29, 2021
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.

2 participants