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

Alexey Samsonov samsonov at google.com
Wed May 22 00:58:51 PDT 2013


  You will also need to update LSan .rst docs to include example usage and add a testcases (see tools/clang/test/Driver/fsanitize.c)


================
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";
 
----------------
Richard Smith wrote:
> 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?
I'm a bit confused =/. Currently we don't want LeakSanitizer to be turned on by default every time we use ASan (this will hopefully change later). So, can we make "-fsanitize=address" and "-fsanitize=leak" independent and do the following:
1) Only "-fsanitize=leak" - ok, link with standalone LSan runtime
2) Only "-fsanitize=address" - ok, run ASan w/o LSan.
3) Both - no warning, run ASan with LSan.
The question is: how can we turn on/off LSan in ASan at link-time? We discussed the option of linking in additional small "library" that
would enable leak checking in ASan.

Alternatively, for now you can disallow -fsanitize=leak with all the sanitizers until it's ready to be turned on by default in ASan.


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



More information about the cfe-commits mailing list