[PATCH] add supporting of the sanitizer arguments into Clang on FreeBSD platform.
Alexey Samsonov
samsonov at google.com
Thu Feb 20 06:27:52 PST 2014
================
Comment at: lib/Driver/SanitizerArgs.cpp:253
@@ -251,3 +252,3 @@
bool IsX86_64 = TC.getTriple().getArch() == llvm::Triple::x86_64;
- if (!(IsLinux && IsX86_64)) {
+ if (!(IsLinuxOrFreeBSD && IsX86_64)) {
filterUnsupportedMask(TC, Kinds, Thread | Memory | DataFlow, Args, A,
----------------
You haven't mailed the changes supporting tsan/msan/dfsan on FreeBSD. I suggest to keep filtering them for now, but leaving this up to you.
================
Comment at: lib/Driver/Tools.cpp:227
@@ -226,3 +226,3 @@
// not supported by old linkers.
- std::string ProfileRT =
- std::string(TC.getDriver().Dir) + "/../lib/libprofile_rt.a";
+ SmallString<128> ProfileRT;
+ if (Triple.getOS() == llvm::Triple::FreeBSD) {
----------------
OMG, it turned out there was the correct addProfileRTLinux(), and this function is long obsolete. I've fixed this problem in r201789.
================
Comment at: lib/Driver/Tools.cpp:1823
@@ -1806,1 +1822,3 @@
+ LibSanitizer, "lib",
+ IsFreeBSD ? "freebsd" : "linux",
(Twine("libclang_rt.") + Sanitizer + "-" +
----------------
You can use a newly introduced getOSNameForCompilerRTLib() for that.
================
Comment at: lib/Driver/Tools.cpp:1856
@@ +1855,3 @@
+ } else if (IsFreeBSD) {
+ if (!Args.hasArg(options::OPT_rdynamic) &&
+ !Args.hasArg(options::OPT_static))
----------------
Why do you need this?
* If the user specifies -rdynamic, --export-dynamic will be added to the link line anyway, no harm in duplicating this.
* -static is not supposed to work with ASan anyway.
================
Comment at: lib/Driver/Tools.cpp:6214
@@ -6120,4 +6213,3 @@
- if (!Args.hasArg(options::OPT_nostdlib) &&
- !Args.hasArg(options::OPT_nostartfiles)) {
- if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
+ if (NeedsDefaultLibs) {
+ if (Args.hasArg(options::OPT_shared) || IsPIE)
----------------
Do you have two if(NeedsDefaultLibs) { ... } blocks in a row?
================
Comment at: lib/Driver/Tools.cpp:6145
@@ +6144,3 @@
+
+ if (NeedsDefaultLibs)
+ CmdArgs.push_back("-lrt");
----------------
You add -lrt because of sanitizers in this place, and you add -lpthread(_p)? in another place below. That's why I suggest to link in all the system libraries required by sanitizers in a single place (addSanitizerRTLinkFlags method). By the way, will the sanitizers actually work if the user specifies smth. like --nodefaultlibs? Maybe, it's fine to fail with link error in that case?
================
Comment at: lib/Driver/Tools.cpp:6131
@@ +6130,3 @@
+ addSanitizerRTLinkFlagsLinuxOrFreeBSD(ToolChain, Args, CmdArgs,
+ "asan", false);
+ if (Sanitize.needsTsanRt())
----------------
Why don't you make use of "BeforeLibStdCXX" parameter of addSanitizerRTLinkFlagsLinux(), or even reuse addAsanRTLinux, addTSanRTLinux, and so on?
================
Comment at: lib/Driver/Tools.cpp:6119
@@ +6118,3 @@
+ // whole-archive.
+ if (NeedsSanRt || NeedsUbsanRt) {
+ if (NeedsUbsanRt) {
----------------
Why not re-use the whole chunk of code that calls add?sanRTLinux() in gnutools::Link::ConstructJob (factored out into a separate function)?
http://llvm-reviews.chandlerc.com/D2644
More information about the cfe-commits
mailing list