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

[Zhongxing Xu]

I later modified the diagnostic.  I admit this could produce a lot of
false positives. But I think checking for malloced pointer does not
make much more sense than checking all pointers. This version is very
fast, and is triggered by a separate command line option, so it can be
treated like a very light weight check, similar to a syntactic check.
Is this reasonable?

2009/11/10 Ted Kremenek <kremenek at apple.com>:
> 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
>
>