[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