[cfe-commits] r86464 - in /cfe/trunk: include/clang/Analysis/LocalCheckers.h include/clang/Frontend/Analyses.def lib/Analysis/CheckSizeofPointer.cpp lib/Frontend/AnalysisConsumer.cpp

[Ted Kremenek]

Hi Zhongxing,

I'm a little concerned about the promiscuity of this check in general,  
as it seems like it could produce a lot of false positives.  Have you  
tried running this check over several real-world projects to get a  
feel on the false positive rate (or even the number of times this  
check triggers)?

It also seems to me that this check needs to be flow/path sensitive.   
The diagnostic says that the pointer is "malloced", but in truth the  
check will fire when taking the sizeof of any pointer expression.  In  
order for the check to be accurate, it seems like we would need to  
track if symbol was actually returned from malloc() and then see if it  
is passed to sizeof().  This seems possible by adding a new PreVisit  
method for SizeOfAlignOfExpr, and checking if the SymbolicRegion wraps  
a conjured symbol that is associated with a CallExpr for malloc().   
That would make the check more precise, and at least the diagnostic  
would match with the actual code.

What do you think?

Ted

On Nov 8, 2009, at 5:10 AM, Zhongxing Xu wrote:

> Author: zhongxingxu
> Date: Sun Nov  8 07:10:34 2009
> New Revision: 86464
>
> URL: http://llvm.org/viewvc/llvm-project?rev=86464&view=rev
> Log:
> Add a checker for CWE-467: Use of sizeof() on a Pointer Type.
>
> Added:
>    cfe/trunk/lib/Analysis/CheckSizeofPointer.cpp
> Modified:
>    cfe/trunk/include/clang/Analysis/LocalCheckers.h
>    cfe/trunk/include/clang/Frontend/Analyses.def
>    cfe/trunk/lib/Frontend/AnalysisConsumer.cpp
>
> Modified: cfe/trunk/include/clang/Analysis/LocalCheckers.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/LocalCheckers.h?rev=86464&r1=86463&r2=86464&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/include/clang/Analysis/LocalCheckers.h (original)
> +++ cfe/trunk/include/clang/Analysis/LocalCheckers.h Sun Nov  8  
> 07:10:34 2009
> @@ -53,7 +53,7 @@
>
> void CheckSecuritySyntaxOnly(const Decl *D, BugReporter &BR);
>
> -
> +void CheckSizeofPointer(const Decl *D, BugReporter &BR);
> } // end namespace clang
>
> #endif
>
> Modified: cfe/trunk/include/clang/Frontend/Analyses.def
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Analyses.def?rev=86464&r1=86463&r2=86464&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/include/clang/Frontend/Analyses.def (original)
> +++ cfe/trunk/include/clang/Frontend/Analyses.def Sun Nov  8  
> 07:10:34 2009
> @@ -48,6 +48,9 @@
> ANALYSIS(CheckerCFRef, "checker-cfref",
>          "Run the [Core] Foundation reference count checker", Code)
>
> +ANALYSIS(WarnSizeofPointer, "warn-sizeof-pointer",
> +         "Warn about unintended use of sizeof() on pointer  
> expressions", Code)
> +
> ANALYSIS(InlineCall, "inline-call",
>          "Experimental transfer function inling callees when its  
> definition"
>          " is available.", TranslationUnit)
>
> Added: cfe/trunk/lib/Analysis/CheckSizeofPointer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CheckSizeofPointer.cpp?rev=86464&view=auto
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/CheckSizeofPointer.cpp (added)
> +++ cfe/trunk/lib/Analysis/CheckSizeofPointer.cpp Sun Nov  8  
> 07:10:34 2009
> @@ -0,0 +1,58 @@
> +//==- CheckSizeofPointer.cpp - Check for sizeof on pointers ------ 
> *- C++ -*-==//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open  
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +//
> +//  This file defines a check for unintended use of sizeof() on  
> pointer
> +//  expressions.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +
> +#include "clang/Analysis/PathSensitive/BugReporter.h"
> +#include "clang/AST/StmtVisitor.h"
> +#include "clang/Analysis/LocalCheckers.h"
> +#include "llvm/Support/Compiler.h"
> +
> +using namespace clang;
> +
> +namespace {
> +class VISIBILITY_HIDDEN WalkAST : public StmtVisitor<WalkAST> {
> +  BugReporter &BR;
> +
> +public:
> +  WalkAST(BugReporter &br) : BR(br) {}
> +  void VisitSizeOfAlignOfExpr(SizeOfAlignOfExpr *E);
> +  void VisitStmt(Stmt *S) { VisitChildren(S); }
> +  void VisitChildren(Stmt *S);
> +};
> +}
> +
> +void WalkAST::VisitChildren(Stmt *S) {
> +  for (Stmt::child_iterator I = S->child_begin(), E = S->child_end 
> (); I!=E; ++I)
> +    if (Stmt *child = *I)
> +      Visit(child);
> +}
> +
> +// CWE-467: Use of sizeof() on a Pointer Type
> +void WalkAST::VisitSizeOfAlignOfExpr(SizeOfAlignOfExpr *E) {
> +  if (!E->isSizeOf())
> +    return;
> +
> +  QualType T = E->getTypeOfArgument();
> +  if (T->isPointerType()) {
> +    SourceRange R = E->getArgumentExpr()->getSourceRange();
> +    BR.EmitBasicReport("Potential unintended use of sizeof() on  
> pointer type",
> +                       "Logic",
> +                       "The code calls sizeof() on a malloced  
> pointer type, which always returns the wordsize/8. This can produce  
> an unexpected result if the programmer intended to determine how  
> much memory has been allocated.",
> +                       E->getLocStart(), &R, 1);
> +  }
> +}
> +
> +void clang::CheckSizeofPointer(const Decl *D, BugReporter &BR) {
> +  WalkAST walker(BR);
> +  walker.Visit(D->getBody());
> +}
>
> Modified: cfe/trunk/lib/Frontend/AnalysisConsumer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/AnalysisConsumer.cpp?rev=86464&r1=86463&r2=86464&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Frontend/AnalysisConsumer.cpp (original)
> +++ cfe/trunk/lib/Frontend/AnalysisConsumer.cpp Sun Nov  8 07:10:34  
> 2009
> @@ -412,6 +412,11 @@
>   CheckObjCInstMethSignature(cast<ObjCImplementationDecl>(D), BR);
> }
>
> +static void ActionWarnSizeofPointer(AnalysisManager &mgr, Decl *D) {
> +  BugReporter BR(mgr);
> +  CheckSizeofPointer(D, BR);
> +}
> +
> static void ActionInlineCall(AnalysisManager &mgr, Decl *D) {
>   if (!D)
>     return;
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits