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

Richard Smith richard at metafoo.co.uk
Tue May 21 18:17:32 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";
 
----------------
Sergey Matveev wrote:
> Richard Smith wrote:
> > 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.
> What effect would -fno-sanitize=address have? It could not disable just the address checking, or else "-fsanitize=address -fno-sanitize=address" would have the counterintuitive effect of enabling leak checking. Similarly, it could not disable both address and leak checking, because that would make "-fsanitize=leak -fsanitize=address -fno-sanitize=address" a no-op, whereas one expects it to have the same effect as -fsanitize=leak.
> 
> To make this approach consistent, we would have to separate -fsanitize=leak into two flags - one for enabling stand-alone leak checking, and one for enabling it in ASan. But that is a completely unnecessary complication. Having the leak checking code built into ASan comes at no additional cost, so there's no reason to want to disable it at link time.
We wouldn't have to split up -fsanitize=leak, we could instead split up -fsanitize=address into two flags. From the user's perspective, you have two separate checkers here, one for leaks and one for address errors, and it makes sense for there to be separate flags controlling them. Perhaps -fsanitize=leak should just be another flag in the -fsanitize=address-full group?


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



More information about the cfe-commits mailing list