Skip to content

Commit

Permalink
[Darwin][Driver][clang] Prioritise -isysroot over --sysroot consi…
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
carlocab and Bo98 committed Nov 13, 2024
1 parent de0fd64 commit d35918e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 9 deletions.
13 changes: 6 additions & 7 deletions clang/lib/Driver/ToolChains/Darwin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -428,15 +428,14 @@ void darwin::Linker::AddLinkArgs(Compilation &C, const ArgList &Args,
Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);

// Give --sysroot= preference, over the Apple specific behavior to also use
// --isysroot as the syslibroot.
StringRef sysroot = C.getSysRoot();
if (sysroot != "") {
CmdArgs.push_back("-syslibroot");
CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
} else if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) {
// Prioritise -isysroot to ensure that the libraries we link to are taken
// from the same SDK that our headers come from.
if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) {
CmdArgs.push_back("-syslibroot");
CmdArgs.push_back(A->getValue());
} else if (StringRef sysroot = C.getSysRoot(); sysroot != "") {
CmdArgs.push_back("-syslibroot");
CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
}

Args.AddLastArg(CmdArgs, options::OPT_twolevel__namespace);
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Driver/sysroot.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
// RUN: FileCheck --check-prefix=CHECK-APPLE-ISYSROOT < %t2 %s
// CHECK-APPLE-ISYSROOT: "-arch" "i386"{{.*}} "-syslibroot" "{{[^"]*}}/FOO"

// Check that honor --sysroot= over -isysroot, for Apple Darwin.
// Check that honor -isysroot over --sysroot=, for Apple Darwin.
// RUN: touch %t3.o
// RUN: %clang -target i386-apple-darwin10 \
// RUN: -isysroot /FOO --sysroot=/BAR -### %t3.o 2> %t3
// RUN: FileCheck --check-prefix=CHECK-APPLE-SYSROOT < %t3 %s
// CHECK-APPLE-SYSROOT: "-arch" "i386"{{.*}} "-syslibroot" "{{[^"]*}}/BAR"
// CHECK-APPLE-SYSROOT: "-arch" "i386"{{.*}} "-syslibroot" "{{[^"]*}}/FOO"

0 comments on commit d35918e

Please sign in to comment.