From: Anna Zaks Date: Tue, 7 Feb 2012 00:56:14 +0000 (+0000) Subject: [analyzer] Allow each CString check to be enabled/disabled X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=57300760964904cc022a175643342f29f46b7e6b;p=clang [analyzer] Allow each CString check to be enabled/disabled separately. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@149947 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h index 0d8a80abd5..dca18926fb 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -22,7 +22,13 @@ namespace ento { class CheckerContext { ExprEngine &Eng; + /// The current exploded(symbolic execution) graph node. ExplodedNode *Pred; + /// The flag is true if the (state of the execution) has been modified + /// by the checker using this context. For example, a new transition has been + /// added or a bug report issued. + bool Changed; + /// The tagged location, which is used to generate all new nodes. const ProgramPoint Location; NodeBuilder &NB; @@ -33,6 +39,7 @@ public: const ProgramPoint &loc) : Eng(eng), Pred(pred), + Changed(false), Location(loc), NB(builder) { assert(Pred->getState() && @@ -57,6 +64,10 @@ public: ExplodedNode *getPredecessor() { return Pred; } ProgramStateRef getState() const { return Pred->getState(); } + /// \brief Check if the checker changed the state of the execution; ex: added + /// a new transition or a bug report. + bool isDifferent() { return Changed; } + /// \brief Returns the number of times the current block has been visited /// along the analyzed path. unsigned getCurrentBlockCount() const { @@ -146,6 +157,7 @@ public: /// \brief Emit the diagnostics report. void EmitReport(BugReport *R) { + Changed = true; Eng.getBugReporter().EmitReport(R); } @@ -187,6 +199,7 @@ private: if (State == Pred->getState() && !Tag && !MarkAsSink) return Pred; + Changed = true; ExplodedNode *node = NB.generateNode(Tag ? Location.withTag(Tag) : Location, State, P ? P : Pred, MarkAsSink); @@ -194,6 +207,14 @@ private: } }; +/// \brief A helper class which wraps a boolean value set to false by default. +struct DefaultBool { + bool Val; + DefaultBool() : Val(false) {} + operator bool() const { return Val; } + DefaultBool &operator=(bool b) { Val = b; return *this; } +}; + } // end GR namespace } // end clang namespace diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 7518b96c63..748fc43a67 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -32,12 +32,26 @@ class CStringChecker : public Checker< eval::Call, check::DeadSymbols, check::RegionChanges > { - mutable OwningPtr BT_Null, BT_Bounds, - BT_Overlap, BT_NotCString, - BT_AdditionOverflow; + mutable OwningPtr BT_Null, + BT_Bounds, + BT_Overlap, + BT_NotCString, + BT_AdditionOverflow; + mutable const char *CurrentFunctionDescription; public: + /// The filter is used to filter out the diagnostics which are not enabled by + /// the user. + struct CStringChecksFilter { + DefaultBool CheckCStringNullArg; + DefaultBool CheckCStringOutOfBounds; + DefaultBool CheckCStringBufferOverlap; + DefaultBool CheckCStringNotNullTerm; + }; + + CStringChecksFilter Filter; + static void *getTag() { static int tag; return &tag; } bool evalCall(const CallExpr *CE, CheckerContext &C) const; @@ -215,12 +229,15 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, llvm::tie(stateNull, stateNonNull) = assumeZero(C, state, l, S->getType()); if (stateNull && !stateNonNull) { + if (!Filter.CheckCStringNullArg) + return NULL; + ExplodedNode *N = C.generateSink(stateNull); if (!N) return NULL; if (!BT_Null) - BT_Null.reset(new BuiltinBug("API", + BT_Null.reset(new BuiltinBug("Unix API", "Null pointer argument in call to byte string function")); SmallString<80> buf; @@ -342,6 +359,10 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C, if (!state) return NULL; + // If out-of-bounds checking is turned off, skip the rest. + if (!Filter.CheckCStringOutOfBounds) + return state; + // Get the access length and make sure it is known. // FIXME: This assumes the caller has already checked that the access length // is positive. And that it's unsigned. @@ -395,6 +416,9 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, const Expr *Size, const Expr *First, const Expr *Second) const { + if (!Filter.CheckCStringBufferOverlap) + return state; + // Do a simple check for overlap: if the two arguments are from the same // buffer, see if the end of the first is greater than the start of the second // or vice versa. @@ -525,6 +549,10 @@ ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C, ProgramStateRef state, NonLoc left, NonLoc right) const { + // If out-of-bounds checking is turned off, skip the rest. + if (!Filter.CheckCStringOutOfBounds) + return state; + // If a previous check has failed, propagate the failure. if (!state) return NULL; @@ -664,9 +692,12 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, // C string. In the context of locations, the only time we can issue such // a warning is for labels. if (loc::GotoLabel *Label = dyn_cast(&Buf)) { + if (!Filter.CheckCStringNotNullTerm) + return UndefinedVal(); + if (ExplodedNode *N = C.addTransition(state)) { if (!BT_NotCString) - BT_NotCString.reset(new BuiltinBug("API", + BT_NotCString.reset(new BuiltinBug("Unix API", "Argument is not a null-terminated string.")); SmallString<120> buf; @@ -683,8 +714,8 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, report->addRange(Ex->getSourceRange()); C.EmitReport(report); } - return UndefinedVal(); + } // If it's not a region and not a label, give up. @@ -721,9 +752,12 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, // Other regions (mostly non-data) can't have a reliable C string length. // In this case, an error is emitted and UndefinedVal is returned. // The caller should always be prepared to handle this case. + if (!Filter.CheckCStringNotNullTerm) + return UndefinedVal(); + if (ExplodedNode *N = C.addTransition(state)) { if (!BT_NotCString) - BT_NotCString.reset(new BuiltinBug("API", + BT_NotCString.reset(new BuiltinBug("Unix API", "Argument is not a null-terminated string.")); SmallString<120> buf; @@ -1715,6 +1749,16 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { // Check and evaluate the call. (this->*evalFunction)(C, CE); + + // If the evaluate call resulted in no change, chain to the next eval call + // handler. + // Note, the custom CString evaluation calls assume that basic safety + // properties are held. However, if the user chooses to turn off some of these + // checks, we ignore the issues and leave the call evaluation to a generic + // handler. + if (!C.isDifferent()) + return false; + return true; } @@ -1850,6 +1894,15 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR, C.addTransition(state); } -void ento::registerCStringChecker(CheckerManager &mgr) { - mgr.registerChecker(); +#define REGISTER_CHECKER(name) \ +void ento::register##name(CheckerManager &mgr) {\ + static CStringChecker *TheChecker = 0; \ + if (TheChecker == 0) \ + TheChecker = mgr.registerChecker(); \ + TheChecker->Filter.Check##name = true; \ } + +REGISTER_CHECKER(CStringNullArg) +REGISTER_CHECKER(CStringOutOfBounds) +REGISTER_CHECKER(CStringBufferOverlap) +REGISTER_CHECKER(CStringNotNullTerm) diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index d08739e2a3..24a79485c8 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -302,8 +302,20 @@ def StreamChecker : Checker<"Stream">, let ParentPackage = CString in { -def CStringChecker : Checker<"Generic">, - HelpText<"Check calls to functions in ">, +def CStringNullArg : Checker<"NullArg">, + HelpText<"Check for null pointers being passed as arguments to C string functions">, + DescFile<"CStringChecker.cpp">; + +def CStringOutOfBounds : Checker<"OutOfBounds">, + HelpText<"Check for out-of-bounds access in string functions">, + DescFile<"CStringChecker.cpp">; + +def CStringBufferOverlap : Checker<"BufferOverlap">, + HelpText<"Checks for overlap in two buffer arguments">, + DescFile<"CStringChecker.cpp">; + +def CStringNotNullTerm : Checker<"NotNullTerminated">, + HelpText<"Check for arguments which are not null-terminating strings">, DescFile<"CStringChecker.cpp">; def CStringSyntaxChecker : Checker<"BadSizeArg">, @@ -408,7 +420,7 @@ def ObjCContainersASTChecker : Checker<"PointerSizedValues">, DescFile<"ObjCContainersASTChecker.cpp">; def ObjCContainersChecker : Checker<"OutOfBounds">, - HelpText<"Checks for index out of bounds when using 'CFArray' API">, + HelpText<"Checks for index out-of-bounds when using 'CFArray' API">, DescFile<"ObjCContainersChecker.cpp">; } diff --git a/test/Analysis/bstring.c b/test/Analysis/bstring.c index 8cdbb18b35..f847a220f9 100644 --- a/test/Analysis/bstring.c +++ b/test/Analysis/bstring.c @@ -1,7 +1,7 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.unix.cstring.Generic -analyzer-store=region -Wno-null-dereference -verify %s -// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core,experimental.unix.cstring.Generic -analyzer-store=region -Wno-null-dereference -verify %s -// RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core,experimental.unix.cstring.Generic -analyzer-store=region -Wno-null-dereference -verify %s -// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,experimental.unix.cstring.Generic -analyzer-store=region -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.unix.cstring -analyzer-store=region -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core,experimental.unix.cstring -analyzer-store=region -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core,experimental.unix.cstring -analyzer-store=region -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,experimental.unix.cstring.NullArg,experimental.unix.cstring.OutOfBounds,experimental.unix.cstring.BufferOverlap,experimental.unix.cstring.NotNullTerminated -analyzer-store=region -Wno-null-dereference -verify %s //===----------------------------------------------------------------------=== // Declarations diff --git a/test/Analysis/string.c b/test/Analysis/string.c index 9b0dea3d6c..43da6d70fc 100644 --- a/test/Analysis/string.c +++ b/test/Analysis/string.c @@ -1,7 +1,7 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.unix.cstring.Generic,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s -// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core,experimental.unix.cstring.Generic,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s -// RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core,experimental.unix.cstring.Generic,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s -// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=experimental.security.taint,core,experimental.unix.cstring.Generic,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.unix.cstring,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -analyzer-checker=core,experimental.unix.cstring,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -DVARIANT -analyzer-checker=core,experimental.unix.cstring,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=experimental.security.taint,core,experimental.unix.cstring,experimental.deadcode.UnreachableCode -analyzer-store=region -Wno-null-dereference -verify %s //===----------------------------------------------------------------------=== // Declarations