Hi, thanks for the review, I was not even aware that this could be tested. Adding a test helped to fix me a couple extra issues (plus the one you already mentioned). New patch attached.
Xan On Wed, Aug 05, 2015 at 09:14:30AM -0400, Rafael Espíndola wrote: > Please git-clang-format this patch. > > + // /usr/gcc/<major>.<minor>/lib/gcc/<major>.<minor>.<patch>/, > > The code appends a triple after the "/lib/gcc". Is the comment missing it? > > The inner loop has no version comparison. Are you depending on the > directory iteration order? > > Can you add a testcase? > > > On 28 July 2015 at 12:35, Xan López <x...@igalia.com> wrote: > > Here it is. > > > > On Tue, Jul 28, 2015 at 01:21:06PM +0200, Xan López wrote: > >> On Tue, Jul 28, 2015 at 01:55:23PM +0300, Yaron Keren wrote: > >> > +cfe-commits > >> > > >> > This is a very large Solaris special case in ScanLibDirForGCCTriple which > >> > shares almost no code with the function. > >> > How about splitting it out to a helper function or > >> > making ScanLibDirForGCCTriple virtual and overriding on Solaris? > >> > >> Yep, at least a helper function makes sense, you are right. I'll send > >> another patch with either of those suggestions later today. > >> > >> > >> Xan > >> _______________________________________________ > >> llvm-commits mailing list > >> llvm-comm...@cs.uiuc.edu > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > > > > _______________________________________________ > > llvm-commits mailing list > > llvm-comm...@cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > >
>From 27215294c9a4aeaf25b2d618839bf413c137f8e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xan=20L=C3=B3pez?= <x...@igalia.com> Date: Fri, 24 Jul 2015 19:17:51 +0200 Subject: [PATCH] [Solaris] Stop hardcoding the location of the c runtime files This refactors the GCC installation detection to work properly on Solaris (which is a bit of a special case compared to Linux distributions), and adds the proper /usr/lib/... logic in the Driver. That way we can just use ToolChain::GetFilePath to locate all crt*.o files instead of hardcoding their location. --- lib/Driver/ToolChains.cpp | 107 +++++++++++++++++++-- lib/Driver/ToolChains.h | 6 ++ lib/Driver/Tools.cpp | 54 ++++------- .../4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crt1.o | 0 .../lib/gcc/sparc-sun-solaris2.11/4.8.2/crtbegin.o | 0 .../lib/gcc/sparc-sun-solaris2.11/4.8.2/crtend.o | 0 .../Inputs/sparc-sun-solaris2.11/usr/lib/crti.o | 0 .../Inputs/sparc-sun-solaris2.11/usr/lib/crtn.o | 0 .../Inputs/sparc-sun-solaris2.11/usr/lib/ld.so.1 | 0 .../sparc-sun-solaris2.11/usr/lib/values-Xa.o | 0 test/Driver/solaris-ld.c | 16 +++ 11 files changed, 140 insertions(+), 43 deletions(-) create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crt1.o create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtbegin.o create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtend.o create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/crti.o create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/crtn.o create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/ld.so.1 create mode 100644 test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/values-Xa.o create mode 100644 test/Driver/solaris-ld.c diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp index 59e6a2e..8e49c5a 100644 --- a/lib/Driver/ToolChains.cpp +++ b/lib/Driver/ToolChains.cpp @@ -1340,9 +1340,21 @@ bool Generic_GCC::GCCInstallationDetector::getBiarchSibling(Multilib &M) const { "s390x-linux-gnu", "s390x-unknown-linux-gnu", "s390x-ibm-linux-gnu", "s390x-suse-linux", "s390x-redhat-linux"}; + // Solaris. + static const char *const SolarisSPARCLibDirs[] = {"/gcc"}; + static const char *const SolarisSPARCTriples[] = {"sparc-sun-solaris2.11", + "i386-pc-solaris2.11"}; + using std::begin; using std::end; + if (TargetTriple.getOS() == llvm::Triple::Solaris) { + LibDirs.append(begin(SolarisSPARCLibDirs), end(SolarisSPARCLibDirs)); + TripleAliases.append(begin(SolarisSPARCTriples), end(SolarisSPARCTriples)); + + return; + } + switch (TargetTriple.getArch()) { case llvm::Triple::aarch64: LibDirs.append(begin(AArch64LibDirs), end(AArch64LibDirs)); @@ -1907,6 +1919,54 @@ static bool findBiarchMultilibs(const llvm::Triple &TargetTriple, return true; } +void Generic_GCC::GCCInstallationDetector::SolarisScanLibDirForGCCTriple( + const llvm::Triple &TargetArch, const llvm::opt::ArgList &Args, + const std::string &LibDir, StringRef CandidateTriple, + bool NeedsBiarchSuffix) { + // Solaris is a special case. The GCC installation is under + // /usr/gcc/<major>.<minor>/lib/gcc/<triple>/<major>.<minor>.<patch>/, so we + // need to iterate twice. + std::error_code EC; + for (llvm::sys::fs::directory_iterator LI(LibDir, EC), LE; !EC && LI != LE; + LI = LI.increment(EC)) { + StringRef VersionText = llvm::sys::path::filename(LI->path()); + GCCVersion CandidateVersion = GCCVersion::Parse(VersionText); + + if (CandidateVersion.Major != -1) // Filter obviously bad entries. + if (!CandidateGCCInstallPaths.insert(LI->path()).second) + continue; // Saw this path before; no need to look at it again. + if (CandidateVersion.isOlderThan(4, 1, 1)) + continue; + if (CandidateVersion <= Version) + continue; + + GCCInstallPath = + LibDir + "/" + VersionText.str() + "/lib/gcc/" + CandidateTriple.str(); + if (!llvm::sys::fs::exists(GCCInstallPath)) + continue; + + // If we make it here there has to be at least one GCC version, let's just + // use the latest one. + std::error_code EEC; + for (llvm::sys::fs::directory_iterator LLI(GCCInstallPath, EEC), LLE; + !EEC && LLI != LLE; LLI = LLI.increment(EEC)) { + + StringRef SubVersionText = llvm::sys::path::filename(LLI->path()); + GCCVersion CandidateSubVersion = GCCVersion::Parse(SubVersionText); + + if (CandidateSubVersion > Version) + Version = CandidateSubVersion; + } + + GCCTriple.setTriple(CandidateTriple); + + GCCInstallPath += "/" + Version.Text; + GCCParentLibPath = GCCInstallPath + "/../../../../"; + + IsValid = true; + } +} + void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple( const llvm::Triple &TargetTriple, const ArgList &Args, const std::string &LibDir, StringRef CandidateTriple, @@ -1937,6 +1997,13 @@ void Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple( "/../..", // <triple>/ "/../../../.." // i386-linux-gnu/gcc/<triple>/ }; + + if (TargetTriple.getOS() == llvm::Triple::Solaris) { + SolarisScanLibDirForGCCTriple(TargetTriple, Args, LibDir, CandidateTriple, + NeedsBiarchSuffix); + return; + } + // Only look at the final, weird Ubuntu suffix for i386-linux-gnu. const unsigned NumLibSuffixes = (llvm::array_lengthof(LibSuffixes) - (TargetArch != llvm::Triple::x86)); @@ -2840,18 +2907,45 @@ Tool *Minix::buildAssembler() const { Tool *Minix::buildLinker() const { return new tools::minix::Linker(*this); } +static void addPathIfExists(Twine Path, ToolChain::path_list &Paths) { + if (llvm::sys::fs::exists(Path)) + Paths.push_back(Path.str()); +} + /// Solaris - Solaris tool chain which can call as(1) and ld(1) directly. Solaris::Solaris(const Driver &D, const llvm::Triple &Triple, const ArgList &Args) : Generic_GCC(D, Triple, Args) { - getProgramPaths().push_back(getDriver().getInstalledDir()); + GCCInstallation.init(D, Triple, Args); + + path_list &Paths = getFilePaths(); + if (GCCInstallation.isValid()) + addPathIfExists(GCCInstallation.getInstallPath(), Paths); + + addPathIfExists(getDriver().getInstalledDir(), Paths); if (getDriver().getInstalledDir() != getDriver().Dir) - getProgramPaths().push_back(getDriver().Dir); + addPathIfExists(getDriver().Dir, Paths); - getFilePaths().push_back(getDriver().Dir + "/../lib"); - getFilePaths().push_back("/usr/lib"); + addPathIfExists(getDriver().SysRoot + getDriver().Dir + "/../lib", Paths); + + std::string LibPath = "/usr/lib/"; + switch (Triple.getArch()) { + case llvm::Triple::x86: + case llvm::Triple::sparc: + break; + case llvm::Triple::x86_64: + LibPath += "amd64/"; + break; + case llvm::Triple::sparcv9: + LibPath += "sparcv9/"; + break; + default: + llvm_unreachable("Unsupported architecture"); + } + + addPathIfExists(getDriver().SysRoot + LibPath, Paths); } Tool *Solaris::buildAssembler() const { @@ -3111,11 +3205,6 @@ static std::string getMultiarchTriple(const llvm::Triple &TargetTriple, return TargetTriple.str(); } -static void addPathIfExists(Twine Path, ToolChain::path_list &Paths) { - if (llvm::sys::fs::exists(Path)) - Paths.push_back(Path.str()); -} - static StringRef getOSLibDir(const llvm::Triple &Triple, const ArgList &Args) { if (isMipsArch(Triple.getArch())) { // lib32 directory has a special meaning on MIPS targets. diff --git a/lib/Driver/ToolChains.h b/lib/Driver/ToolChains.h index cc2a5bb..91d7651 100644 --- a/lib/Driver/ToolChains.h +++ b/lib/Driver/ToolChains.h @@ -145,6 +145,12 @@ public: const std::string &LibDir, StringRef CandidateTriple, bool NeedsBiarchSuffix = false); + + void SolarisScanLibDirForGCCTriple(const llvm::Triple &TargetArch, + const llvm::opt::ArgList &Args, + const std::string &LibDir, + StringRef CandidateTriple, + bool NeedsBiarchSuffix = false); }; protected: diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp index a04be9c..d1409ec 100644 --- a/lib/Driver/Tools.cpp +++ b/lib/Driver/Tools.cpp @@ -6770,25 +6770,6 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA, const InputInfoList &Inputs, const ArgList &Args, const char *LinkingOutput) const { - // FIXME: Find a real GCC, don't hard-code versions here - std::string GCCLibPath = "/usr/gcc/4.5/lib/gcc/"; - const llvm::Triple &T = getToolChain().getTriple(); - std::string LibPath = "/usr/lib/"; - const llvm::Triple::ArchType Arch = T.getArch(); - switch (Arch) { - case llvm::Triple::x86: - GCCLibPath += - ("i386-" + T.getVendorName() + "-" + T.getOSName()).str() + "/4.5.2/"; - break; - case llvm::Triple::x86_64: - GCCLibPath += ("i386-" + T.getVendorName() + "-" + T.getOSName()).str(); - GCCLibPath += "/4.5.2/amd64/"; - LibPath += "amd64/"; - break; - default: - llvm_unreachable("Unsupported architecture"); - } - ArgStringList CmdArgs; // Demangle C++ names in errors @@ -6809,7 +6790,8 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back("-shared"); } else { CmdArgs.push_back("--dynamic-linker"); - CmdArgs.push_back(Args.MakeArgString(LibPath + "ld.so.1")); + CmdArgs.push_back( + Args.MakeArgString(getToolChain().GetFilePath("ld.so.1"))); } } @@ -6822,21 +6804,24 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA, if (!Args.hasArg(options::OPT_nostdlib) && !Args.hasArg(options::OPT_nostartfiles)) { - if (!Args.hasArg(options::OPT_shared)) { - CmdArgs.push_back(Args.MakeArgString(LibPath + "crt1.o")); - CmdArgs.push_back(Args.MakeArgString(LibPath + "crti.o")); - CmdArgs.push_back(Args.MakeArgString(LibPath + "values-Xa.o")); - CmdArgs.push_back(Args.MakeArgString(GCCLibPath + "crtbegin.o")); - } else { - CmdArgs.push_back(Args.MakeArgString(LibPath + "crti.o")); - CmdArgs.push_back(Args.MakeArgString(LibPath + "values-Xa.o")); - CmdArgs.push_back(Args.MakeArgString(GCCLibPath + "crtbegin.o")); - } + if (!Args.hasArg(options::OPT_shared)) + CmdArgs.push_back( + Args.MakeArgString(getToolChain().GetFilePath("crt1.o"))); + + CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crti.o"))); + CmdArgs.push_back( + Args.MakeArgString(getToolChain().GetFilePath("values-Xa.o"))); + CmdArgs.push_back( + Args.MakeArgString(getToolChain().GetFilePath("crtbegin.o"))); + if (getToolChain().getDriver().CCCIsCXX()) - CmdArgs.push_back(Args.MakeArgString(LibPath + "cxa_finalize.o")); + CmdArgs.push_back( + Args.MakeArgString(getToolChain().GetFilePath("cxa_finalize.o"))); } - CmdArgs.push_back(Args.MakeArgString("-L" + GCCLibPath)); + const ToolChain::path_list &Paths = getToolChain().getFilePaths(); + for (const auto &Path : Paths) + CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + Path)); Args.AddAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group, options::OPT_e, options::OPT_r}); @@ -6857,9 +6842,10 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA, if (!Args.hasArg(options::OPT_nostdlib) && !Args.hasArg(options::OPT_nostartfiles)) { - CmdArgs.push_back(Args.MakeArgString(GCCLibPath + "crtend.o")); + CmdArgs.push_back( + Args.MakeArgString(getToolChain().GetFilePath("crtend.o"))); } - CmdArgs.push_back(Args.MakeArgString(LibPath + "crtn.o")); + CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o"))); addProfileRT(getToolChain(), Args, CmdArgs); diff --git a/test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crt1.o b/test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crt1.o new file mode 100644 index 0000000..e69de29 diff --git a/test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtbegin.o b/test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtbegin.o new file mode 100644 index 0000000..e69de29 diff --git a/test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtend.o b/test/Driver/Inputs/sparc-sun-solaris2.11/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtend.o new file mode 100644 index 0000000..e69de29 diff --git a/test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/crti.o b/test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/crti.o new file mode 100644 index 0000000..e69de29 diff --git a/test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/crtn.o b/test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/crtn.o new file mode 100644 index 0000000..e69de29 diff --git a/test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/ld.so.1 b/test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/ld.so.1 new file mode 100644 index 0000000..e69de29 diff --git a/test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/values-Xa.o b/test/Driver/Inputs/sparc-sun-solaris2.11/usr/lib/values-Xa.o new file mode 100644 index 0000000..e69de29 diff --git a/test/Driver/solaris-ld.c b/test/Driver/solaris-ld.c new file mode 100644 index 0000000..9590115 --- /dev/null +++ b/test/Driver/solaris-ld.c @@ -0,0 +1,16 @@ +// Test ld invocation on Solaris targets. + +// Check sparc-sun-solaris2.1 +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=sparc-sun-solaris2.11 \ +// RUN: --gcc-toolchain="" \ +// RUN: --sysroot=%S/Inputs/sparc-sun-solaris2.11 \ +// RUN: | FileCheck %s +// CHECK: "-cc1" "-triple" "sparc-sun-solaris2.11" +// CHECK: ld{{.*}}" +// CHECK: "--dynamic-linker" "{{.*}}/usr/lib/ld.so.1" +// CHECK: "{{.*}}/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crt1.o" +// CHECK: "{{.*}}/usr/lib/crti.o" +// CHECK: "{{.*}}/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtbegin.o" +// CHECK: "{{.*}}/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/crtend.o" +// CHECK: "{{.*}}/usr/lib/crtn.o" -- 2.4.3
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits