Index: lib/Analysis/CFG.cpp =================================================================== --- lib/Analysis/CFG.cpp +++ lib/Analysis/CFG.cpp @@ -3486,57 +3486,6 @@ } CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(BinaryOperator *E) { - if (E->isLogicalOp()) { - // Destructors for temporaries in LHS expression should be called after - // those for RHS expression. Even if this will unnecessarily create a block, - // this block will be used at least by the full expression. - autoCreateBlock(); - CFGBlock *ConfluenceBlock = VisitForTemporaryDtors(E->getLHS()); - if (badCFG) - return NULL; - - Succ = ConfluenceBlock; - Block = NULL; - CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getRHS()); - - if (RHSBlock) { - if (badCFG) - return NULL; - - // If RHS expression did produce destructors we need to connect created - // blocks to CFG in same manner as for binary operator itself. - CFGBlock *LHSBlock = createBlock(false); - LHSBlock->setTerminator(CFGTerminator(E, true)); - - // For binary operator LHS block is before RHS in list of predecessors - // of ConfluenceBlock. - std::reverse(ConfluenceBlock->pred_begin(), - ConfluenceBlock->pred_end()); - - // See if this is a known constant. - TryResult KnownVal = tryEvaluateBool(E->getLHS()); - if (KnownVal.isKnown() && (E->getOpcode() == BO_LOr)) - KnownVal.negate(); - - // Link LHSBlock with RHSBlock exactly the same way as for binary operator - // itself. - if (E->getOpcode() == BO_LOr) { - addSuccessor(LHSBlock, KnownVal.isTrue() ? NULL : ConfluenceBlock); - addSuccessor(LHSBlock, KnownVal.isFalse() ? NULL : RHSBlock); - } else { - assert (E->getOpcode() == BO_LAnd); - addSuccessor(LHSBlock, KnownVal.isFalse() ? NULL : RHSBlock); - addSuccessor(LHSBlock, KnownVal.isTrue() ? NULL : ConfluenceBlock); - } - - Block = LHSBlock; - return LHSBlock; - } - - Block = ConfluenceBlock; - return ConfluenceBlock; - } - if (E->isAssignmentOp()) { // For assignment operator (=) LHS expression is visited // before RHS expression. For destructors visit them in reverse order. @@ -3569,10 +3518,18 @@ Succ = B; Block = createNoReturnBlock(); } else { - autoCreateBlock(); + if (B) + Succ = B; + Block = createBlock(); } appendTemporaryDtor(Block, E); + + CFGBlock *Decision = createBlock(false); + Decision->setTerminator(CFGTerminator(E, true)); + addSuccessor(Decision, Block); + addSuccessor(Decision, Succ); + Block = Decision; B = Block; } return B; @@ -3580,65 +3537,10 @@ CFGBlock *CFGBuilder::VisitConditionalOperatorForTemporaryDtors( AbstractConditionalOperator *E, bool BindToTemporary) { - // First add destructors for condition expression. Even if this will - // unnecessarily create a block, this block will be used at least by the full - // expression. - autoCreateBlock(); - CFGBlock *ConfluenceBlock = VisitForTemporaryDtors(E->getCond()); - if (badCFG) - return NULL; - if (BinaryConditionalOperator *BCO - = dyn_cast(E)) { - ConfluenceBlock = VisitForTemporaryDtors(BCO->getCommon()); - if (badCFG) - return NULL; - } - - // Try to add block with destructors for LHS expression. - CFGBlock *LHSBlock = NULL; - Succ = ConfluenceBlock; - Block = NULL; - LHSBlock = VisitForTemporaryDtors(E->getTrueExpr(), BindToTemporary); - if (badCFG) - return NULL; - - // Try to add block with destructors for RHS expression; - Succ = ConfluenceBlock; - Block = NULL; - CFGBlock *RHSBlock = VisitForTemporaryDtors(E->getFalseExpr(), - BindToTemporary); - if (badCFG) - return NULL; - - if (!RHSBlock && !LHSBlock) { - // If neither LHS nor RHS expression had temporaries to destroy don't create - // more blocks. - Block = ConfluenceBlock; - return Block; - } - - Block = createBlock(false); - Block->setTerminator(CFGTerminator(E, true)); - assert(Block->getTerminator().isTemporaryDtorsBranch()); - - // See if this is a known constant. - const TryResult &KnownVal = tryEvaluateBool(E->getCond()); - - if (LHSBlock) { - addSuccessor(Block, LHSBlock, !KnownVal.isFalse()); - } else if (KnownVal.isFalse()) { - addSuccessor(Block, NULL); - } else { - addSuccessor(Block, ConfluenceBlock); - std::reverse(ConfluenceBlock->pred_begin(), ConfluenceBlock->pred_end()); - } - - if (!RHSBlock) - RHSBlock = ConfluenceBlock; - - addSuccessor(Block, RHSBlock, !KnownVal.isTrue()); - - return Block; + CFGBlock *CondBlock = VisitForTemporaryDtors(E->getCond()); + CFGBlock *TrueBlock = VisitForTemporaryDtors(E->getTrueExpr()); + CFGBlock *FalseBlock = VisitForTemporaryDtors(E->getFalseExpr()); + return FalseBlock ? FalseBlock : (TrueBlock ? TrueBlock : CondBlock); } } // end anonymous namespace Index: lib/StaticAnalyzer/Core/CoreEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/CoreEngine.cpp +++ lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -346,6 +346,10 @@ default: llvm_unreachable("Analysis for this terminator not implemented."); + case Stmt::CXXBindTemporaryExprClass: + HandleBranch(B->getTerminator().getStmt(), Term, B, Pred); + return; + // Model static initializers. case Stmt::DeclStmtClass: HandleStaticInit(cast(Term), B, Pred); Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -51,6 +51,10 @@ STATISTIC(NumTimesRetriedWithoutInlining, "The # of times we re-evaluated a call without inlining"); +REGISTER_TRAIT_WITH_PROGRAMSTATE( + InitializedTemporariesSet, + llvm::ImmutableSet); + //===----------------------------------------------------------------------===// // Engine construction and deletion. //===----------------------------------------------------------------------===// @@ -757,6 +761,20 @@ // Handled due to fully linearised CFG. break; + case Stmt::CXXBindTemporaryExprClass: { + Bldr.takeNodes(Pred); + const CXXBindTemporaryExpr *BTE = cast(S); + StmtNodeBuilder StmtBldr(Pred, Dst, *currBldrCtx); + ProgramStateRef State = Pred->getState(); + assert(!getAnalysisManager().options.includeTemporaryDtorsInCFG() || + !State->contains(BTE)); + State = State->add(BTE); + StmtBldr.generateNode(BTE, Pred, State); + + Bldr.addNodes(Dst); + break; + } + // Cases not handled yet; but will handle some day. case Stmt::DesignatedInitExprClass: case Stmt::ExtVectorElementExprClass: @@ -794,7 +812,6 @@ case Stmt::SizeOfPackExprClass: case Stmt::StringLiteralClass: case Stmt::ObjCStringLiteralClass: - case Stmt::CXXBindTemporaryExprClass: case Stmt::CXXPseudoDestructorExprClass: case Stmt::SubstNonTypeTemplateParmExprClass: case Stmt::CXXNullPtrLiteralExprClass: { @@ -1431,6 +1448,21 @@ return; } + if (const CXXBindTemporaryExpr *BTE = + dyn_cast(Condition)) { + BranchNodeBuilder TempDtorBuilder(Pred, Dst, BldCtx, DstT, DstF); + if (Pred->getState()->contains(BTE)) { + TempDtorBuilder.markInfeasible(false); + ProgramStateRef State = Pred->getState(); + State = State->remove(BTE); + TempDtorBuilder.generateNode(State, true, Pred); + } else { + TempDtorBuilder.markInfeasible(true); + TempDtorBuilder.generateNode(Pred->getState(), false, Pred); + } + return; + } + if (const Expr *Ex = dyn_cast(Condition)) Condition = Ex->IgnoreParens(); Index: test/Analysis/temporaries.cpp =================================================================== --- test/Analysis/temporaries.cpp +++ test/Analysis/temporaries.cpp @@ -193,8 +193,7 @@ (i == 4 || i == 4 || compute(i == 5 && (i == 4 || check(NoReturnDtor()))))) || i != 4) { - // FIXME: This shouldn't cause a warning. - clang_analyzer_eval(true); // expected-warning{{TRUE}} + clang_analyzer_eval(true); // no warning, unreachable code } } @@ -211,8 +210,7 @@ void testConsistencyNestedComplex(bool value) { if (value) { if (!value || !value || check(NoReturnDtor())) { - // FIXME: This shouldn't cause a warning. - clang_analyzer_eval(true); // expected-warning{{TRUE}} + clang_analyzer_eval(true); // no warning, unreachable code } } } @@ -225,6 +223,55 @@ } } } + // PR16664 and PR18159 + void testConsistencyNestedComplexMidBranch(bool value) { + if (value) { + if (!value || !value || check(NoReturnDtor()) || value) { + clang_analyzer_eval(true); // no warning, unreachable code + } + } + } + + // PR16664 and PR18159 + void testConsistencyNestedComplexNestedBranch(bool value) { + if (value) { + if (!value || (!value || check(NoReturnDtor()) || value)) { + clang_analyzer_eval(true); // no warning, unreachable code + } + } + } + + // PR16664 and PR18159 + void testConsistencyNestedVariableModification(bool value) { + bool other = true; + if (value) { + if (!other || !value || (other = false) || check(NoReturnDtor()) + || !other) { + clang_analyzer_eval(true); // no warning, unreachable code + } + } + } + + void testTernary(bool value) { + if (value) { + bool b = value && value ? check(NoReturnDtor()) : true; + clang_analyzer_eval(true); // no warning, unreachable code + } + if (value) { + bool b = !value && !value ? true : check(NoReturnDtor()); + clang_analyzer_eval(true); // no warning, unreachable code + } + } + + void testLoop() { + for (int i = 0; i < 10; ++i) { + if (i < 3 && (i >= 2 || check(NoReturnDtor()))) { + // FIXME: Figure out why we don't run into this for i == 2. + // If we add if (i == 2 && (...)) we run into this case, though. + clang_analyzer_eval(true); + } + } + } void testBinaryOperatorShortcut(bool value) { if (value) {