[Driver] Use --push/pop-state with Sanitizer link deps

[phosek]

Summary

Sanitizer runtime link deps handling passes --no-as-needed because of
PR15823, but it never undoes it and this flag may affect other libraries
that come later on the link line. To avoid this, wrap Sanitizer link
deps in --push/pop-state.

[morehouse]

LGTM

[pcc]

Unfortunately it looks like the Android NDK uses some ancient version of gold that doesn't support --push-state, so we probably can't rely on being able to use it.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/17287/steps/run%20lit%20tests%20%5Bi686%2Ffugu-userdebug%2FN2G48C%5D/logs/stdio
As an alternative solution, could we add these flags at the start of the linker command line? That way, we're guaranteed that the linker will be in the --no-as-needed state.

[MaskRay]

In D54805#1305749, @pcc wrote:

Unfortunately it looks like the Android NDK uses some ancient version of gold that doesn't support --push-state, so we probably can't rely on being able to use it.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/17287/steps/run%20lit%20tests%20%5Bi686%2Ffugu-userdebug%2FN2G48C%5D/logs/stdio
As an alternative solution, could we add these flags at the start of the linker command line? That way, we're guaranteed that the linker will be in the --no-as-needed state.

gold --push-state seems a new thing. It is available since Dec 2016 (version 1.14)

I'm also interested in the history of

// Force linking against the system libraries sanitizers depends on
// (see PR15823 why this is necessary).
CmdArgs.push_back("--no-as-needed");

PR15823 leans slightly toward the user-error side to me. Shouldn't the user ensure the use of -Wl,--as-needed is eventually closed so that system libraries start with the -Wl--no-as-needed state?

[phosek]

I have already reverted the change as r347430 after seeing the failure on sanitizer bots.

lib/Driver/ToolChains/CommonArgs.cpp

 }
 
 
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
                                      ArgStringList &CmdArgs) {
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
+  CmdArgs.push_back("--push-state");
   CmdArgs.push_back("--no-as-needed");
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
       !TC.getTriple().isAndroid()) {
     CmdArgs.push_back("-lpthread");
     if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
       CmdArgs.push_back("-lrt");
   }
   CmdArgs.push_back("-lm");
   // There's no libdl on all OSes.
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
       TC.getTriple().getOS() != llvm::Triple::NetBSD &&
       TC.getTriple().getOS() != llvm::Triple::OpenBSD &&
       TC.getTriple().getOS() != llvm::Triple::RTEMS)
     CmdArgs.push_back("-ldl");
   // Required for backtrace on some OSes
   if (TC.getTriple().getOS() == llvm::Triple::NetBSD ||
       TC.getTriple().getOS() == llvm::Triple::FreeBSD)
     CmdArgs.push_back("-lexecinfo");
+  CmdArgs.push_back("--pop-state");
 }
 
 static void
 collectSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
                          SmallVectorImpl<StringRef> &SharedRuntimes,
                          SmallVectorImpl<StringRef> &StaticRuntimes,
                          SmallVectorImpl<StringRef> &NonWholeStaticRuntimes,
                          SmallVectorImpl<StringRef> &HelperStaticRuntimes,

test/Driver/sanitizer-ld.c

 // RUN:     -target sparcel-myriad-rtems-elf -fuse-ld=ld -fsanitize=address \
 // RUN:     --sysroot=%S/Inputs/basic_myriad_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-MYRIAD %s
 //
 // CHECK-ASAN-MYRIAD: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-ASAN-MYRIAD-NOT: "-lc"
 // CHECK-ASAN-MYRIAD: libclang_rt.asan-sparcel.a"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN:     -target i386-unknown-linux -fuse-ld=ld -fsanitize=address -static \
+// RUN:     -resource-dir=%S/Inputs/resource_dir \
+// RUN:     --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-LINUX-STATIC %s
+//
+// CHECK-ASAN-LINUX-STATIC: "--push-state"
+// CHECK-ASAN-LINUX-STATIC: "--no-as-needed"
+// CHECK-ASAN-LINUX-STATIC: "-lpthread"
+// CHECK-ASAN-LINUX-STATIC: "-lrt"
+// CHECK-ASAN-LINUX-STATIC: "-lm"
+// CHECK-ASAN-LINUX-STATIC: "-ldl"
+// CHECK-ASAN-LINUX-STATIC: "--pop-state"
+
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     -target x86_64-unknown-linux -fuse-ld=ld -stdlib=platform -lstdc++ \
 // RUN:     -fsanitize=thread \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
 // RUN:     --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-TSAN-LINUX-CXX %s
 //
 // CHECK-TSAN-LINUX-CXX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"