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 <[email protected]>: > 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 >> [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
