[PATCH] Add -fsanitize=leak to driver options.

Richard Smith richard at metafoo.co.uk
Tue May 21 11:51:16 PDT 2013



================
Comment at: lib/Driver/Tools.cpp:1611-1615
@@ -1602,1 +1610,7 @@
+      << lastArgumentForKind(D, Args, NeedsMsanRt);
+  // LSan is built into ASan, so enabling them both is redundant.
+  if (NeedsLsan && NeedsAsan)
+    D.Diag(diag::warn_drv_redundant_sanitizer)
+     << "-fsanitize=leak"
+     << "-fsanitize=address";
 
----------------
This seems backwards from how I'd expect this to work: if -fsanitize=address includes this functionality, then I would have expected -fsanitize=address,leak would be fine, and -fsanitize=address -fno-sanitize=leak would give me ASan with no leak checking. If we can't support the latter, then *that* is what we should warn about.

Also, please use lastArgumentForKind rather than hard-coding strings which may not match what the user typed.

================
Comment at: lib/Driver/Tools.cpp:1749-1751
@@ +1748,5 @@
+                           ArgStringList &CmdArgs) {
+  if (!Args.hasArg(options::OPT_shared)) {
+    addSanitizerRTLinkFlagsLinux(TC, Args, CmdArgs, "lsan", true);
+  }
+}
----------------
No braces here, please.

================
Comment at: lib/Driver/Tools.cpp:6146
@@ -6122,1 +6145,3 @@
     addMsanRTLinux(getToolChain(), Args, CmdArgs);
+  if (Sanitize.needsLsanRt() && !Sanitize.needsAsanRt())
+    addLsanRTLinux(getToolChain(), Args, CmdArgs);
----------------
Please put the ASan check in 'needsLsanRt'.


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



More information about the cfe-commits mailing list