From: Ted Kremenek Date: Tue, 16 Feb 2010 08:33:59 +0000 (+0000) Subject: Add simpler checker to check if variables captured by a block are uninitialized. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=94fd0b8c88db9b1cd99457d3cd8cd333341dd39c;p=clang Add simpler checker to check if variables captured by a block are uninitialized. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@96341 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Checker/BugReporter/BugReporter.h b/include/clang/Checker/BugReporter/BugReporter.h index ad32f7c974..6c41668cce 100644 --- a/include/clang/Checker/BugReporter/BugReporter.h +++ b/include/clang/Checker/BugReporter/BugReporter.h @@ -202,7 +202,7 @@ public: ~RangedBugReport(); // FIXME: Move this out of line. - void addRange(SourceRange R) { + void addRange(SourceRange R) { assert(R.isValid()); Ranges.push_back(R); } @@ -464,6 +464,10 @@ const Stmt *GetRetValExpr(const ExplodedNode *N); void registerTrackNullOrUndefValue(BugReporterContext& BRC, const void *stmt, const ExplodedNode* N); +void registerFindLastStore(BugReporterContext& BRC, const void *memregion, + const ExplodedNode *N); + + } // end namespace clang::bugreporter //===----------------------------------------------------------------------===// diff --git a/lib/Checker/BugReporterVisitors.cpp b/lib/Checker/BugReporterVisitors.cpp index b8f657b31b..6cf41b14dc 100644 --- a/lib/Checker/BugReporterVisitors.cpp +++ b/lib/Checker/BugReporterVisitors.cpp @@ -323,7 +323,7 @@ void clang::bugreporter::registerTrackNullOrUndefValue(BugReporterContext& BRC, if (isa(V) || isa(V) || V.isUndef()) { - registerFindLastStore(BRC, R, V); + ::registerFindLastStore(BRC, R, V); } } } @@ -347,3 +347,21 @@ void clang::bugreporter::registerTrackNullOrUndefValue(BugReporterContext& BRC, } } } + +void clang::bugreporter::registerFindLastStore(BugReporterContext& BRC, + const void *data, + const ExplodedNode* N) { + + const MemRegion *R = static_cast(data); + + if (!R) + return; + + const GRState *state = N->getState(); + SVal V = state->getSVal(R); + + if (V.isUnknown()) + return; + + BRC.addVisitor(new FindLastStoreBRVisitor(V, R)); +} diff --git a/lib/Checker/CMakeLists.txt b/lib/Checker/CMakeLists.txt index 2aca818caa..7b21d08dcb 100644 --- a/lib/Checker/CMakeLists.txt +++ b/lib/Checker/CMakeLists.txt @@ -58,6 +58,7 @@ add_clang_library(clangChecker Store.cpp SymbolManager.cpp UndefBranchChecker.cpp + UndefCapturedBlockVarChecker.cpp UndefResultChecker.cpp UndefinedArraySubscriptChecker.cpp UndefinedAssignmentChecker.cpp diff --git a/lib/Checker/GRExprEngine.cpp b/lib/Checker/GRExprEngine.cpp index 7f40d4db1c..7f86319374 100644 --- a/lib/Checker/GRExprEngine.cpp +++ b/lib/Checker/GRExprEngine.cpp @@ -311,6 +311,7 @@ static void RegisterInternalChecks(GRExprEngine &Eng) { RegisterUndefinedArraySubscriptChecker(Eng); RegisterUndefinedAssignmentChecker(Eng); RegisterUndefBranchChecker(Eng); + RegisterUndefCapturedBlockVarChecker(Eng); RegisterUndefResultChecker(Eng); // This is not a checker yet. diff --git a/lib/Checker/GRExprEngineInternalChecks.h b/lib/Checker/GRExprEngineInternalChecks.h index 1246703b86..64a930d504 100644 --- a/lib/Checker/GRExprEngineInternalChecks.h +++ b/lib/Checker/GRExprEngineInternalChecks.h @@ -36,8 +36,8 @@ void RegisterArrayBoundChecker(GRExprEngine &Eng); void RegisterUndefinedArraySubscriptChecker(GRExprEngine &Eng); void RegisterUndefinedAssignmentChecker(GRExprEngine &Eng); void RegisterUndefBranchChecker(GRExprEngine &Eng); +void RegisterUndefCapturedBlockVarChecker(GRExprEngine &Eng); void RegisterUndefResultChecker(GRExprEngine &Eng); - void RegisterNoReturnFunctionChecker(GRExprEngine &Eng); void RegisterBuiltinFunctionChecker(GRExprEngine &Eng); void RegisterOSAtomicChecker(GRExprEngine &Eng); diff --git a/lib/Checker/UndefCapturedBlockVarChecker.cpp b/lib/Checker/UndefCapturedBlockVarChecker.cpp new file mode 100644 index 0000000000..a8d7284b40 --- /dev/null +++ b/lib/Checker/UndefCapturedBlockVarChecker.cpp @@ -0,0 +1,101 @@ +// UndefCapturedBlockVarChecker.cpp - Uninitialized captured vars -*- C++ -*-=// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This checker detects blocks that capture uninitialized values. +// +//===----------------------------------------------------------------------===// + +#include "GRExprEngineInternalChecks.h" +#include "clang/Checker/PathSensitive/CheckerVisitor.h" +#include "clang/Checker/PathSensitive/GRExprEngine.h" +#include "clang/Checker/BugReporter/BugReporter.h" +#include "llvm/Support/raw_ostream.h" + +using namespace clang; + +namespace { +class UndefCapturedBlockVarChecker + : public CheckerVisitor { + BugType *BT; + +public: + UndefCapturedBlockVarChecker() : BT(0) {} + static void *getTag() { static int tag = 0; return &tag; } + void PostVisitBlockExpr(CheckerContext &C, const BlockExpr *BE); +}; +} // end anonymous namespace + +void clang::RegisterUndefCapturedBlockVarChecker(GRExprEngine &Eng) { + Eng.registerCheck(new UndefCapturedBlockVarChecker()); +} + +static const BlockDeclRefExpr *FindBlockDeclRefExpr(const Stmt *S, + const VarDecl *VD){ + if (const BlockDeclRefExpr *BR = dyn_cast(S)) + if (BR->getDecl() == VD) + return BR; + + for (Stmt::const_child_iterator I = S->child_begin(), E = S->child_end(); + I!=E; ++I) + if (const Stmt *child = *I) { + const BlockDeclRefExpr *BR = FindBlockDeclRefExpr(child, VD); + if (BR) + return BR; + } + + return NULL; +} + +void +UndefCapturedBlockVarChecker::PostVisitBlockExpr(CheckerContext &C, + const BlockExpr *BE) { + if (!BE->hasBlockDeclRefExprs()) + return; + + const GRState *state = C.getState(); + const BlockDataRegion *R = + cast(state->getSVal(BE).getAsRegion()); + + BlockDataRegion::referenced_vars_iterator I = R->referenced_vars_begin(), + E = R->referenced_vars_end(); + + for (; I != E; ++I) { + // This VarRegion is the region associated with the block; we need + // the one associated with the encompassing context. + const VarRegion *VR = *I; + const VarDecl *VD = VR->getDecl(); + + if (VD->getAttr() || !VD->hasLocalStorage()) + continue; + + // Get the VarRegion associated with VD in the local stack frame. + const LocationContext *LC = C.getPredecessor()->getLocationContext(); + VR = C.getValueManager().getRegionManager().getVarRegion(VD, LC); + + if (state->getSVal(VR).isUndef()) + if (ExplodedNode *N = C.GenerateSink()) { + if (!BT) + BT = new BuiltinBug("Captured block variable is uninitialized"); + + // Generate a bug report. + llvm::SmallString<128> buf; + llvm::raw_svector_ostream os(buf); + + os << "Variable '" << VD->getName() << "' is captured by block with " + "a garbage value"; + + EnhancedBugReport *R = new EnhancedBugReport(*BT, os.str(), N); + if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD)) + R->addRange(Ex->getSourceRange()); + R->addVisitorCreator(bugreporter::registerFindLastStore, VR); + // need location of block + C.EmitReport(R); + } + } +} diff --git a/test/Analysis/blocks.m b/test/Analysis/blocks.m index 029aefcb86..e8e96a22cf 100644 --- a/test/Analysis/blocks.m +++ b/test/Analysis/blocks.m @@ -67,3 +67,19 @@ void test1(NSString *format, ...) { __builtin_va_end(args); } + +// test2 - Test that captured variables that are uninitialized are flagged +// as such. +void test2() { + static int y = 0; + int x; + ^{ y = x + 1; }(); // expected-warning{{Variable 'x' is captured by block with a garbage value}} +} + +void test2_b() { + static int y = 0; + __block int x; + // This is also a bug, but should be found not by checking the value + // 'x' is bound at block creation. + ^{ y = x + 1; }(); // no-warning +}