[cfe-commits] r150846 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CStringChecker.cpp lib/StaticAnalyzer/Checkers/InterCheckerAPI.h lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc.c

[Jordy Rose]

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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits