[compiler-rt] Use @rpath as LC_ID_DYLIB for ASan dylib on OS X

[kubamracek]

Summary

The current LC_ID_DYLIB is hardcoded to be the absolute path to the build result. This means that ASan only works out-of-the-box if you never move the compiler/libraries from the directory you built it in. We currently have an issue with the prebuilt binary releases, because of the hardcoded path: http://llvm.org/bugs/show_bug.cgi?id=21316

This patch proposes to change the LC_ID_DYLIB of ASan's dynamic libraries on OS X to be set to "@rpath/libclang_rt.asan_osx_dynamic.dylib" and similarly for iossim. Clang driver then sets the "-rpath" to be the real path to where clang currently has the dylib (because clang uses the relative path to its current executable). This means if you move the compiler or install the binary release, -fsanitize=address will link to the proper library. The remaining issue is that if you delete or move the dylib, the previously linked executables will still have the old path in them, but this issue already exists with the current state as well.

I also modified one of ASan tests not to depend on the fact that the dylib is referenced with the full path. Instead, it now locates the dylib by issuing "clang -fsanitize=address -###" and finding the dylib path there.

[kubamracek]

Updated the patch to also add @executable_path to rpath to support having the dylib next to the executable. Added comments explaining the use cases. Added a clang test case.

[glider]

LGTM, but we probably want to hear Nick's opinion on this.

tools/clang/lib/Driver/ToolChains.cpp

402–403

Can you please comment which these bool constants stand for?
E.g. "/*AlwaysLink*/ true" etc.

[kubamracek]

Added the comments for bool constants.

[samsonov]

tools/clang/lib/Driver/ToolChains.cpp

313

DarwinStaticLib name makes no sense at this point. Consider fixing this in a preliminary commit.

tools/clang/test/Driver/darwin-asan-rpath.c

1

Can you instead fix the existing test for linking ASan runtime on Darwin (if there is any)?

[kledzik]

I'm not a clang driver guy, but it would probably be cleaner to leave AddLinkRuntimeLib() as is, and instead add the CmdArgs.push_back()s inside the if (Sanitize.needsAsanRt()).

It looks like the clang driver will always add two rpath entries when building for ASan? Is there anyway to suppress that if Xcode knows better? That is, Xcode knows a better location for the runtime lib?

[kubamracek]

DarwinStaticLib name makes no sense at this point. Consider fixing this in a preliminary commit.

Fixed in r220939, http://reviews.llvm.org/D6040.

Can you instead fix the existing test for linking ASan runtime on Darwin (if there is any)?

Done.

I'm not a clang driver guy, but it would probably be cleaner to leave AddLinkRuntimeLib() as is, and instead add the CmdArgs.push_back()s inside the if (Sanitize.needsAsanRt()).

You're right that this currently applies only to ASan, but we might want to reuse this rpath logic for other future compiler-rt sanitizer (tsan, ubsan) dylibs. Leaving it for now, but if you strongly feel that we shouldn't touch AddLinkRuntimeLib, let me know and I'll change the patch.

It looks like the clang driver will always add two rpath entries when building for ASan? Is there anyway to suppress that if Xcode knows better? That is, Xcode knows a better location for the runtime lib?

I think Xcode would have to use install_name_tool to change the rpath entries manually. We could add another flag to disable adding these rpaths in the driver. Alexander, what do you think? Either way, if we agree on adding these rpaths by default, let's start with this patch and then consider adding that flag in a separate patch.

[samsonov]

I'm OK with the patch as is. I'd like to avoid adding one more flag for controlling linking strategy for as long as possible =)

[kubamracek]

Landed in r221278 and r221279.

projects/compiler-rt/lib/asan/CMakeLists.txt

 # Build for the AddressSanitizer runtime support library.
 
-if(APPLE)
-# Don't set rpath for the ASan libraries. Developers are encouraged to ship
-# their binaries together with the corresponding ASan runtime libraries,
-# so they'll anyway need to fix the rpath and the install name.
-set(CMAKE_BUILD_WITH_INSTALL_RPATH OFF)
-endif()
-
 set(ASAN_SOURCES
   asan_allocator2.cc
   asan_activation.cc

projects/compiler-rt/test/asan/TestCases/Darwin/interface_symbols_darwin.c

 // RUN: %clang_asan -dead_strip -O2 %s -o %t.exe
 // RUN: rm -f %t.symbols %t.interface
 
-// RUN: nm -g `otool -L %t.exe | grep "asan_osx_dynamic.dylib" | \
-// RUN:                       tr -d '\011' | \
-// RUN:                       sed "s/.dylib.*/.dylib/"` \
+// RUN: nm -g `%clang_asan %s -fsanitize=address -### 2>&1 | grep "libclang_rt.asan_osx_dynamic.dylib" | sed -e 's/.*"\(.*libclang_rt.asan_osx_dynamic.dylib\)".*/\1/'` \
 // RUN:   | grep " T " | sed "s/.* T //" \
 // RUN:   | grep "__asan_" | sed "s/___asan_/__asan_/" \
 // RUN:   | sed -E "s/__asan_init_v[0-9]+/__asan_init/" \

tools/clang/lib/Driver/ToolChains.h

                          llvm::opt::ArgStringList &CmdArgs,
                          StringRef DarwinStaticLib,
                          bool AlwaysLink = false,
-                         bool IsEmbedded = false) const;
+                         bool IsEmbedded = false,
+                         bool AddRPath = false) const;
 
   /// }
   /// @name ToolChain Implementation

tools/clang/lib/Driver/ToolChains.cpp

 
 void MachO::AddLinkRuntimeLib(const ArgList &Args, ArgStringList &CmdArgs,
                               StringRef DarwinStaticLib, bool AlwaysLink,
-                              bool IsEmbedded) const {
-  SmallString<128> P(getDriver().ResourceDir);
-  llvm::sys::path::append(P, "lib", IsEmbedded ? "macho_embedded" : "darwin",
-                          DarwinStaticLib);
+                              bool IsEmbedded, bool AddRPath) const {
+  SmallString<128> Dir(getDriver().ResourceDir);
+  llvm::sys::path::append(Dir, "lib", IsEmbedded ? "macho_embedded" : "darwin");
 
+  SmallString<128> P(Dir);
+  llvm::sys::path::append(P, DarwinStaticLib);
+
   // For now, allow missing resource libraries to support developers who may
   // not have compiler-rt checked out or integrated into their build (unless
   // we explicitly force linking with this library).
   if (AlwaysLink || llvm::sys::fs::exists(P.str()))
     CmdArgs.push_back(Args.MakeArgString(P.str()));
+
+  // Adding the rpaths might negatively interact when other rpaths are involved,
+  // so we should make sure we add the rpaths last, after all user-specified
+  // 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 (AddRPath) {
+    assert(DarwinStaticLib.endswith(".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.
+    CmdArgs.push_back("-rpath");
+    CmdArgs.push_back(Args.MakeArgString(Dir.str()));
+  }
 }
 
 void DarwinClang::AddLinkRuntimeLibArgs(const ArgList &Args,

       if (isTargetMacOS()) {
         AddLinkRuntimeLib(Args, CmdArgs,
                           "libclang_rt.asan_osx_dynamic.dylib",
-                          true);
+                          /*AlwaysLink*/ true, /*IsEmbedded*/ false,
+                          /*AddRPath*/ true);
       } else {
         if (isTargetIOSSimulator()) {
           AddLinkRuntimeLib(Args, CmdArgs,
                             "libclang_rt.asan_iossim_dynamic.dylib",
-                            true);
+                            /*AlwaysLink*/ true, /*IsEmbedded*/ false,
+                            /*AddRPath*/ true);
         }
       }
     }

tools/clang/test/Driver/darwin-asan-rpath.c

+// RUN: %clang -x c -target x86_64-apple-darwin -fsanitize=address %s -### 2>&1 | FileCheck --check-prefix=CHECK-OSX %s
+// CHECK-OSX: "{{.*}}/lib/darwin/libclang_rt.asan_osx_dynamic.dylib"
+// CHECK-OSX: "-rpath" "{{.*}}/lib/darwin"
+
+// RUN: %clang -x c -target x86_64-apple-darwin -mios-simulator-version-min=7.0 -fsanitize=address %s -### 2>&1 | FileCheck --check-prefix=CHECK-IOSSIM %s
+// CHECK-IOSSIM: "{{.*}}/lib/darwin/libclang_rt.asan_iossim_dynamic.dylib"
+// CHECK-IOSSIM: "-rpath" "{{.*}}/lib/darwin"