Does MallocChecker really "need" to know this? I mean, I doubt CStringCheckerBasic is a huge performance hit, but it seems like someone who wants the analyzer to model C library string functions should enable CStringChecker themselves.
(I see that someone on Radar thought the answer was "yes", but...) Also, separately...is this the way we're going to do inter-checker dependencies? I like how simple it is! And since all the checker registration is done with vectors, it even handles ordering. But I don't know how it will scale, and it does sort of compel us to expose a registration function for /every/ checker. (Just like warning flags, if we decide it's okay not to have a registration function for some checker, we'll end up in a situation where we need it.) Jordy On Feb 18, 2012, at 5:35, Anna Zaks wrote: > Author: zaks > Date: Fri Feb 17 16:35:31 2012 > New Revision: 150846 > > URL: http://llvm.org/viewvc/llvm-project?rev=150846&view=rev > Log: > [analyzer] Fix another false positive in the Malloc Checker, by making > it aware of CString APIs that return the input parameter. > > Malloc Checker needs to know how the 'strcpy' function is > evaluated. Introduce the dependency on CStringChecker for that. > CStringChecker knows all about these APIs. > > Addresses radar://10864450 > > Added: > cfe/trunk/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h > Modified: > cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp > cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp > cfe/trunk/test/Analysis/malloc.c > > Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=150846&r1=150845&r2=150846&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Fri Feb 17 > 16:35:31 2012 > @@ -13,6 +13,7 @@ > //===----------------------------------------------------------------------===// > > #include "ClangSACheckers.h" > +#include "InterCheckerAPI.h" > #include "clang/StaticAnalyzer/Core/Checker.h" > #include "clang/StaticAnalyzer/Core/CheckerManager.h" > #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" > @@ -1924,3 +1925,7 @@ > REGISTER_CHECKER(CStringOutOfBounds) > REGISTER_CHECKER(CStringBufferOverlap) > REGISTER_CHECKER(CStringNotNullTerm) > + > +void ento::registerCStringCheckerBasic(CheckerManager &Mgr) { > + registerCStringNullArg(Mgr); > +} > > Added: cfe/trunk/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h?rev=150846&view=auto > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h (added) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h Fri Feb 17 > 16:35:31 2012 > @@ -0,0 +1,22 @@ > +//==--- InterCheckerAPI.h ---------------------------------------*- C++ > -*-==// > +// > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open Source > +// License. See LICENSE.TXT for details. > +// > +//===----------------------------------------------------------------------===// > +// This file allows introduction of checker dependencies. It contains APIs > for > +// inter-checker communications. > +//===----------------------------------------------------------------------===// > + > +#ifndef INTERCHECKERAPI_H_ > +#define INTERCHECKERAPI_H_ > +namespace clang { > +namespace ento { > + > +/// Register the checker which evaluates CString API calls. > +void registerCStringCheckerBasic(CheckerManager &Mgr); > + > +}} > +#endif /* INTERCHECKERAPI_H_ */ > > Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=150846&r1=150845&r2=150846&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Fri Feb 17 > 16:35:31 2012 > @@ -13,6 +13,7 @@ > //===----------------------------------------------------------------------===// > > #include "ClangSACheckers.h" > +#include "InterCheckerAPI.h" > #include "clang/StaticAnalyzer/Core/Checker.h" > #include "clang/StaticAnalyzer/Core/CheckerManager.h" > #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" > @@ -1130,6 +1131,7 @@ > > #define REGISTER_CHECKER(name) \ > void ento::register##name(CheckerManager &mgr) {\ > + registerCStringCheckerBasic(mgr); \ > mgr.registerChecker<MallocChecker>()->Filter.C##name = true;\ > } > > > Modified: cfe/trunk/test/Analysis/malloc.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=150846&r1=150845&r2=150846&view=diff > ============================================================================== > --- cfe/trunk/test/Analysis/malloc.c (original) > +++ cfe/trunk/test/Analysis/malloc.c Fri Feb 17 16:35:31 2012 > @@ -594,6 +594,26 @@ > strcpy(p, s); // expected-warning {{leak}} > } > > +// Rely on the CString checker evaluation of the strcpy API to convey that > the result of strcpy is equal to p. > +void symbolLostWithStrcpy(char *s) { > + char *p = malloc(12); > + p = strcpy(p, s); > + free(p); > +} > + > + > +// The same test as the one above, but with what is actually generated on a > mac. > +static __inline char * > +__inline_strcpy_chk (char *restrict __dest, const char *restrict __src) > +{ > + return __builtin___strcpy_chk (__dest, __src, __builtin_object_size > (__dest, 2 > 1)); > +} > + > +void symbolLostWithStrcpy_InlineStrcpyVersion(char *s) { > + char *p = malloc(12); > + p = ((__builtin_object_size (p, 0) != (size_t) -1) ? > __builtin___strcpy_chk (p, s, __builtin_object_size (p, 2 > 1)) : > __inline_strcpy_chk (p, s)); > + free(p); > +} > // Below are the known false positives. > > // TODO: There should be no warning here. This one might be difficult to get > rid of. > @@ -627,13 +647,6 @@ > return p;// expected-warning {{Memory is never released; potential memory > leak}} > } > > -// TODO: This is a false positve that should be fixed by making CString > checker smarter. > -void symbolLostWithStrcpy(char *s) { > - char *p = malloc(12); > - p = strcpy(p, s); > - free(p);// expected-warning {{leak}} > -} > - > // False negatives. > > // TODO: This requires tracking symbols stored inside the structs/arrays. > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
