[PATCH] Add compiler support for dynamic Asan runtime

Alexey Samsonov samsonov at google.com
Mon Mar 24 08:09:28 PDT 2014


  General comments:
  1) please don't modify the behavior of Clang driver for static runtime. I think that some users may depend on the fact that there's a single ASan .a runtime, and wouldn't like to break them without strong reasons.
  2) nit: please decrease the amount of added vertical whitespaces.


================
Comment at: lib/Driver/Tools.cpp:1819
@@ -1821,1 +1818,3 @@
+    const ToolChain &TC, const StringRef Sanitizer, const ArgList &Args,
+    bool NeedsSharedRT = false) {
   // Sanitizer runtime has name "libclang_rt.<Sanitizer>-<ArchName>.a".
----------------
just "bool Shared". Also, I think you can avoid using the default value this argument.

================
Comment at: lib/Driver/Tools.cpp:1820
@@ -1821,2 +1819,3 @@
+    bool NeedsSharedRT = false) {
   // Sanitizer runtime has name "libclang_rt.<Sanitizer>-<ArchName>.a".
   SmallString<128> LibSanitizer = getCompilerRTLibDir(TC);
----------------
Update the comment.

================
Comment at: lib/Driver/Tools.cpp:1870
@@ +1869,3 @@
+    addSanitizerRTStaticLib(TC, Args, CmdArgs, Sanitizer, BeforeLibStdCXX);
+
+    CmdArgs.push_back("-lpthread");
----------------
Add the comment specifying that we need to explicitly link with system libraries static ASan runtime depends on.

================
Comment at: lib/Driver/Tools.cpp:1901
@@ -1868,3 +1900,3 @@
                              getArchNameForCompilerRTLib(TC) + "-android.so"));
     CmdArgs.insert(CmdArgs.begin(), Args.MakeArgString(LibAsan));
   } else {
----------------
use an early return here to decrease identation:
  if (Android) {
    // code
    return;
  }
  // more code



http://llvm-reviews.chandlerc.com/D3043



More information about the llvm-commits mailing list