[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