[clang][driver] Correcting arguments when using libFuzzer with -shared-libsan

[bjosv]

This PR contains two commits:

• Add required dependencies when using -shared-libsan and fuzzer.
  Since libFuzzer is a static library we need to make sure that we add its dependencies when building with -shared-libsan. E.g libFuzzer uses ceilf() from libm.so when building on Gnu toolchain.
  Previously, the resulting command did not contain the required link libraries, giving build failures
  (only a static sanitizer runtime would trigger the call to linkSanitizerRuntimeDeps).

• Correcting dependency order when using fuzzer.
  When building using -shared-libsan the sanitizer library needs to be first in link order.
  Since the fuzzer requires -lstdc++ we have to make sure that the sanitizer library is added before -lstdc++.

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Björn Svensson (bjosv)

Changes

This PR contains two commits:

• Add required dependencies when using -shared-libsan and fuzzer.
  Since libFuzzer is a static library we need to make sure that we add its dependencies when building with -shared-libsan. E.g libFuzzer uses ceilf() from libm.so when building on Gnu toolchain.
  Previously, the resulting command did not contain the required link libraries, giving build failures
  (only a static sanitizer runtime would trigger the call to linkSanitizerRuntimeDeps).

• Correcting dependency order when using fuzzer.
  When building using -shared-libsan the sanitizer library needs to be first in link order.
  Since the fuzzer requires -lstdc++ we have to make sure that the sanitizer library is added before -lstdc++.

---

Full diff: https://github.com/llvm/llvm-project/pull/164842.diff

3 Files Affected:

• (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+8-3)
• (modified) clang/test/Driver/fuzzer.c (+7-1)
• (modified) clang/test/Driver/sanitizer-ld.c (+8)

diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 99400ac701fbe..bbd30b83ccefd 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1716,11 +1716,17 @@ bool tools::addSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
     CmdArgs.push_back(Args.MakeArgString(S));
   }
 
+  // Add shared runtimes before adding fuzzer and its dependencies.
+  for (auto RT : SharedRuntimes)
+    addSanitizerRuntime(TC, Args, CmdArgs, RT, true, false);
+
   // Inject libfuzzer dependencies.
+  bool FuzzerNeedsSanitizerDeps = false;
   if (SanArgs.needsFuzzer() && SanArgs.linkRuntimes() &&
       !Args.hasArg(options::OPT_shared)) {
 
     addSanitizerRuntime(TC, Args, CmdArgs, "fuzzer", false, true);
+    FuzzerNeedsSanitizerDeps = true;
     if (SanArgs.needsFuzzerInterceptors())
       addSanitizerRuntime(TC, Args, CmdArgs, "fuzzer_interceptors", false,
                           true);
@@ -1735,8 +1741,6 @@ bool tools::addSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
     }
   }
 
-  for (auto RT : SharedRuntimes)
-    addSanitizerRuntime(TC, Args, CmdArgs, RT, true, false);
   for (auto RT : HelperStaticRuntimes)
     addSanitizerRuntime(TC, Args, CmdArgs, RT, false, true);
   bool AddExportDynamic = false;
@@ -1769,7 +1773,8 @@ bool tools::addSanitizerRuntimes(const ToolChain &TC, const ArgList &Args,
       CmdArgs.push_back("--android-memtag-stack");
   }
 
-  return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
+  return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty() ||
+         FuzzerNeedsSanitizerDeps;
 }
 
 bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, ArgStringList &CmdArgs) {
diff --git a/clang/test/Driver/fuzzer.c b/clang/test/Driver/fuzzer.c
index 409fbfac8ce1d..8ca7f32bf4d8e 100644
--- a/clang/test/Driver/fuzzer.c
+++ b/clang/test/Driver/fuzzer.c
@@ -26,7 +26,7 @@
 // CHECK-NOLIB-NOT: libclang_rt.libfuzzer
 // CHECK-COV: -fsanitize-coverage-inline-8bit-counters
 
-// Check that we respect whether thes tandard library should be linked
+// Check that we respect whether the standard library should be linked.
 // statically.
 //
 // RUN: %clang -fsanitize=fuzzer --target=i386-unknown-linux -stdlib=libstdc++ %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LIBSTDCXX-DYNAMIC %s
@@ -43,6 +43,12 @@
 // RUN: %clang -fsanitize=fuzzer --target=i386-unknown-linux -stdlib=libc++ -static-libstdc++ %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LIBCXX-STATIC %s
 // CHECK-LIBCXX-STATIC: "-Bstatic" "-lc++"
 
+// Check that we add required sanitizer dependencies when dynamically linking
+// the sanitizer runtime (e.g. libFuzzer uses ceilf in libm).
+//
+// RUN: %clang -fsanitize=fuzzer -shared-libsan --target=x86_64-linux-gnu %s -### 2>&1 | FileCheck --check-prefixes=CHECK-SHARED-LIBSAN %s
+// CHECK-SHARED-LIBSAN: -lm
+
 int LLVMFuzzerTestOneInput(const char *Data, long Size) {
   return 0;
 }
diff --git a/clang/test/Driver/sanitizer-ld.c b/clang/test/Driver/sanitizer-ld.c
index ac1851286af63..89003b4e10ccd 100644
--- a/clang/test/Driver/sanitizer-ld.c
+++ b/clang/test/Driver/sanitizer-ld.c
@@ -1393,3 +1393,11 @@
 // RUN:   | %{filecheck} --check-prefix=CHECK-RELOCATABLE-LINK-TSAN-RTLIB
 //
 // CHECK-RELOCATABLE-LINK-TSAN-RTLIB-NOT: "{{.*}}tsan{{.*}}"
+
+// RUN: %clang -fsanitize=fuzzer,address -shared-libsan -### %s 2>&1 \
+// RUN:     --target=x86_64-unknown-linux -fuse-ld=ld \
+// RUN:     -resource-dir=%S/Inputs/resource_dir \
+// RUN:     --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck %s --check-prefix=CHECK-FUZZER-WITH-SHARED-ASAN-ORDER
+//
+// CHECK-FUZZER-WITH-SHARED-ASAN-ORDER: "{{.*}}/libclang_rt.asan.so" "--whole-archive" "{{.*}}/libclang_rt.fuzzer.a" "--no-whole-archive" "-lstdc++"

[bjosv]

@vitalybuka Maybe you have knowledge within this area and might have some comments?

[bjosv]

@ndrewh As someone knowledgeable in sanitizers and the fuzzer, do you have any thoughts about this correction?

[ndrewh]

I think this makes sense, but I won't have a chance to test it until Monday. I also added @DanBlackwell who may have some thoughts.