xazax.hun updated this revision to Diff 34285. xazax.hun added a comment. - Updated to newest trunk. - Moved the feature behind an option. - Fixed a crash when an operator() of a lambda is analyzed as a top level function, and a ThisExpr is referring to the this in the enclosing scope (this can only happen when lambda support is turned off). - Added a new test case for nested lambdas capturing 'this'.
http://reviews.llvm.org/D12652 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/MemRegion.cpp test/Analysis/dead-stores.cpp test/Analysis/lambdas.cpp test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp =================================================================== --- test/Analysis/temporaries.cpp +++ test/Analysis/temporaries.cpp @@ -299,13 +299,7 @@ void testRecursiveFramesStart() { testRecursiveFrames(false); } void testLambdas() { - // This is the test we would like to write: - // []() { check(NoReturnDtor()); } != nullptr || check(Dtor()); - // But currently the analyzer stops when it encounters a lambda: - [] {}; - // The CFG for this now looks correct, but we still do not reach the line - // below. - clang_analyzer_warnIfReached(); // FIXME: Should warn. + []() { check(NoReturnDtor()); } != nullptr || check(Dtor()); } void testGnuExpressionStatements(int v) { Index: test/Analysis/lambdas.cpp =================================================================== --- test/Analysis/lambdas.cpp +++ test/Analysis/lambdas.cpp @@ -1,9 +1,167 @@ -// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=debug.DumpCFG %s > %t 2>&1 +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.DumpCFG -analyzer-config inline-lambdas=true %s > %t 2>&1 // RUN: FileCheck --input-file=%t %s +void clang_analyzer_warnIfReached(); +void clang_analyzer_eval(int); + struct X { X(const X&); }; void f(X x) { (void) [x]{}; } + +// Lambda semantics tests. + +void basicCapture() { + int i = 5; + [i]() mutable { + // clang_analyzer_eval does nothing in inlined functions. + if (i != 5) + clang_analyzer_warnIfReached(); + ++i; + }(); + [&i] { + if (i != 5) + clang_analyzer_warnIfReached(); + }(); + [&i] { + if (i != 5) + clang_analyzer_warnIfReached(); + i++; + }(); + clang_analyzer_eval(i == 6); // expected-warning{{TRUE}} +} + +void deferredLambdaCall() { + int i = 5; + auto l1 = [i]() mutable { + if (i != 5) + clang_analyzer_warnIfReached(); + ++i; + }; + auto l2 = [&i] { + if (i != 5) + clang_analyzer_warnIfReached(); + }; + auto l3 = [&i] { + if (i != 5) + clang_analyzer_warnIfReached(); + i++; + }; + l1(); + l2(); + l3(); + clang_analyzer_eval(i == 6); // expected-warning{{TRUE}} +} + +void multipleCaptures() { + int i = 5, j = 5; + [i, &j]() mutable { + if (i != 5 && j != 5) + clang_analyzer_warnIfReached(); + ++i; + ++j; + }(); + clang_analyzer_eval(i == 5); // expected-warning{{TRUE}} + clang_analyzer_eval(j == 6); // expected-warning{{TRUE}} + [=]() mutable { + if (i != 5 && j != 6) + clang_analyzer_warnIfReached(); + ++i; + ++j; + }(); + clang_analyzer_eval(i == 5); // expected-warning{{TRUE}} + clang_analyzer_eval(j == 6); // expected-warning{{TRUE}} + [&]() mutable { + if (i != 5 && j != 6) + clang_analyzer_warnIfReached(); + ++i; + ++j; + }(); + clang_analyzer_eval(i == 6); // expected-warning{{TRUE}} + clang_analyzer_eval(j == 7); // expected-warning{{TRUE}} +} + +void testReturnValue() { + int i = 5; + auto l = [i] (int a) { + return i + a; + }; + int b = l(3); + clang_analyzer_eval(b == 8); // expected-warning{{TRUE}} +} + +// Nested lambdas. + +void testNestedLambdas() { + int i = 5; + auto l = [i]() mutable { + [&i]() { + ++i; + }(); + if (i != 6) + clang_analyzer_warnIfReached(); + }; + l(); + clang_analyzer_eval(i == 5); // expected-warning{{TRUE}} +} + +// Captured this. + +class RandomClass { + int i; + + void captureFields() { + i = 5; + [this]() { + // clang_analyzer_eval does nothing in inlined functions. + if (i != 5) + clang_analyzer_warnIfReached(); + ++i; + }(); + clang_analyzer_eval(i == 6); // expected-warning{{TRUE}} + } +}; + + +// Nested this capture. + +class RandomClass2 { + int i; + + void captureFields() { + i = 5; + [this]() { + // clang_analyzer_eval does nothing in inlined functions. + if (i != 5) + clang_analyzer_warnIfReached(); + ++i; + [this]() { + // clang_analyzer_eval does nothing in inlined functions. + if (i != 6) + clang_analyzer_warnIfReached(); + ++i; + }(); + }(); + clang_analyzer_eval(i == 7); // expected-warning{{TRUE}} + } +}; + + +// Captured function pointers. + +void inc(int &x) { + ++x; +} + +void testFunctionPointerCapture() { + void (*func)(int &) = inc; + int i = 5; + [&i, func] { + func(i); + }(); + clang_analyzer_eval(i == 6); // expected-warning{{TRUE}} +} + // CHECK: [B2 (ENTRY)] // CHECK: Succs (1): B1 // CHECK: [B1] Index: test/Analysis/dead-stores.cpp =================================================================== --- test/Analysis/dead-stores.cpp +++ test/Analysis/dead-stores.cpp @@ -174,3 +174,17 @@ return radar13213575_testit<true>(5) + radar13213575_testit<false>(3); } +//===----------------------------------------------------------------------===// +// Dead store checking involving lambdas. +//===----------------------------------------------------------------------===// + +int basicLambda(int i, int j) { + i = 5; // no warning + j = 6; // no warning + [i] { (void)i; }(); + [&j] { (void)j; }(); + i = 2; + j = 3; + return i + j; +} + Index: lib/StaticAnalyzer/Core/MemRegion.cpp =================================================================== --- lib/StaticAnalyzer/Core/MemRegion.cpp +++ lib/StaticAnalyzer/Core/MemRegion.cpp @@ -737,7 +737,8 @@ static llvm::PointerUnion<const StackFrameContext *, const VarRegion *> getStackOrCaptureRegionForDeclContext(const LocationContext *LC, const DeclContext *DC, - const VarDecl *VD) { + const VarDecl *VD, + MemRegionManager *Mmgr) { while (LC) { if (const StackFrameContext *SFC = dyn_cast<StackFrameContext>(LC)) { if (cast<DeclContext>(SFC->getDecl()) == DC) @@ -795,7 +796,7 @@ // 'D' to the proper LocationContext. const DeclContext *DC = D->getDeclContext(); llvm::PointerUnion<const StackFrameContext *, const VarRegion *> V = - getStackOrCaptureRegionForDeclContext(LC, DC, D); + getStackOrCaptureRegionForDeclContext(LC, DC, D, this); if (V.is<const VarRegion*>()) return V.get<const VarRegion*>(); @@ -1013,10 +1014,21 @@ const CXXThisRegion* MemRegionManager::getCXXThisRegion(QualType thisPointerTy, const LocationContext *LC) { - const StackFrameContext *STC = LC->getCurrentStackFrame(); - assert(STC); const PointerType *PT = thisPointerTy->getAs<PointerType>(); assert(PT); + // Inside the body of the operator() of a lambda a this expr might refer to an + // object in one of the parent location contexts. + const auto *D = dyn_cast<CXXMethodDecl>(LC->getDecl()); + // FIXME: when operator() of lambda is analyzed as a top level function and + // 'this' refers to a this to the enclosing scope, there is no right region to + // return. + while (!LC->inTopFrame() && + PT != D->getThisType(getContext())->getAs<PointerType>()) { + LC = LC->getParent(); + D = dyn_cast<CXXMethodDecl>(LC->getDecl()); + } + const StackFrameContext *STC = LC->getCurrentStackFrame(); + assert(STC); return getSubRegion<CXXThisRegion>(PT, getStackArgumentsRegion(STC)); } Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -513,3 +513,41 @@ SVal V = state->getSVal(loc::MemRegionVal(R)); Bldr.generateNode(TE, Pred, state->BindExpr(TE, LCtx, V)); } + +void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred, + ExplodedNodeSet &Dst) { + const LocationContext *LocCtxt = Pred->getLocationContext(); + + // Get the region of the lambda itself. + const MemRegion *R = svalBuilder.getRegionManager().getCXXTempObjectRegion( + LE, LocCtxt); + SVal V = loc::MemRegionVal(R); + + ProgramStateRef State = Pred->getState(); + + // If we created a new MemRegion for the lambda, we should explicitly bind + // the captures. + CXXRecordDecl::field_iterator CurField = LE->getLambdaClass()->field_begin(); + for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(), + e = LE->capture_init_end(); + i != e; ++i, ++CurField) { + SVal Field = State->getLValue(*CurField, V); + SVal InitExpr = State->getSVal(*i, LocCtxt); + State = State->bindLoc(Field, InitExpr); + } + + // Decay the Loc into an RValue, because there might be a + // MaterializeTemporaryExpr node above this one which expects the bound value + // to be an RValue. + SVal LambdaRVal = State->getSVal(R); + + ExplodedNodeSet Tmp; + StmtNodeBuilder Bldr(Pred, Tmp, *currBldrCtx); + // FIXME: is this the right program point kind? + Bldr.generateNode(LE, Pred, + State->BindExpr(LE, LocCtxt, LambdaRVal), + nullptr, ProgramPoint::PostLValueKind); + + // FIXME: Move all post/pre visits to ::Visit(). + getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this); +} Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -769,7 +769,6 @@ case Stmt::SEHTryStmtClass: case Stmt::SEHExceptStmtClass: case Stmt::SEHLeaveStmtClass: - case Stmt::LambdaExprClass: case Stmt::SEHFinallyStmtClass: { const ExplodedNode *node = Bldr.generateSink(S, Pred, Pred->getState()); Engine.addAbortedBlock(node, currBldrCtx->getBlock()); @@ -1013,6 +1012,17 @@ Bldr.addNodes(Dst); break; + case Stmt::LambdaExprClass: + if (AMgr.options.shouldInlineLambdas()) { + Bldr.takeNodes(Pred); + VisitLambdaExpr(cast<LambdaExpr>(S), Pred, Dst); + Bldr.addNodes(Dst); + } else { + const ExplodedNode *node = Bldr.generateSink(S, Pred, Pred->getState()); + Engine.addAbortedBlock(node, currBldrCtx->getBlock()); + } + break; + case Stmt::BinaryOperatorClass: { const BinaryOperator* B = cast<BinaryOperator>(S); if (B->isLogicalOp()) { @@ -1853,11 +1863,35 @@ // C permits "extern void v", and if you cast the address to a valid type, // you can even do things with it. We simply pretend assert(Ex->isGLValue() || VD->getType()->isVoidType()); - SVal V = state->getLValue(VD, Pred->getLocationContext()); + const LocationContext *LocCtxt = Pred->getLocationContext(); + const Decl *D = LocCtxt->getDecl(); + const auto *MD = D ? dyn_cast<CXXMethodDecl>(D) : nullptr; + const auto *DeclRefEx = dyn_cast<DeclRefExpr>(Ex); + SVal V; + bool CaptureByReference = false; + if (AMgr.options.shouldInlineLambdas() && DeclRefEx && + DeclRefEx->refersToEnclosingVariableOrCapture() && MD && + MD->getParent()->isLambda()) { + // Lookup the field of the lambda. + const CXXRecordDecl *CXXRec = MD->getParent(); + llvm::DenseMap<const VarDecl *, FieldDecl *> LambdaCaptureFields; + FieldDecl *LambdaThisCaptureField; + CXXRec->getCaptureFields(LambdaCaptureFields, LambdaThisCaptureField); + const FieldDecl *FD = LambdaCaptureFields[VD]; + Loc CXXThis = + svalBuilder.getCXXThis(MD, LocCtxt->getCurrentStackFrame()); + SVal CXXThisVal = state->getSVal(CXXThis); + V = state->getLValue(FD, CXXThisVal); + if (FD->getType()->isReferenceType() && + !VD->getType()->isReferenceType()) + CaptureByReference = true; + } else { + V = state->getLValue(VD, LocCtxt); + } // For references, the 'lvalue' is the pointer address stored in the // reference region. - if (VD->getType()->isReferenceType()) { + if (VD->getType()->isReferenceType() || CaptureByReference) { if (const MemRegion *R = V.getAsRegion()) V = state->getSVal(R); else Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp =================================================================== --- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -325,3 +325,7 @@ bool AnalyzerOptions::shouldConditionalizeStaticInitializers() { return getBooleanOption("cfg-conditional-static-initializers", true); } + +bool AnalyzerOptions::shouldInlineLambdas() { + return getBooleanOption("inline-lambdas", /*Default=*/false); +} Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -341,6 +341,10 @@ void VisitBlockExpr(const BlockExpr *BE, ExplodedNode *Pred, ExplodedNodeSet &Dst); + /// VisitLambdaExpr - Transfer function logic for LambdaExprs. + void VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred, + ExplodedNodeSet &Dst); + /// VisitBinaryOperator - Transfer function logic for binary operators. void VisitBinaryOperator(const BinaryOperator* B, ExplodedNode *Pred, ExplodedNodeSet &Dst); Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h =================================================================== --- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -256,6 +256,9 @@ /// \sa getMaxNodesPerTopLevelFunction Optional<unsigned> MaxNodesPerTopLevelFunction; + /// \sa shouldInlineLambdas + Optional<bool> InlineLambdas; + /// A helper function that retrieves option for a given full-qualified /// checker name. /// Options for checkers can be specified via 'analyzer-config' command-line @@ -509,6 +512,10 @@ /// This is controlled by the 'max-nodes' config option. unsigned getMaxNodesPerTopLevelFunction(); + /// Returns true if lambdas should be inlined. Otherwise a sink node will be + /// generated each time a LambdaExpr is visited. + bool shouldInlineLambdas(); + public: AnalyzerOptions() : AnalysisStoreOpt(RegionStoreModel),
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits