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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
