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

llvm: Clang doesn't use SDKROOT when linking #197277

Open
4 tasks done
madsmtm opened this issue Nov 10, 2024 · 22 comments
Open
4 tasks done

llvm: Clang doesn't use SDKROOT when linking #197277

madsmtm opened this issue Nov 10, 2024 · 22 comments
Labels
stale No recent activity upstream issue An upstream issue report is needed

Comments

@madsmtm
Copy link

madsmtm commented Nov 10, 2024

brew gist-logs <formula> link OR brew config AND brew doctor output

% brew gist-logs llvm@18
Error: No logs.

% brew gist-logs llvm@19
Error: No logs.

% brew config
HOMEBREW_VERSION: 4.4.4
ORIGIN: https://github.com/Homebrew/brew
HEAD: 824efa8836dc226aa92dbbf7404c1b4a66707cca
Last commit: 7 days ago
Core tap JSON: 10 Nov 22:51 UTC
Core cask tap JSON: 10 Nov 22:51 UTC
HOMEBREW_PREFIX: /opt/homebrew
HOMEBREW_CASK_OPTS: []
HOMEBREW_MAKE_JOBS: 8
Homebrew Ruby: 3.3.5 => /opt/homebrew/Library/Homebrew/vendor/portable-ruby/3.3.5/bin/ruby
CPU: octa-core 64-bit dunno
Clang: 16.0.0 build 1600
Git: 2.39.5 => /Library/Developer/CommandLineTools/usr/bin/git
Curl: 8.7.1 => /usr/bin/curl
macOS: 14.6.1-arm64
CLT: 16.1.0.0.1.1729049160
Xcode: N/A
Rosetta 2: false

% brew doctor
Your system is ready to brew.

Note that the system is a clean virtual machine with a newly installed homebrew. The problem has also been reproduced here.

Verification

  • My brew doctor output says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update and am still able to reproduce my issue.
  • I have resolved all warnings from brew doctor and that did not fix my problem.
  • I searched for recent similar issues at https://github.com/Homebrew/homebrew-core/issues?q=is%3Aissue and found no duplicates.

What were you trying to do (and why)?

It is generally expected that setting the SDKROOT environment variable (or the -isysroot flag) when compiling with Clang on macOS selects a different SDK. xcrun, for example, uses this to make it easy to select a different SDK when compiling.

This is somewhat supported by the Clang provided by the llvm package, but the SDK that llvm was built with is always selected when linking instead of the value set by xcrun / in SDKROOT / with -isysroot.

This is likely to affect cross compilation (to iOS/tvOS/watchOS/visionOS). I'm investigating this because of rust-lang/rust#131477 and rust-lang/cc-rs#1278, since it is unclear how downstream users such as rustc should pass a different SDK root.

The following contrived example program fails to link with the macOS 15.1 SDK for this reason:

#import <os/lock.h>

int main(int argc, char *argv[]) {
    if (@available(macos 15.0, *)) {
        // Use some API that's only available on macOS 15.0 or newer
        os_unfair_lock lock = OS_UNFAIR_LOCK_INIT;
        os_unfair_lock_lock_with_flags(&lock, OS_UNFAIR_LOCK_FLAG_ADAPTIVE_SPIN);
    }
    return 0;
}

What happened (include all command output)?

Linking fails with:

Undefined symbols for architecture arm64:
 "_os_unfair_lock_lock_with_flags", referenced from:
      _main in foo-21aa0e.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

What did you expect to happen?

The macOS 15.1 SDK is used, also when linking, and compilation and linking succeeds.
OR
An error at compile-time because the macOS 14.0 SDK is consistently used.

Step-by-step reproduction instructions (by running brew commands)

  1. Install llvm package (any version):

    brew install llvm@19

  2. Install Xcode 15.4 and Xcode 16.1 (or equivalent Command Line Tools), so that the macOS 14.0 and macOS 15.1 SDKs are available.

  3. Put the example program above in foo.m.

  4. Try to compile with the macOS 14.0 SDK (this fails as expected, because the function isn't available in the old SDK):

    xcrun -sdk macosx14.0 clang foo.m
    xcrun -sdk macosx14.0 /opt/homebrew/opt/llvm@19/bin/clang foo.m
  5. Compile with Xcode Clang and macOS 15.1 SDK (this works):

    xcrun -sdk macosx15.1 clang foo.m
  6. Compile with Clang provided by llvm package and macOS 15.1 SDK (this fails linking):

    xcrun -sdk macosx15.1 /opt/homebrew/opt/llvm@19/bin/clang foo.m

Use -v to see that Clang always passes the same value to ld's -syslibroot.

@madsmtm madsmtm added the bug Reproducible Homebrew/homebrew-core bug label Nov 10, 2024
@madsmtm
Copy link
Author

madsmtm commented Nov 10, 2024

See also discussion in #196094 (comment)

@carlocab
Copy link
Member

Works for me:

❯ xcrun -sdk macosx15.1 /opt/homebrew/opt/llvm@19/bin/clang foo.m; echo $?
0

But I'm guessing the important bit that's left out of your reproduction steps is that you must be testing this on macOS 14 (which I am not).

@carlocab
Copy link
Member

I think this is an upstream bug. You can reproduce this with Apple Clang as well:

❯ xcrun -sdk macosx15.1 clang --config=/opt/homebrew/etc/clang/arm64-apple-darwin23.cfg foo.m
Undefined symbols for architecture arm64:
  "_os_unfair_lock_lock_with_flags", referenced from:
      _main in foo-28462b.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@carlocab carlocab added upstream issue An upstream issue report is needed and removed bug Reproducible Homebrew/homebrew-core bug labels Nov 11, 2024
@madsmtm
Copy link
Author

madsmtm commented Nov 11, 2024

But I'm guessing the important bit that's left out of your reproduction steps is that you must be testing this on macOS 14 (which I am not).

Indeed, this was on a macOS 14.0 VM.

The problem can also be encountered if you try to cross-compile (assuming you have Xcode installed):

brew install llvm@18 # Different problem on llvm@19 because of #197278
echo """
#import <UIKit/UIKit.h>
int main(int argc, char *argv[]) {
    return UIApplicationMain(argc, argv, nil, nil);
}
""" > bar.m
xcrun -sdk iphoneos /opt/homebrew/opt/llvm@18/bin/clang -target arm64-apple-ios -framework UIKit bar.m

@madsmtm
Copy link
Author

madsmtm commented Nov 11, 2024

I think this is an upstream bug

Dunno, I guess it depends on what the desired behaviour of setting both --sysroot and -isysroot is?

But it could be somewhat fixed by Homebrew by setting -isysroot instead of --sysroot. Or by creating trampoline binaries similar to those under /usr/bin/* that set SDKROOT if not set.

@carlocab
Copy link
Member

The problem can also be encountered if you try to cross-compile (assuming you have Xcode installed):

brew install llvm@18 # Different problem on llvm@19 because of #197278
echo """
#import <UIKit/UIKit.h>
int main(int argc, char *argv[]) {
    return UIApplicationMain(argc, argv, nil, nil);
}
""" > bar.m
xcrun -sdk iphoneos /opt/homebrew/opt/llvm@18/bin/clang -target arm64-apple-ios -framework UIKit bar.m

This makes me think that the issue is not related to #196094, then, because we set DEFAULT_SYSROOT instead of using config files on llvm@18:

args << "-DDEFAULT_SYSROOT=#{macos_sdk}" if macos_sdk

@carlocab
Copy link
Member

But it could be somewhat fixed by Homebrew by setting -isysroot instead of --sysroot.

Not convinced -- setting -isysroot is exactly what will make clang ignore SDKROOT:

https://github.com/llvm/llvm-project/blob/3006dddfe091bcb95924d72dddbb84f73186a344/clang/lib/Driver/ToolChains/Darwin.cpp#L2256-L2270

Does it work for you if you replace --sysroot= with -isysroot in the .cfg files?

Or by creating trampoline binaries similar to those under /usr/bin/* that set SDKROOT if not set.

Yea, we could maybe just make clang a wrapper that does xcrun clang "$@". Not sure if it's the right behaviour if you actually want to target a non-Apple host, though. (But this was arguably already incorrect when we set DEFAULT_SYSROOT.)

@madsmtm
Copy link
Author

madsmtm commented Nov 11, 2024

This makes me think that the issue is not related to #196094, then, because we set DEFAULT_SYSROOT instead of using config files on llvm@18:

Indeed, this is not related to #196094, I only linked it for the context.

But it could be somewhat fixed by Homebrew by setting -isysroot instead of --sysroot.

Not convinced -- setting -isysroot is exactly what will make clang ignore SDKROOT

Agree that it's not a good solution, since it would completely ignore xcrun. But at least it would make it consistent, instead of now where we end up pulling in different headers when compiling vs. the libraries used when linking.

Or by creating trampoline binaries similar to those under /usr/bin/* that set SDKROOT if not set.

Yea, we could maybe just make clang a wrapper that does xcrun clang "$@". Not sure if it's the right behaviour if you actually want to target a non-Apple host, though. (But this was arguably already incorrect when we set DEFAULT_SYSROOT.)

Agreed that I'm not sure what the right behaviour is either 🤷.

@carlocab
Copy link
Member

Indeed, this is not related to #196094, I only linked it for the context.

Right -- in that case, this is definitely an upstream bug.

Can you reproduce this with llvm if you wipe out all the config files in $(brew --prefix)/etc/clang? (I don't have a macOS 14 VM on hand to test this properly with.)

@madsmtm
Copy link
Author

madsmtm commented Nov 11, 2024

I removed all the config files in $(brew --prefix)/etc/clang, and xcrun -sdk macosx15.1 /opt/homebrew/opt/llvm@19/bin/clang foo.m now compiles and links.

If you're debugging this, then the behaviour here can be reproduced with Apple Clang with:

clang --sysroot $(xcrun -sdk macosx14.5 --show-sdk-path) -isysroot $(xcrun -sdk macosx15.1 --show-sdk-path) foo.m
# Or
xcrun -sdk macosx15.1 clang --sysroot $(xcrun -sdk macosx14.5 --show-sdk-path) foo.m
# Or
SDKROOT=$(xcrun -sdk macosx15.1 --show-sdk-path) clang --sysroot $(xcrun -sdk macosx14.5 --show-sdk-path) foo.m

I.e. specify an old SDK for --sysroot and a newer SDK for -isysroot.

@carlocab
Copy link
Member

carlocab commented Nov 13, 2024

I think one workaround here is to pass --no-default-config when invoking clang. This only works -- if it does -- now that we use config files. It most likely would not work if we set DEFAULT_SYSROOT.

You might also be able to do

mkdir -p ~/.config/clang
touch ~/.config/clang/$(clang --print-target-triple).cfg

to make

xcrun -sdk macosx15.1 /opt/homebrew/opt/llvm@19/bin/clang foo.m

just work.

@carlocab
Copy link
Member

Or, I guess the most obvious workaround: pass --sysroot=$(xcrun -sdk macosx15.1 --show-sdk-path) when invoking clang directly. This should override anything we've put in our configuration files.

@madsmtm
Copy link
Author

madsmtm commented Nov 13, 2024

I am indeed considering whether rustc needs to pass both -isysroot and --sysroot, but that has other problems (we don't really want to do compiler detection, and I suspect not all compilers support both of these flags, and I'm undecided if rustc wants to override what Homebrew is doing).

@carlocab
Copy link
Member

carlocab commented Nov 13, 2024

You could try passing --target? I think after #197410 doing

/opt/homebrew/opt/llvm@19/bin/clang --target=arm64-apple-macosx15.1 foo.m

should just work for you, even on macOS 14. Though maybe you don't want to target macOS 15.1, and instead want to target something older but use the macosx15.1 SDK.

@Bo98
Copy link
Member

Bo98 commented Nov 13, 2024

The behaviour comes from llvm/llvm-project@8438464. Looking at the code that was from a time --sysroot did override -isysroot for header includes too.

This is no longer the case so I believe the code is now wrong and should probably be inverted. I think it makes sense to propose this upstream and apply a patch rather than try do weird workarounds here, particularly given the workarounds wouldn't even be consistent between llvm@18 and llvm@19 anyway.

Hard to pinpoint when the header includes logic changed but I suspect it's been a while, so this issue probably affects every LLVM version we ship.

@madsmtm
Copy link
Author

madsmtm commented Nov 13, 2024

Can confirm that this issue is present in llvm@12 too.

Do you want to work on a fix upstream? If not, then I can try to, but I get the feeling you're more experienced with that.

@carlocab
Copy link
Member

I'll send the patch -- @Bo98 already has two outstanding PRs waiting for a review upstream.

@carlocab
Copy link
Member

llvm/llvm-project#115993

carlocab added a commit to carlocab/llvm-project that referenced this issue Nov 13, 2024
…stently

On Darwin, `clang` currently prioritises `-isysroot` over `--sysroot`
when selecting headers, but does the reverse when setting `-syslibroot`
for the linker, which determines library selection.

This is wrong, because it leads to using headers that are of different
versions from the libraries being linked.

We can see this from:

    ❯ bin/clang -v -xc - -o /dev/null \
        -isysroot /Applications/Xcode-14.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.sdk \
        --sysroot=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.sdk <<<'int main(){}'
    clang version 20.0.0git (https://github.com/llvm/llvm-project.git 3de5dbb)
    Target: arm64-apple-darwin24.1.0
    [snip]
    #include "..." search starts here:
    #include <...> search starts here:
     /Users/carlocab/github/llvm-project/build-clang-config/lib/clang/20/include
     /Applications/Xcode-14.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.sdk/usr/include
     /Applications/Xcode-14.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.sdk/System/Library/Frameworks (framework directory)
    End of search list.
     "/usr/bin/ld" -demangle -lto_library /Users/carlocab/github/llvm-project/build-clang-config/lib/libLTO.dylib -no_deduplicate -dynamic -arch arm64 -platform_version macos 13.3.0 13.3 -syslibroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.sdk -mllvm -enable-linkonceodr-outlining -o /dev/null /var/folders/nj/9vfk4f0n377605jhxxmngd8h0000gn/T/--b914c6.o -lSystem

We can fix this by reversing the order in which the `-syslibroot` flag
is determined.

Downstream bug report: Homebrew/homebrew-core#197277

Co-authored-by: Bo Anderson <[email protected]>
Copy link
Contributor

github-actions bot commented Dec 4, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Dec 4, 2024
@madsmtm
Copy link
Author

madsmtm commented Dec 4, 2024

Not stale, the LLVM patch is being discussed

@github-actions github-actions bot removed the stale No recent activity label Dec 4, 2024
carlocab added a commit that referenced this issue Dec 4, 2024
Let's create config files that target `aarch64` too. This is used as the
target by Python. See Homebrew/discussions#5778.

Also, use `-isysroot` instead of `--sysroot` so that our choice can be
overridden by setting `SDKROOT`. Fixes #197277.
@carlocab
Copy link
Member

carlocab commented Dec 4, 2024

Upstream's not very receptive to the change -- maybe #200047 will work instead.

carlocab added a commit that referenced this issue Dec 4, 2024
Also:

Let's create config files that target `aarch64` too. This is used as the
target by Python. See Homebrew/discussions#5778.

Also, use `-isysroot` instead of `--sysroot` so that our choice can be
overridden by setting `SDKROOT`. Fixes #197277.

Co-authored-by: Carlo Cabrera <[email protected]>
carlocab added a commit to carlocab/llvm-project that referenced this issue Dec 5, 2024
…YSROOT`

If a toolchain is configured with `DEFAULT_SYSROOT`, then this could
result in an unintended value for `-syslibroot` being passed to the
linker if the user manually sets `-isysroot` or `SDKROOT`.

Let's fix this by prioritising command line flags when determining
`-syslibroot` before checking `getSysRoot`.

Downstream bug report: Homebrew/homebrew-core#197277

Co-authored-by: Bo Anderson <[email protected]>
carlocab added a commit to carlocab/llvm-project that referenced this issue Dec 9, 2024
…YSROOT`

If a toolchain is configured with `DEFAULT_SYSROOT`, then this could
result in an unintended value for `-syslibroot` being passed to the
linker if the user manually sets `-isysroot` or `SDKROOT`.

Let's fix this by prioritising command line flags when determining
`-syslibroot` before checking `getSysRoot`.

Downstream bug report: Homebrew/homebrew-core#197277

Co-authored-by: Bo Anderson <[email protected]>
carlocab added a commit to llvm/llvm-project that referenced this issue Dec 12, 2024
…SROOT` (#115993)

If a toolchain is configured with `DEFAULT_SYSROOT`, then this could
result in an unintended value for `-syslibroot` being passed to the
linker if the user manually sets `-isysroot` or `SDKROOT`.

Let's fix this by prioritising command line flags when determining
`-syslibroot` before checking `getSysRoot`.

Downstream bug report:
Homebrew/homebrew-core#197277

Co-authored-by: Bo Anderson <[email protected]>

Co-authored-by: Bo Anderson <[email protected]>
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale No recent activity upstream issue An upstream issue report is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants