From ac73ea8c12772fd0dcec71b83c193a2837de7f8b Mon Sep 17 00:00:00 2001 From: Jordy Rose Date: Fri, 10 Jun 2011 08:49:37 +0000 Subject: [PATCH] [analyzer] PR8962 again. Ban ParenExprs (and friends) from block-level expressions (by calling IgnoreParens before adding expressions to blocks). Undo 132769 (LiveVariables' local IgnoreParens), since it's no longer necessary. Also, have Environment stop looking through NoOp casts; it didn't match the behavior of LiveVariables. And once that's gone, the whole cast block of that switch is unnecessary. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@132840 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/AnalysisContext.cpp | 8 ++++---- lib/Analysis/CFG.cpp | 25 +++++++++++++++---------- lib/Analysis/LiveVariables.cpp | 16 +++------------- lib/StaticAnalyzer/Core/Environment.cpp | 15 --------------- test/Analysis/misc-ps.c | 19 +++++++++++++++++++ 5 files changed, 41 insertions(+), 42 deletions(-) diff --git a/lib/Analysis/AnalysisContext.cpp b/lib/Analysis/AnalysisContext.cpp index ddc5e88703..678f02fd71 100644 --- a/lib/Analysis/AnalysisContext.cpp +++ b/lib/Analysis/AnalysisContext.cpp @@ -78,16 +78,16 @@ void AnalysisContext::registerForcedBlockExpression(const Stmt *stmt) { if (!forcedBlkExprs) forcedBlkExprs = new CFG::BuildOptions::ForcedBlkExprs(); // Default construct an entry for 'stmt'. - if (const ParenExpr *pe = dyn_cast(stmt)) - stmt = pe->IgnoreParens(); + if (const Expr *e = dyn_cast(stmt)) + stmt = e->IgnoreParens(); (void) (*forcedBlkExprs)[stmt]; } const CFGBlock * AnalysisContext::getBlockForRegisteredExpression(const Stmt *stmt) { assert(forcedBlkExprs); - if (const ParenExpr *pe = dyn_cast(stmt)) - stmt = pe->IgnoreParens(); + if (const Expr *e = dyn_cast(stmt)) + stmt = e->IgnoreParens(); CFG::BuildOptions::ForcedBlkExprs::const_iterator itr = forcedBlkExprs->find(stmt); assert(itr != forcedBlkExprs->end()); diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp index 621adb3e51..3e540203ea 100644 --- a/lib/Analysis/CFG.cpp +++ b/lib/Analysis/CFG.cpp @@ -397,6 +397,8 @@ private: if (alwaysAdd(S)) cachedEntry->second = B; + // All block-level expressions should have already been IgnoreParens()ed. + assert(!isa(S) || cast(S)->IgnoreParens() == S); B->appendStmt(const_cast(S), cfg->getBumpVectorContext()); } void appendInitializer(CFGBlock *B, CXXCtorInitializer *I) { @@ -841,11 +843,14 @@ void CFGBuilder::prependAutomaticObjDtorsWithTerminator(CFGBlock* Blk, /// blocks for ternary operators, &&, and ||. We also process "," and /// DeclStmts (which may contain nested control-flow). CFGBlock* CFGBuilder::Visit(Stmt * S, AddStmtChoice asc) { -tryAgain: if (!S) { badCFG = true; return 0; } + + if (Expr *E = dyn_cast(S)) + S = E->IgnoreParens(); + switch (S->getStmtClass()) { default: return VisitStmt(S, asc); @@ -957,10 +962,6 @@ tryAgain: case Stmt::ObjCForCollectionStmtClass: return VisitObjCForCollectionStmt(cast(S)); - case Stmt::ParenExprClass: - S = cast(S)->getSubExpr(); - goto tryAgain; - case Stmt::NullStmtClass: return Block; @@ -3049,6 +3050,7 @@ static BlkExprMapTy* PopulateBlkExprMap(CFG& cfg) { if (!CS) continue; if (Expr* Exp = dyn_cast(CS->getStmt())) { + assert((Exp->IgnoreParens() == Exp) && "No parens on block-level exps"); if (BinaryOperator* B = dyn_cast(Exp)) { // Assignment expressions that are not nested within another @@ -3056,13 +3058,16 @@ static BlkExprMapTy* PopulateBlkExprMap(CFG& cfg) { // another expression. if (B->isAssignmentOp() && !SubExprAssignments.count(Exp)) continue; - } else if (const StmtExpr* Terminator = dyn_cast(Exp)) { + } else if (const StmtExpr* SE = dyn_cast(Exp)) { // Special handling for statement expressions. The last statement in // the statement expression is also a block-level expr. - const CompoundStmt* C = Terminator->getSubStmt(); + const CompoundStmt* C = SE->getSubStmt(); if (!C->body_empty()) { + const Stmt *Last = C->body_back(); + if (const Expr *LastEx = dyn_cast(Last)) + Last = LastEx->IgnoreParens(); unsigned x = M->size(); - (*M)[C->body_back()] = x; + (*M)[Last] = x; } } @@ -3076,8 +3081,8 @@ static BlkExprMapTy* PopulateBlkExprMap(CFG& cfg) { Stmt* S = (*I)->getTerminatorCondition(); if (S && M->find(S) == M->end()) { - unsigned x = M->size(); - (*M)[S] = x; + unsigned x = M->size(); + (*M)[S] = x; } } diff --git a/lib/Analysis/LiveVariables.cpp b/lib/Analysis/LiveVariables.cpp index 0fe87e8719..7b36f85ab6 100644 --- a/lib/Analysis/LiveVariables.cpp +++ b/lib/Analysis/LiveVariables.cpp @@ -142,12 +142,8 @@ void TransferFuncs::Visit(Stmt *S) { if (AD.Observer) AD.Observer->ObserveStmt(S, currentBlock, AD, LiveState); - if (getCFG().isBlkExpr(S)) { - if (Expr *E = dyn_cast(S)) - LiveState(E->IgnoreParens(), AD) = Dead; - else - LiveState(S, AD) = Dead; - } + if (getCFG().isBlkExpr(S)) + LiveState(S, AD) = Dead; StmtVisitor::Visit(S); } @@ -161,10 +157,7 @@ void TransferFuncs::Visit(Stmt *S) { } else { // For block-level expressions, mark that they are live. - if (Expr *E = dyn_cast(S)) - LiveState(E->IgnoreParens(), AD) = Alive; - else - LiveState(S, AD) = Alive; + LiveState(S, AD) = Alive; } } @@ -181,9 +174,6 @@ void TransferFuncs::VisitTerminator(CFGBlock* B) { return; assert (getCFG().isBlkExpr(E)); - - if (const Expr *Ex = dyn_cast(E)) - E = Ex->IgnoreParens(); LiveState(E, AD) = Alive; } diff --git a/lib/StaticAnalyzer/Core/Environment.cpp b/lib/StaticAnalyzer/Core/Environment.cpp index e9f5016b36..48f126bfd8 100644 --- a/lib/StaticAnalyzer/Core/Environment.cpp +++ b/lib/StaticAnalyzer/Core/Environment.cpp @@ -77,21 +77,6 @@ SVal Environment::getSVal(const Stmt *E, SValBuilder& svalBuilder, // For special C0xx nullptr case, make a null pointer SVal. case Stmt::CXXNullPtrLiteralExprClass: return svalBuilder.makeNull(); - case Stmt::ImplicitCastExprClass: - case Stmt::CXXFunctionalCastExprClass: - case Stmt::CStyleCastExprClass: { - // We blast through no-op casts to get the descendant - // subexpression that has a value. - const CastExpr* C = cast(E); - QualType CT = C->getType(); - if (CT->isVoidType()) - return UnknownVal(); - if (C->getCastKind() == CK_NoOp) { - E = C->getSubExpr(); - continue; - } - break; - } case Stmt::ExprWithCleanupsClass: E = cast(E)->getSubExpr(); continue; diff --git a/test/Analysis/misc-ps.c b/test/Analysis/misc-ps.c index 84992c6bb2..bef5b06181 100644 --- a/test/Analysis/misc-ps.c +++ b/test/Analysis/misc-ps.c @@ -62,3 +62,22 @@ int PR8962_d (int *t) { return *t; // no-warning } +int PR8962_e (int *t) { + // Redundant casts can mess things up! + // Environment used to skip through NoOp casts, but LiveVariables didn't! + if (({ (t ? (int)(int)0L : (int)(int)1L); })) return 0; + return *t; // no-warning +} + +int PR8962_f (int *t) { + // The StmtExpr isn't a block-level expression here, + // the __extension__ is. But the value should be attached to the StmtExpr + // anyway. Make sure the block-level check is /before/ IgnoreParens. + if ( __extension__({ + _Bool r; + if (t) r = 0; + else r = 1; + r; + }) ) return 0; + return *t; // no-warning +} -- 2.50.1