[cfe-commits] r146144 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp test/Analysis/malloc-sizeof.c

Ted Kremenek kremenek at apple.com
Thu Dec 8 08:54:42 PST 2011


Hi Peter,

This looks very well written.  Have you run it over a reasonable body of code to see if it finds bugs in practice or what kind of false positives it may trigger?

Thanks for doing this,
Ted

On Dec 8, 2011, at 12:31 AM, Peter Collingbourne wrote:

> Author: pcc
> Date: Thu Dec  8 02:31:14 2011
> New Revision: 146144
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=146144&view=rev
> Log:
> Add an experimental MallocSizeofChecker, which reports inconsistencies
> between the casted type of the return value of a malloc/calloc/realloc
> call and the operand of any sizeof expressions contained within
> its argument(s).
> 
> Added:
>    cfe/trunk/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
>    cfe/trunk/test/Analysis/malloc-sizeof.c
> Modified:
>    cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
>    cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=146144&r1=146143&r2=146144&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Thu Dec  8 02:31:14 2011
> @@ -37,6 +37,7 @@
>   MacOSXAPIChecker.cpp
>   MallocChecker.cpp
>   MallocOverflowSecurityChecker.cpp
> +  MallocSizeofChecker.cpp
>   NSAutoreleasePoolChecker.cpp
>   NSErrorChecker.cpp
>   NoReturnFunctionChecker.cpp
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=146144&r1=146143&r2=146144&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Thu Dec  8 02:31:14 2011
> @@ -256,6 +256,10 @@
>   HelpText<"Check for potential memory leaks, double free, and use-after-free problems">,
>   DescFile<"MallocChecker.cpp">;
> 
> +def MallocSizeofChecker : Checker<"MallocSizeof">,
> +  HelpText<"Check for dubious malloc arguments involving sizeof">,
> +  DescFile<"MallocSizeofChecker.cpp">;
> +
> def PthreadLockChecker : Checker<"PthreadLock">,
>   HelpText<"Simple lock -> unlock checker">,
>   DescFile<"PthreadLockChecker.cpp">;
> 
> Added: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp?rev=146144&view=auto
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp (added)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp Thu Dec  8 02:31:14 2011
> @@ -0,0 +1,208 @@
> +// MallocSizeofChecker.cpp - Check for dubious malloc arguments ---*- C++ -*-=//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// Reports inconsistencies between the casted type of the return value of a
> +// malloc/calloc/realloc call and the operand of any sizeof expressions
> +// contained within its argument(s).
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "ClangSACheckers.h"
> +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
> +#include "clang/StaticAnalyzer/Core/Checker.h"
> +#include "clang/StaticAnalyzer/Core/CheckerManager.h"
> +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
> +#include "clang/AST/StmtVisitor.h"
> +#include "clang/AST/TypeLoc.h"
> +
> +using namespace clang;
> +using namespace ento;
> +
> +namespace {
> +
> +typedef std::pair<const TypeSourceInfo *, const CallExpr *> TypeCallPair;
> +typedef llvm::PointerUnion<const Stmt *, const VarDecl *> ExprParent;
> +
> +class CastedAllocFinder
> +  : public ConstStmtVisitor<CastedAllocFinder, TypeCallPair> {
> +  IdentifierInfo *II_malloc, *II_calloc, *II_realloc;
> +
> +public:
> +  struct CallRecord {
> +    ExprParent CastedExprParent;
> +    const Expr *CastedExpr;
> +    const TypeSourceInfo *ExplicitCastType;
> +    const CallExpr *AllocCall;
> +
> +    CallRecord(ExprParent CastedExprParent, const Expr *CastedExpr,
> +               const TypeSourceInfo *ExplicitCastType,
> +               const CallExpr *AllocCall)
> +      : CastedExprParent(CastedExprParent), CastedExpr(CastedExpr),
> +        ExplicitCastType(ExplicitCastType), AllocCall(AllocCall) {}
> +  };
> +
> +  typedef std::vector<CallRecord> CallVec;
> +  CallVec Calls;
> +
> +  CastedAllocFinder(ASTContext *Ctx) :
> +    II_malloc(&Ctx->Idents.get("malloc")),
> +    II_calloc(&Ctx->Idents.get("calloc")),
> +    II_realloc(&Ctx->Idents.get("realloc")) {}
> +
> +  void VisitChild(ExprParent Parent, const Stmt *S) {
> +    TypeCallPair AllocCall = Visit(S);
> +    if (AllocCall.second && AllocCall.second != S)
> +      Calls.push_back(CallRecord(Parent, cast<Expr>(S), AllocCall.first,
> +                                 AllocCall.second));
> +  }
> +
> +  void VisitChildren(const Stmt *S) {
> +    for (Stmt::const_child_iterator I = S->child_begin(), E = S->child_end();
> +         I!=E; ++I)
> +      if (const Stmt *child = *I)
> +        VisitChild(S, child);
> +  }
> +
> +  TypeCallPair VisitCastExpr(const CastExpr *E) {
> +    return Visit(E->getSubExpr());
> +  }
> +
> +  TypeCallPair VisitExplicitCastExpr(const ExplicitCastExpr *E) {
> +    return TypeCallPair(E->getTypeInfoAsWritten(),
> +                        Visit(E->getSubExpr()).second);
> +  }
> +
> +  TypeCallPair VisitParenExpr(const ParenExpr *E) {
> +    return Visit(E->getSubExpr());
> +  }
> +
> +  TypeCallPair VisitStmt(const Stmt *S) {
> +    VisitChildren(S);
> +    return TypeCallPair();
> +  }
> +
> +  TypeCallPair VisitCallExpr(const CallExpr *E) {
> +    VisitChildren(E);
> +    const FunctionDecl *FD = E->getDirectCallee();
> +    if (FD) {
> +      IdentifierInfo *II = FD->getIdentifier();
> +      if (II == II_malloc || II == II_calloc || II == II_realloc)
> +        return TypeCallPair(0, E);
> +    }
> +    return TypeCallPair();
> +  }
> +
> +  TypeCallPair VisitDeclStmt(const DeclStmt *S) {
> +    for (DeclStmt::const_decl_iterator I = S->decl_begin(), E = S->decl_end();
> +         I!=E; ++I)
> +      if (const VarDecl *VD = dyn_cast<VarDecl>(*I))
> +        if (const Expr *Init = VD->getInit())
> +          VisitChild(VD, Init);
> +    return TypeCallPair();
> +  }
> +};
> +
> +class SizeofFinder : public ConstStmtVisitor<SizeofFinder> {
> +public:
> +  std::vector<const UnaryExprOrTypeTraitExpr *> Sizeofs;
> +
> +  void VisitBinMul(const BinaryOperator *E) {
> +    Visit(E->getLHS());
> +    Visit(E->getRHS());
> +  }
> +
> +  void VisitBinAdd(const BinaryOperator *E) {
> +    Visit(E->getLHS());
> +    Visit(E->getRHS());
> +  }
> +
> +  void VisitImplicitCastExpr(const ImplicitCastExpr *E) {
> +    return Visit(E->getSubExpr());
> +  }
> +
> +  void VisitParenExpr(const ParenExpr *E) {
> +    return Visit(E->getSubExpr());
> +  }
> +
> +  void VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E) {
> +    if (E->getKind() != UETT_SizeOf)
> +      return;
> +
> +    Sizeofs.push_back(E);
> +  }
> +};
> +
> +class MallocSizeofChecker : public Checker<check::ASTCodeBody> {
> +public:
> +  void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
> +                        BugReporter &BR) const {
> +    AnalysisDeclContext *ADC = mgr.getAnalysisDeclContext(D);
> +    CastedAllocFinder Finder(&BR.getContext());
> +    Finder.Visit(D->getBody());
> +    for (CastedAllocFinder::CallVec::iterator i = Finder.Calls.begin(),
> +         e = Finder.Calls.end(); i != e; ++i) {
> +      QualType CastedType = i->CastedExpr->getType();
> +      if (!CastedType->isPointerType())
> +        continue;
> +      QualType PointeeType = CastedType->getAs<PointerType>()->getPointeeType();
> +      if (PointeeType->isVoidType())
> +        continue;
> +
> +      for (CallExpr::const_arg_iterator ai = i->AllocCall->arg_begin(),
> +           ae = i->AllocCall->arg_end(); ai != ae; ++ai) {
> +        if (!(*ai)->getType()->isIntegerType())
> +          continue;
> +
> +        SizeofFinder SFinder;
> +        SFinder.Visit(*ai);
> +        if (SFinder.Sizeofs.size() != 1)
> +          continue;
> +
> +        QualType SizeofType = SFinder.Sizeofs[0]->getTypeOfArgument();
> +        if (!BR.getContext().hasSameUnqualifiedType(PointeeType, SizeofType)) {
> +          const TypeSourceInfo *TSI = 0;
> +          if (i->CastedExprParent.is<const VarDecl *>()) {
> +            TSI =
> +              i->CastedExprParent.get<const VarDecl *>()->getTypeSourceInfo();
> +          } else {
> +            TSI = i->ExplicitCastType;
> +          }
> +
> +          llvm::SmallString<64> buf;
> +          llvm::raw_svector_ostream OS(buf);
> +
> +          OS << "Result of '"
> +             << i->AllocCall->getDirectCallee()->getIdentifier()->getName()
> +             << "' is converted to type '"
> +             << CastedType.getAsString() << "', whose pointee type '"
> +             << PointeeType.getAsString() << "' is incompatible with "
> +             << "sizeof operand type '" << SizeofType.getAsString() << "'";
> +          llvm::SmallVector<SourceRange, 4> Ranges;
> +          Ranges.push_back(i->AllocCall->getCallee()->getSourceRange());
> +          Ranges.push_back(SFinder.Sizeofs[0]->getSourceRange());
> +          if (TSI)
> +            Ranges.push_back(TSI->getTypeLoc().getSourceRange());
> +
> +          PathDiagnosticLocation L =
> +            PathDiagnosticLocation::createBegin(i->AllocCall->getCallee(),
> +                                                BR.getSourceManager(), ADC);
> +
> +          BR.EmitBasicReport("allocator sizeof operand mismatch", OS.str(), L,
> +                             Ranges.data(), Ranges.size());
> +        }
> +      }
> +    }
> +  }
> +};
> +
> +}
> +
> +void ento::registerMallocSizeofChecker(CheckerManager &mgr) {
> +  mgr.registerChecker<MallocSizeofChecker>();
> +}
> 
> Added: cfe/trunk/test/Analysis/malloc-sizeof.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc-sizeof.c?rev=146144&view=auto
> ==============================================================================
> --- cfe/trunk/test/Analysis/malloc-sizeof.c (added)
> +++ cfe/trunk/test/Analysis/malloc-sizeof.c Thu Dec  8 02:31:14 2011
> @@ -0,0 +1,27 @@
> +// RUN: %clang_cc1 -analyze -analyzer-checker=experimental.unix.MallocSizeof -verify %s
> +
> +#include <stddef.h>
> +
> +void *malloc(size_t size);
> +void *calloc(size_t nmemb, size_t size);
> +void *realloc(void *ptr, size_t size);
> +
> +struct A {};
> +struct B {};
> +
> +void foo() {
> +  int *ip1 = malloc(sizeof(1));
> +  int *ip2 = malloc(4 * sizeof(int));
> +
> +  long *lp1 = malloc(sizeof(short)); // expected-warning {{Result of 'malloc' is converted to type 'long *', whose pointee type 'long' is incompatible with sizeof operand type 'short'}}
> +  long *lp2 = malloc(5 * sizeof(double)); // expected-warning {{Result of 'malloc' is converted to type 'long *', whose pointee type 'long' is incompatible with sizeof operand type 'double'}}
> +  long *lp3 = malloc(5 * sizeof(char) + 2); // expected-warning {{Result of 'malloc' is converted to type 'long *', whose pointee type 'long' is incompatible with sizeof operand type 'char'}}
> +
> +  struct A *ap1 = calloc(1, sizeof(struct A));
> +  struct A *ap2 = calloc(2, sizeof(*ap1));
> +  struct A *ap3 = calloc(2, sizeof(ap1)); // expected-warning {{Result of 'calloc' is converted to type 'struct A *', whose pointee type 'struct A' is incompatible with sizeof operand type 'struct A *'}}
> +  struct A *ap4 = calloc(3, sizeof(struct A*)); // expected-warning {{Result of 'calloc' is converted to type 'struct A *', whose pointee type 'struct A' is incompatible with sizeof operand type 'struct A *'}}
> +  struct A *ap5 = calloc(4, sizeof(struct B)); // expected-warning {{Result of 'calloc' is converted to type 'struct A *', whose pointee type 'struct A' is incompatible with sizeof operand type 'struct B'}}
> +  struct A *ap6 = realloc(ap5, sizeof(struct A));
> +  struct A *ap7 = realloc(ap5, sizeof(struct B)); // expected-warning {{Result of 'realloc' is converted to type 'struct A *', whose pointee type 'struct A' is incompatible with sizeof operand type 'struct B'}}
> +}
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list