[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring
This revision was automatically updated to reflect the committed changes. Closed by commit rL317849: [analyzer] [NFC] Minor ExprEngineC refactoring (authored by george.karpenkov). Changed prior to commit: https://reviews.llvm.org/D39584?vs=121814&id=122345#toc Repository: rL LLVM https://reviews.llvm.org/D39584 Files: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -20,6 +20,24 @@ using namespace ento; using llvm::APSInt; +/// \brief Optionally conjure and return a symbol for offset when processing +/// an expression \p Expression. +/// If \p Other is a location, conjure a symbol for \p Symbol +/// (offset) if it is unknown so that memory arithmetic always +/// results in an ElementRegion. +/// \p Count The number of times the current basic block was visited. +static SVal conjureOffsetSymbolOnLocation( +SVal Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder, +unsigned Count, const LocationContext *LCtx) { + QualType Ty = Expression->getType(); + if (Other.getAs() && + Ty->isIntegralOrEnumerationType() && + Symbol.isUnknown()) { +return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count); + } + return Symbol; +} + void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -63,24 +81,13 @@ StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx); if (B->isAdditiveOp()) { -// If one of the operands is a location, conjure a symbol for the other -// one (offset) if it's unknown so that memory arithmetic always -// results in an ElementRegion. // TODO: This can be removed after we enable history tracking with // SymSymExpr. unsigned Count = currBldrCtx->blockCount(); -if (LeftV.getAs() && -RHS->getType()->isIntegralOrEnumerationType() && -RightV.isUnknown()) { - RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(), -Count); -} -if (RightV.getAs() && -LHS->getType()->isIntegralOrEnumerationType() && -LeftV.isUnknown()) { - LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(), - Count); -} +RightV = conjureOffsetSymbolOnLocation( +RightV, LeftV, RHS, svalBuilder, Count, LCtx); +LeftV = conjureOffsetSymbolOnLocation( +LeftV, RightV, LHS, svalBuilder, Count, LCtx); } // Although we don't yet model pointers-to-members, we do need to make Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -20,6 +20,24 @@ using namespace ento; using llvm::APSInt; +/// \brief Optionally conjure and return a symbol for offset when processing +/// an expression \p Expression. +/// If \p Other is a location, conjure a symbol for \p Symbol +/// (offset) if it is unknown so that memory arithmetic always +/// results in an ElementRegion. +/// \p Count The number of times the current basic block was visited. +static SVal conjureOffsetSymbolOnLocation( +SVal Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder, +unsigned Count, const LocationContext *LCtx) { + QualType Ty = Expression->getType(); + if (Other.getAs() && + Ty->isIntegralOrEnumerationType() && + Symbol.isUnknown()) { +return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count); + } + return Symbol; +} + void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -63,24 +81,13 @@ StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx); if (B->isAdditiveOp()) { -// If one of the operands is a location, conjure a symbol for the other -// one (offset) if it's unknown so that memory arithmetic always -// results in an ElementRegion. // TODO: This can be removed after we enable history tracking with // SymSymExpr. unsigned Count = currBldrCtx->blockCount(); -if (LeftV.getAs() && -RHS->getType()->isIntegralOrEnumerationType() && -RightV.isUnknown()) { - RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(), -Count); -} -if (RightV.getAs() && -LHS->getType()->isIntegralOrEnumerationType() && -LeftV.isUnknown
[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring
dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. Yes, looks great. Thanks! https://reviews.llvm.org/D39584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring
george.karpenkov added a comment. @dcoughlin OK to commit? https://reviews.llvm.org/D39584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring
george.karpenkov updated this revision to Diff 121814. george.karpenkov edited the summary of this revision. george.karpenkov added a comment. Addressed review comments. https://reviews.llvm.org/D39584 Files: lib/StaticAnalyzer/Core/ExprEngineC.cpp Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp === --- lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -20,6 +20,24 @@ using namespace ento; using llvm::APSInt; +/// \brief Optionally conjure and return a symbol for offset when processing +/// an expression \p Expression. +/// If \p Other is a location, conjure a symbol for \p Symbol +/// (offset) if it is unknown so that memory arithmetic always +/// results in an ElementRegion. +/// \p Count The number of times the current basic block was visited. +static SVal conjureOffsetSymbolOnLocation( +SVal Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder, +unsigned Count, const LocationContext *LCtx) { + QualType Ty = Expression->getType(); + if (Other.getAs() && + Ty->isIntegralOrEnumerationType() && + Symbol.isUnknown()) { +return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count); + } + return Symbol; +} + void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -63,24 +81,13 @@ StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx); if (B->isAdditiveOp()) { -// If one of the operands is a location, conjure a symbol for the other -// one (offset) if it's unknown so that memory arithmetic always -// results in an ElementRegion. // TODO: This can be removed after we enable history tracking with // SymSymExpr. unsigned Count = currBldrCtx->blockCount(); -if (LeftV.getAs() && -RHS->getType()->isIntegralOrEnumerationType() && -RightV.isUnknown()) { - RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(), -Count); -} -if (RightV.getAs() && -LHS->getType()->isIntegralOrEnumerationType() && -LeftV.isUnknown()) { - LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(), - Count); -} +RightV = conjureOffsetSymbolOnLocation( +RightV, LeftV, RHS, svalBuilder, Count, LCtx); +LeftV = conjureOffsetSymbolOnLocation( +LeftV, RightV, LHS, svalBuilder, Count, LCtx); } // Although we don't yet model pointers-to-members, we do need to make Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp === --- lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -20,6 +20,24 @@ using namespace ento; using llvm::APSInt; +/// \brief Optionally conjure and return a symbol for offset when processing +/// an expression \p Expression. +/// If \p Other is a location, conjure a symbol for \p Symbol +/// (offset) if it is unknown so that memory arithmetic always +/// results in an ElementRegion. +/// \p Count The number of times the current basic block was visited. +static SVal conjureOffsetSymbolOnLocation( +SVal Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder, +unsigned Count, const LocationContext *LCtx) { + QualType Ty = Expression->getType(); + if (Other.getAs() && + Ty->isIntegralOrEnumerationType() && + Symbol.isUnknown()) { +return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count); + } + return Symbol; +} + void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -63,24 +81,13 @@ StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx); if (B->isAdditiveOp()) { -// If one of the operands is a location, conjure a symbol for the other -// one (offset) if it's unknown so that memory arithmetic always -// results in an ElementRegion. // TODO: This can be removed after we enable history tracking with // SymSymExpr. unsigned Count = currBldrCtx->blockCount(); -if (LeftV.getAs() && -RHS->getType()->isIntegralOrEnumerationType() && -RightV.isUnknown()) { - RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(), -Count); -} -if (RightV.getAs() && -LHS->getType()->isIntegralOrEnumerationType() && -LeftV.isUnknown()) { - LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(), - Count); -} +RightV = conjureOffsetSymbolO
[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring
dcoughlin added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:26 +/// results in an ElementRegion. +static void conjureOffsetSymbolOnLocation( +SVal &Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder, I think it would be more clear at the call site what the inputs and output are if this function returned the conjured SVal (or 'Symbol' if it wouldn't be conjured). It would also be good to describe the role of the parameters in the doxygen comment. https://reviews.llvm.org/D39584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring
george.karpenkov created this revision. Herald added subscribers: szepet, xazax.hun. couldn't resist adding a function here. I think this would be the last one. https://reviews.llvm.org/D39584 Files: lib/StaticAnalyzer/Core/ExprEngineC.cpp Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp === --- lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -20,6 +20,20 @@ using namespace ento; using llvm::APSInt; +/// If one of the operands is a location, conjure a symbol for the other +/// one (offset) if it's unknown so that memory arithmetic always +/// results in an ElementRegion. +static void conjureOffsetSymbolOnLocation( +SVal &Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder, +unsigned Count, const LocationContext *LCtx) { + QualType Ty = Expression->getType(); + if (Other.getAs() && + Ty->isIntegralOrEnumerationType() && + Symbol.isUnknown()) { +Symbol = svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count); + } +} + void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -63,24 +77,13 @@ StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx); if (B->isAdditiveOp()) { -// If one of the operands is a location, conjure a symbol for the other -// one (offset) if it's unknown so that memory arithmetic always -// results in an ElementRegion. // TODO: This can be removed after we enable history tracking with // SymSymExpr. unsigned Count = currBldrCtx->blockCount(); -if (LeftV.getAs() && -RHS->getType()->isIntegralOrEnumerationType() && -RightV.isUnknown()) { - RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(), -Count); -} -if (RightV.getAs() && -LHS->getType()->isIntegralOrEnumerationType() && -LeftV.isUnknown()) { - LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(), - Count); -} +conjureOffsetSymbolOnLocation( +RightV, LeftV, RHS, svalBuilder, Count, LCtx); +conjureOffsetSymbolOnLocation( +LeftV, RightV, LHS, svalBuilder, Count, LCtx); } // Although we don't yet model pointers-to-members, we do need to make Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp === --- lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -20,6 +20,20 @@ using namespace ento; using llvm::APSInt; +/// If one of the operands is a location, conjure a symbol for the other +/// one (offset) if it's unknown so that memory arithmetic always +/// results in an ElementRegion. +static void conjureOffsetSymbolOnLocation( +SVal &Symbol, SVal Other, Expr* Expression, SValBuilder &svalBuilder, +unsigned Count, const LocationContext *LCtx) { + QualType Ty = Expression->getType(); + if (Other.getAs() && + Ty->isIntegralOrEnumerationType() && + Symbol.isUnknown()) { +Symbol = svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count); + } +} + void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -63,24 +77,13 @@ StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx); if (B->isAdditiveOp()) { -// If one of the operands is a location, conjure a symbol for the other -// one (offset) if it's unknown so that memory arithmetic always -// results in an ElementRegion. // TODO: This can be removed after we enable history tracking with // SymSymExpr. unsigned Count = currBldrCtx->blockCount(); -if (LeftV.getAs() && -RHS->getType()->isIntegralOrEnumerationType() && -RightV.isUnknown()) { - RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(), -Count); -} -if (RightV.getAs() && -LHS->getType()->isIntegralOrEnumerationType() && -LeftV.isUnknown()) { - LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(), - Count); -} +conjureOffsetSymbolOnLocation( +RightV, LeftV, RHS, svalBuilder, Count, LCtx); +conjureOffsetSymbolOnLocation( +LeftV, RightV, LHS, svalBuilder, Count, LCtx); } // Although we don't yet model pointers-to-members, we do need to make ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi