r296927 - Add arch-specific directory to search path

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 6 10:29:22 PST 2017


On Mon, Mar 6, 2017 at 7:00 PM, Pirama Arumuga Nainar <pirama at google.com> wrote:
> Adding Reid, Michal
>
>
> On Mon, Mar 6, 2017 at 5:01 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>>
>> On Sat, Mar 4, 2017 at 12:20 AM, Pirama Arumuga Nainar via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>> > Author: pirama
>> > Date: Fri Mar  3 17:20:49 2017
>> > New Revision: 296927
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=296927&view=rev
>> > Log:
>> > Add arch-specific directory to search path
>> >
>> > Summary:
>> >
>> > This change adds an arch-specific subdirectory in <ResourceDir>/lib/<OS>
>> > to the linker search path.  This path also gets added as '-rpath' for
>> > native compilation if a runtime is linked in as a shared object.  This
>> > allows arch-specific libraries to be installed alongside clang.
>> >
>> > Reviewers: danalbert, cbergstrom, javed.absar
>> >
>> > Subscribers: srhines
>> >
>> > Differential Revision: https://reviews.llvm.org/D30015
>> >
>> > Added:
>> >     cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/
>> >     cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/aarch64/
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/aarch64/.keep
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/arm/
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/arm/.keep
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/i386/
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/i386/.keep
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64/
>> >
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64/.keep
>> >     cfe/trunk/test/Driver/arch-specific-libdir-rpath.c
>> >     cfe/trunk/test/Driver/arch-specific-libdir.c
>> > Modified:
>> >     cfe/trunk/include/clang/Driver/ToolChain.h
>> >     cfe/trunk/lib/Driver/ToolChain.cpp
>> >     cfe/trunk/lib/Driver/Tools.cpp
>> >     cfe/trunk/test/lit.cfg
>> >
>> > Modified: cfe/trunk/include/clang/Driver/ToolChain.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=296927&r1=296926&r2=296927&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/include/clang/Driver/ToolChain.h (original)
>> > +++ cfe/trunk/include/clang/Driver/ToolChain.h Fri Mar  3 17:20:49 2017
>> > @@ -299,6 +299,11 @@ public:
>> >    const char *getCompilerRTArgString(const llvm::opt::ArgList &Args,
>> >                                       StringRef Component,
>> >                                       bool Shared = false) const;
>> > +
>> > +  // Returns <ResourceDir>/lib/<OSName>/<arch>.  This is used by
>> > runtimes (such
>> > +  // as OpenMP) to find arch-specific libraries.
>> > +  std::string getArchSpecificLibPath() const;
>> > +
>> >    /// needsProfileRT - returns true if instrumentation profile is on.
>> >    static bool needsProfileRT(const llvm::opt::ArgList &Args);
>> >
>> >
>> > Modified: cfe/trunk/lib/Driver/ToolChain.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=296927&r1=296926&r2=296927&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Driver/ToolChain.cpp (original)
>> > +++ cfe/trunk/lib/Driver/ToolChain.cpp Fri Mar  3 17:20:49 2017
>> > @@ -10,6 +10,7 @@
>> >  #include "clang/Driver/ToolChain.h"
>> >  #include "Tools.h"
>> >  #include "clang/Basic/ObjCRuntime.h"
>> > +#include "clang/Basic/VirtualFileSystem.h"
>> >  #include "clang/Config/config.h"
>> >  #include "clang/Driver/Action.h"
>> >  #include "clang/Driver/Driver.h"
>> > @@ -74,6 +75,10 @@ ToolChain::ToolChain(const Driver &D, co
>> >      if (!isThreadModelSupported(A->getValue()))
>> >        D.Diag(diag::err_drv_invalid_thread_model_for_target)
>> >            << A->getValue() << A->getAsString(Args);
>> > +
>> > +  std::string CandidateLibPath = getArchSpecificLibPath();
>> > +  if (getVFS().exists(CandidateLibPath))
>> > +    getFilePaths().push_back(CandidateLibPath);
>> >  }
>> >
>> >  ToolChain::~ToolChain() {
>> > @@ -320,6 +325,14 @@ const char *ToolChain::getCompilerRTArgS
>> >    return Args.MakeArgString(getCompilerRT(Args, Component, Shared));
>> >  }
>> >
>> > +std::string ToolChain::getArchSpecificLibPath() const {
>> > +  SmallString<128> Path(getDriver().ResourceDir);
>> > +  StringRef OSLibName = getTriple().isOSFreeBSD() ? "freebsd" :
>> > getOS();
>> > +  llvm::sys::path::append(Path, "lib", OSLibName,
>> > +                          llvm::Triple::getArchTypeName(getArch()));
>> > +  return Path.str();
>> > +}
>> > +
>> >  bool ToolChain::needsProfileRT(const ArgList &Args) {
>> >    if (Args.hasFlag(options::OPT_fprofile_arcs,
>> > options::OPT_fno_profile_arcs,
>> >                     false) ||
>> >
>> > Modified: cfe/trunk/lib/Driver/Tools.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=296927&r1=296926&r2=296927&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Driver/Tools.cpp (original)
>> > +++ cfe/trunk/lib/Driver/Tools.cpp Fri Mar  3 17:20:49 2017
>> > @@ -14,6 +14,7 @@
>> >  #include "clang/Basic/LangOptions.h"
>> >  #include "clang/Basic/ObjCRuntime.h"
>> >  #include "clang/Basic/Version.h"
>> > +#include "clang/Basic/VirtualFileSystem.h"
>> >  #include "clang/Config/config.h"
>> >  #include "clang/Driver/Action.h"
>> >  #include "clang/Driver/Compilation.h"
>> > @@ -238,8 +239,9 @@ static void AddLinkerInputs(const ToolCh
>> >
>> >    // LIBRARY_PATH - included following the user specified library
>> > paths.
>> >    //                and only supported on native toolchains.
>> > -  if (!TC.isCrossCompiling())
>> > +  if (!TC.isCrossCompiling()) {
>> >      addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
>> > +  }
>> >  }
>> >
>> >  /// Add OpenMP linker script arguments at the end of the argument list
>> > so that
>> > @@ -2000,6 +2002,19 @@ static void CollectArgsForIntegratedAsse
>> >    }
>> >  }
>> >
>> > +static void addArchSpecificRPath(const ToolChain &TC, const ArgList
>> > &Args,
>> > +                                 ArgStringList &CmdArgs) {
>> > +  // In the cross-compilation case, arch-specific library path is
>> > likely
>> > +  // unavailable at runtime.
>> > +  if (TC.isCrossCompiling()) return;
>>
>> Why? This is fundamentally against the design of the clang driver. It
>> should be always possible to use clang as a cross compiler and the
>> existence of isCrossCompiling() is an accident of history. What's
>> breaking without this check? Please either make this compatible with
>> cross compilation or revert this patch.
>
>
> This is just a heuristic to prevent an rpath that is most likely unavailable
> at runtime from being embedded into an object.   Michal suggested I add the
> rpath during review.
>
> I'm happy to add the rpath in all cases, if that'd make the Driver behave
> consistently.

Always adding the rpath sounds good to me.

>>
>> > +
>> > +  std::string CandidateRPath = TC.getArchSpecificLibPath();
>> > +  if (TC.getVFS().exists(CandidateRPath)) {
>> > +    CmdArgs.push_back("-rpath");
>> > +    CmdArgs.push_back(Args.MakeArgString(CandidateRPath.c_str()));
>> > +  }
>> > +}
>> > +
>> >  static void addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain
>> > &TC,
>> >                                const ArgList &Args) {
>> >    if (!Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
>> > @@ -2020,6 +2035,8 @@ static void addOpenMPRuntime(ArgStringLi
>> >      // Already diagnosed.
>> >      break;
>> >    }
>> > +
>> > +  addArchSpecificRPath(TC, Args, CmdArgs);
>> >  }
>> >
>> >  static void addSanitizerRuntime(const ToolChain &TC, const ArgList
>> > &Args,
>> > @@ -2030,6 +2047,10 @@ static void addSanitizerRuntime(const To
>> >    if (IsWhole) CmdArgs.push_back("-whole-archive");
>> >    CmdArgs.push_back(TC.getCompilerRTArgString(Args, Sanitizer,
>> > IsShared));
>> >    if (IsWhole) CmdArgs.push_back("-no-whole-archive");
>> > +
>> > +  if (IsShared) {
>> > +    addArchSpecificRPath(TC, Args, CmdArgs);
>> > +  }
>> >  }
>> >
>> >  // Tries to use a file with the list of dynamic symbols that need to be
>> > exported
>> > @@ -9002,6 +9023,8 @@ void gnutools::Linker::ConstructJob(Comp
>> >          }
>> >          if (JA.isHostOffloading(Action::OFK_OpenMP))
>> >            CmdArgs.push_back("-lomptarget");
>> > +
>> > +        addArchSpecificRPath(ToolChain, Args, CmdArgs);
>> >        }
>> >
>> >        AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
>> >
>> > Added:
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/aarch64/.keep
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/aarch64/.keep?rev=296927&view=auto
>> >
>> > ==============================================================================
>> >     (empty)
>> >
>> > Added:
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/arm/.keep
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/arm/.keep?rev=296927&view=auto
>> >
>> > ==============================================================================
>> >     (empty)
>> >
>> > Added:
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/i386/.keep
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/i386/.keep?rev=296927&view=auto
>> >
>> > ==============================================================================
>> >     (empty)
>> >
>> > Added:
>> > cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64/.keep
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64/.keep?rev=296927&view=auto
>> >
>> > ==============================================================================
>> >     (empty)
>> >
>> > Added: cfe/trunk/test/Driver/arch-specific-libdir-rpath.c
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arch-specific-libdir-rpath.c?rev=296927&view=auto
>> >
>> > ==============================================================================
>> > --- cfe/trunk/test/Driver/arch-specific-libdir-rpath.c (added)
>> > +++ cfe/trunk/test/Driver/arch-specific-libdir-rpath.c Fri Mar  3
>> > 17:20:49 2017
>> > @@ -0,0 +1,50 @@
>> > +// Test that the driver adds an arch-specific subdirectory in
>> > +// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath'
>> > for native
>> > +// compilations.
>> > +//
>> > +// -rpath only gets added during native compilation.  To keep the test
>> > simple,
>> > +// just test for x86_64-linux native compilation.
>> > +// REQUIRES: x86_64-linux
>> > +//
>> > +// Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
>> > +// RUN: %clang %s -### 2>&1 -fsanitize=undefined \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
>> > +//
>> > +// Add LIBPATH, RPATH for -fsanitize=address -shared-libasan
>> > +// RUN: %clang %s -### 2>&1 -target x86_64-linux \
>> > +// RUN:     -fsanitize=address -shared-libasan \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
>> > +//
>> > +// Add LIBPATH, RPATH with -fsanitize=address for Android
>> > +// RUN: %clang %s -### 2>&1 -target x86_64-linux-android
>> > -fsanitize=address \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
>> > +//
>> > +// Add LIBPATH, RPATH for OpenMP
>> > +// RUN: %clang %s -### 2>&1 -fopenmp \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
>> > +//
>> > +// Add LIBPATH but no RPATH for ubsan (or any other sanitizer)
>> > +// RUN: %clang %s -### 2>&1 -fsanitize=undefined \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
>> > +//
>> > +// Add LIBPATH but no RPATH if no sanitizer or runtime is specified
>> > +// RUN: %clang %s -### 2>&1 \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
>> > +//
>> > +// Do not add LIBPATH or RPATH if arch-specific subdir doesn't exist
>> > +// RUN: %clang %s -### 2>&1 \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-LIBPATH,NO-RPATH %s
>> > +//
>> > +//
>> > +// FILEPATH: "-x" "c" "[[FILE_PATH:.*]]/{{.*}}.c"
>> > +// LIBPATH:
>> > -L[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64
>> > +// RPATH: "-rpath"
>> > "[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64"
>> > +// NO-LIBPATH-NOT: -L{{.*}}Inputs/resource_dir
>> > +// NO-RPATH-NOT: "-rpath" {{.*}}/Inputs/resource_dir
>> >
>> > Added: cfe/trunk/test/Driver/arch-specific-libdir.c
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arch-specific-libdir.c?rev=296927&view=auto
>> >
>> > ==============================================================================
>> > --- cfe/trunk/test/Driver/arch-specific-libdir.c (added)
>> > +++ cfe/trunk/test/Driver/arch-specific-libdir.c Fri Mar  3 17:20:49
>> > 2017
>> > @@ -0,0 +1,53 @@
>> > +// Test that the driver adds an arch-specific subdirectory in
>> > +// {RESOURCE_DIR}/lib/linux to the search path.
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target i386-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-i386 %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target i386-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-ARCHDIR %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target i686-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-i386 %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target i686-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-ARCHDIR %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target x86_64-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-x86_64 %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target x86_64-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-ARCHDIR %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target arm-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-arm %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target arm-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-ARCHDIR %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target aarch64-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-aarch64 %s
>> > +//
>> > +// RUN: %clang %s -### 2>&1 -target aarch64-unknown-linux \
>> > +// RUN:     -resource-dir=%S/Inputs/resource_dir \
>> > +// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-ARCHDIR %s
>> > +//
>> > +//
>> > +// FILEPATH: "-x" "c" "[[FILE_PATH:.*]]/{{.*}}.c"
>> > +// ARCHDIR-i386:
>> > -L[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/i386
>> > +// ARCHDIR-x86_64:
>> > -L[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64
>> > +// ARCHDIR-arm:
>> > -L[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/arm
>> > +// ARCHDIR-aarch64:
>> > -L[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/aarch64
>> > +//
>> > +// Have a stricter check for no-archdir - that the driver doesn't add
>> > any
>> > +// subdirectory from the provided resource directory.
>> > +// NO-ARCHDIR-NOT: -L[[FILE_PATH]]/Inputs/resource_dir
>> >
>> > Modified: cfe/trunk/test/lit.cfg
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg?rev=296927&r1=296926&r2=296927&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/test/lit.cfg (original)
>> > +++ cfe/trunk/test/lit.cfg Fri Mar  3 17:20:49 2017
>> > @@ -397,6 +397,10 @@ if platform.system() not in ['Darwin', '
>> >  if config.host_triple == config.target_triple:
>> >      config.available_features.add("native")
>> >
>> > +# Test Driver/arch-specific-libdir-rpath.c is restricted to
>> > x86_64-linux
>> > +if re.match(r'^x86_64.*-linux', config.target_triple):
>> > +    config.available_features.add("x86_64-linux")
>> > +
>> >  # Case-insensitive file system
>> >  def is_filesystem_case_insensitive():
>> >      handle, path = tempfile.mkstemp(prefix='case-test',
>> > dir=config.test_exec_root)
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>


More information about the cfe-commits mailing list