Skip to content

[clang][driver] Fix -print-libgcc-file-name on Darwin platforms #98325

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 19, 2024
Merged

[clang][driver] Fix -print-libgcc-file-name on Darwin platforms #98325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions clang/include/clang/Driver/Driver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,8 +628,9 @@ class Driver {
/// treated before building actions or binding tools.
///
/// \return Whether any compilation should be built for this
/// invocation.
bool HandleImmediateArgs(const Compilation &C);
/// invocation. The compilation can only be modified when
/// this function returns false.
bool HandleImmediateArgs(Compilation &C);

/// ConstructAction - Construct the appropriate action to do for
/// \p Phase on the \p Input, taking in to account arguments
Expand Down
10 changes: 9 additions & 1 deletion clang/lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2128,7 +2128,7 @@ void Driver::HandleAutocompletions(StringRef PassedFlags) const {
llvm::outs() << llvm::join(SuggestedCompletions, "\n") << '\n';
}

bool Driver::HandleImmediateArgs(const Compilation &C) {
bool Driver::HandleImmediateArgs(Compilation &C) {
// The order these options are handled in gcc is all over the place, but we
// don't expect inconsistencies w.r.t. that to matter in practice.

Expand Down Expand Up @@ -2271,6 +2271,14 @@ bool Driver::HandleImmediateArgs(const Compilation &C) {
if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) {
ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs());
const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs()));
// The 'Darwin' toolchain is initialized only when its arguments are
// computed. Get the default arguments for OFK_None to ensure that
// initialization is performed before trying to access properties of
// the toolchain in the functions below.
// FIXME: Remove when darwin's toolchain is initialized during construction.
// FIXME: For some more esoteric targets the default toolchain is not the
// correct one.
C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_None);
RegisterEffectiveTriple TripleRAII(TC, Triple);
switch (RLT) {
case ToolChain::RLT_CompilerRT:
Expand Down
65 changes: 44 additions & 21 deletions clang/lib/Driver/ToolChains/Darwin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,23 +1272,8 @@ unsigned DarwinClang::GetDefaultDwarfVersion() const {
void MachO::AddLinkRuntimeLib(const ArgList &Args, ArgStringList &CmdArgs,
StringRef Component, RuntimeLinkOptions Opts,
bool IsShared) const {
SmallString<64> DarwinLibName = StringRef("libclang_rt.");
// On Darwin the builtins component is not in the library name.
if (Component != "builtins") {
DarwinLibName += Component;
if (!(Opts & RLO_IsEmbedded))
DarwinLibName += "_";
}

DarwinLibName += getOSLibraryNameSuffix();
DarwinLibName += IsShared ? "_dynamic.dylib" : ".a";
SmallString<128> Dir(getDriver().ResourceDir);
llvm::sys::path::append(Dir, "lib", "darwin");
if (Opts & RLO_IsEmbedded)
llvm::sys::path::append(Dir, "macho_embedded");

SmallString<128> P(Dir);
llvm::sys::path::append(P, DarwinLibName);
std::string P = getCompilerRT(
Args, Component, IsShared ? ToolChain::FT_Shared : ToolChain::FT_Static);

// For now, allow missing resource libraries to support developers who may
// not have compiler-rt checked out or integrated into their build (unless
Expand All @@ -1303,18 +1288,56 @@ void MachO::AddLinkRuntimeLib(const ArgList &Args, ArgStringList &CmdArgs,
// rpaths. This is currently true from this place, but we need to be
// careful if this function is ever called before user's rpaths are emitted.
if (Opts & RLO_AddRPath) {
assert(DarwinLibName.ends_with(".dylib") && "must be a dynamic library");
assert(StringRef(P).ends_with(".dylib") && "must be a dynamic library");

// Add @executable_path to rpath to support having the dylib copied with
// the executable.
CmdArgs.push_back("-rpath");
CmdArgs.push_back("@executable_path");

// Add the path to the resource dir to rpath to support using the dylib
// from the default location without copying.
// Add the compiler-rt library's directory to rpath to support using the
// dylib from the default location without copying.
CmdArgs.push_back("-rpath");
CmdArgs.push_back(Args.MakeArgString(Dir));
CmdArgs.push_back(Args.MakeArgString(llvm::sys::path::parent_path(P)));
}
}

std::string MachO::getCompilerRT(const ArgList &, StringRef Component,
FileType Type) const {
assert(Type != ToolChain::FT_Object &&
"it doesn't make sense to ask for the compiler-rt library name as an "
"object file");
SmallString<64> MachOLibName = StringRef("libclang_rt");
// On MachO, the builtins component is not in the library name
if (Component != "builtins") {
MachOLibName += '.';
MachOLibName += Component;
}
MachOLibName += Type == ToolChain::FT_Shared ? "_dynamic.dylib" : ".a";

SmallString<128> FullPath(getDriver().ResourceDir);
llvm::sys::path::append(FullPath, "lib", "darwin", "macho_embedded",
MachOLibName);
return std::string(FullPath);
}

std::string Darwin::getCompilerRT(const ArgList &, StringRef Component,
FileType Type) const {
assert(Type != ToolChain::FT_Object &&
"it doesn't make sense to ask for the compiler-rt library name as an "
"object file");
SmallString<64> DarwinLibName = StringRef("libclang_rt.");
// On Darwin, the builtins component is not in the library name
if (Component != "builtins") {
DarwinLibName += Component;
DarwinLibName += '_';
}
DarwinLibName += getOSLibraryNameSuffix();
DarwinLibName += Type == ToolChain::FT_Shared ? "_dynamic.dylib" : ".a";

SmallString<128> FullPath(getDriver().ResourceDir);
llvm::sys::path::append(FullPath, "lib", "darwin", DarwinLibName);
return std::string(FullPath);
}

StringRef Darwin::getPlatformFamily() const {
Expand Down
13 changes: 13 additions & 0 deletions clang/lib/Driver/ToolChains/Darwin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,13 @@ class LLVM_LIBRARY_VISIBILITY MachO : public ToolChain {
// There aren't any profiling libs for embedded targets currently.
}

// Return the full path of the compiler-rt library on a non-Darwin MachO
// system. Those are under
// <resourcedir>/lib/darwin/macho_embedded/<...>(.dylib|.a).
std::string
getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
FileType Type = ToolChain::FT_Static) const override;

/// }
/// @name ToolChain Implementation
/// {
Expand Down Expand Up @@ -356,6 +363,12 @@ class LLVM_LIBRARY_VISIBILITY Darwin : public MachO {
void addProfileRTLibs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;

// Return the full path of the compiler-rt library on a Darwin MachO system.
// Those are under <resourcedir>/lib/darwin/<...>(.dylib|.a).
std::string
getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
FileType Type = ToolChain::FT_Static) const override;

protected:
/// }
/// @name Darwin specific Toolchain functions
Expand Down
91 changes: 91 additions & 0 deletions clang/test/Driver/darwin-print-libgcc-file-name.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Test the output of -print-libgcc-file-name on Darwin.
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the embedded targets are already tested to some extent inside clang/test/Driver/darwin-embedded.c. I would add a new test file clang/test/Driver/darwin-embedded-print-libgcc-file-name.c and add something like this in it:

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN:     --target=armv7em-apple-darwin \
// RUN:     -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-HARD_STATIC %s
// CHECK-CLANGRT-HARD_STATIC: libclang_rt.hard_static.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN:     --target=armv7em-apple-darwin -msoft-float \
// RUN:     -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-SOFT_STATIC %s
// CHECK-CLANGRT-SOFT_STATIC: libclang_rt.soft_static.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN:     --target=armv7em-apple-darwin -fPIC \
// RUN:     -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-HARD_PIC %s
// CHECK-CLANGRT-HARD_PIC: libclang_rt.hard_pic.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN:     --target=armv7em-apple-darwin -msoft-float -fPIC \
// RUN:     -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-SOFT_PIC %s
// CHECK-CLANGRT-SOFT_PIC: libclang_rt.soft_pic.a

I'm not certain what we're testing, but we're testing the behavior that seems to exist today, so at least we'll have coverage for whatever we do today.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added these tests and unfortunately they fail (both before and after my change). I believe part of the problem is that -print-libgcc-file-name is still only dealing with the default toolchain we construct first. The actual toolchain object used is constructed much much later, so early initialization is not sufficient to fix this particular problem. I added these tests as expected to fail to document this shortcoming. I still believe we should take this change as it at least fixes the behavior of this flag for the non-embedded scenarios.

The proper fix would be really involved. I shortly experimented with moving the processing of -print-libgcc-file-name to a later stage in the driver, but that is also problematic as clang might start to emit errors about missing input files in that case (because having an input file is not a requirement for this particular flag, but it is a requirement to build the jobs/actions that happen before we construct the actual toolchain that has the correct data we need).

I am not really sure what is the correct solution here, I start to think that the -print-libgcc-file-name flag is just not compatible with the way clang works, and we might need a separate flag that needs input files as well and is processed at a later stage and is more reliable for all targets.

Copy link
Member

Choose a reason for hiding this comment

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

Ack. I think it's reasonable not to try to fix everything in one go since it seems really involved. However, my thinking is more that the Darwin driver is the odd one out here, the driver should be initialized when we construct it, not "lazily" when we compute the arguments. I think it's the only driver which does that, and isn't that the source of all our problems?


//
// All platforms
//

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: --target=x86_64-apple-macos \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-MACOS %s
// CHECK-CLANGRT-MACOS: libclang_rt.osx.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: --target=arm64-apple-ios \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-IOS %s
// CHECK-CLANGRT-IOS: libclang_rt.ios.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: --target=arm64-apple-watchos \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-WATCHOS %s
// CHECK-CLANGRT-WATCHOS: libclang_rt.watchos.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: --target=arm64-apple-tvos \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-TVOS %s
// CHECK-CLANGRT-TVOS: libclang_rt.tvos.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: --target=arm64-apple-driverkit \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-DRIVERKIT %s
// CHECK-CLANGRT-DRIVERKIT: libclang_rt.driverkit.a

//
// Simulators
//

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: --target=arm64-apple-ios-simulator \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-IOS-SIMULATOR %s
// CHECK-CLANGRT-IOS-SIMULATOR: libclang_rt.iossim.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: --target=arm64-apple-watchos-simulator \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-WATCHOS-SIMULATOR %s
// CHECK-CLANGRT-WATCHOS-SIMULATOR: libclang_rt.watchossim.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: --target=arm64-apple-tvos-simulator \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-TVOS-SIMULATOR %s
// CHECK-CLANGRT-TVOS-SIMULATOR: libclang_rt.tvossim.a

// Check the sanitizer and profile variants
// While the driver also links in sanitizer-specific dylibs, the result of
// -print-libgcc-file-name is the path of the basic compiler-rt library.

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: -fsanitize=address --target=x86_64-apple-macos \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-MACOS-SAN %s
// CHECK-CLANGRT-MACOS-SAN: libclang_rt.osx.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: -fsanitize=address --target=arm64-apple-ios \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-IOS-SAN %s
// CHECK-CLANGRT-IOS-SAN: libclang_rt.ios.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: -fsanitize=address --target=arm64-apple-watchos \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-WATCHOS-SAN %s
// CHECK-CLANGRT-WATCHOS-SAN: libclang_rt.watchos.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: -fsanitize=address --target=arm64-apple-tvos \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-TVOS-SAN %s
// CHECK-CLANGRT-TVOS-SAN: libclang_rt.tvos.a

// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \
// RUN: -fsanitize=address --target=arm64-apple-driverkit \
// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-DRIVERKIT-SAN %s
// CHECK-CLANGRT-DRIVERKIT-SAN: libclang_rt.driverkit.a
Loading