Jordan Rose | 51327f9 | 2013-11-08 01:15:39 +0000 | [diff] [blame] | 1 | //== IdenticalExprChecker.cpp - Identical expression checker----------------==// |
| 2 | // |
| 3 | // The LLVM Compiler Infrastructure |
| 4 | // |
| 5 | // This file is distributed under the University of Illinois Open Source |
| 6 | // License. See LICENSE.TXT for details. |
| 7 | // |
| 8 | //===----------------------------------------------------------------------===// |
| 9 | /// |
| 10 | /// \file |
| 11 | /// \brief This defines IdenticalExprChecker, a check that warns about |
| 12 | /// unintended use of identical expressions. |
| 13 | /// |
| 14 | /// It checks for use of identical expressions with comparison operators. |
| 15 | /// |
| 16 | //===----------------------------------------------------------------------===// |
| 17 | |
| 18 | #include "ClangSACheckers.h" |
| 19 | #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" |
| 20 | #include "clang/StaticAnalyzer/Core/Checker.h" |
| 21 | #include "clang/StaticAnalyzer/Core/CheckerManager.h" |
| 22 | #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" |
| 23 | #include "clang/AST/RecursiveASTVisitor.h" |
| 24 | |
| 25 | using namespace clang; |
| 26 | using namespace ento; |
| 27 | |
| 28 | static bool isIdenticalExpr(const ASTContext &Ctx, const Expr *Expr1, |
| 29 | const Expr *Expr2); |
| 30 | //===----------------------------------------------------------------------===// |
| 31 | // FindIdenticalExprVisitor - Identify nodes using identical expressions. |
| 32 | //===----------------------------------------------------------------------===// |
| 33 | |
Benjamin Kramer | e8a2c18 | 2013-11-14 15:46:10 +0000 | [diff] [blame^] | 34 | namespace { |
Jordan Rose | 51327f9 | 2013-11-08 01:15:39 +0000 | [diff] [blame] | 35 | class FindIdenticalExprVisitor |
| 36 | : public RecursiveASTVisitor<FindIdenticalExprVisitor> { |
| 37 | public: |
| 38 | explicit FindIdenticalExprVisitor(BugReporter &B, AnalysisDeclContext *A) |
| 39 | : BR(B), AC(A) {} |
| 40 | // FindIdenticalExprVisitor only visits nodes |
| 41 | // that are binary operators. |
| 42 | bool VisitBinaryOperator(const BinaryOperator *B); |
| 43 | |
| 44 | private: |
| 45 | BugReporter &BR; |
| 46 | AnalysisDeclContext *AC; |
| 47 | }; |
Benjamin Kramer | e8a2c18 | 2013-11-14 15:46:10 +0000 | [diff] [blame^] | 48 | } // end anonymous namespace |
Jordan Rose | 51327f9 | 2013-11-08 01:15:39 +0000 | [diff] [blame] | 49 | |
| 50 | bool FindIdenticalExprVisitor::VisitBinaryOperator(const BinaryOperator *B) { |
| 51 | BinaryOperator::Opcode Op = B->getOpcode(); |
| 52 | if (!BinaryOperator::isComparisonOp(Op)) |
| 53 | return true; |
| 54 | // |
| 55 | // Special case for floating-point representation. |
| 56 | // |
| 57 | // If expressions on both sides of comparison operator are of type float, |
| 58 | // then for some comparison operators no warning shall be |
| 59 | // reported even if the expressions are identical from a symbolic point of |
| 60 | // view. Comparison between expressions, declared variables and literals |
| 61 | // are treated differently. |
| 62 | // |
| 63 | // != and == between float literals that have the same value should NOT warn. |
| 64 | // < > between float literals that have the same value SHOULD warn. |
| 65 | // |
| 66 | // != and == between the same float declaration should NOT warn. |
| 67 | // < > between the same float declaration SHOULD warn. |
| 68 | // |
| 69 | // != and == between eq. expressions that evaluates into float |
| 70 | // should NOT warn. |
| 71 | // < > between eq. expressions that evaluates into float |
| 72 | // should NOT warn. |
| 73 | // |
| 74 | const Expr *LHS = B->getLHS()->IgnoreParenImpCasts(); |
| 75 | const Expr *RHS = B->getRHS()->IgnoreParenImpCasts(); |
| 76 | |
| 77 | const DeclRefExpr *DeclRef1 = dyn_cast<DeclRefExpr>(LHS); |
| 78 | const DeclRefExpr *DeclRef2 = dyn_cast<DeclRefExpr>(RHS); |
| 79 | const FloatingLiteral *FloatLit1 = dyn_cast<FloatingLiteral>(LHS); |
| 80 | const FloatingLiteral *FloatLit2 = dyn_cast<FloatingLiteral>(RHS); |
| 81 | if ((DeclRef1) && (DeclRef2)) { |
| 82 | if ((DeclRef1->getType()->hasFloatingRepresentation()) && |
| 83 | (DeclRef2->getType()->hasFloatingRepresentation())) { |
| 84 | if (DeclRef1->getDecl() == DeclRef2->getDecl()) { |
| 85 | if ((Op == BO_EQ) || (Op == BO_NE)) { |
| 86 | return true; |
| 87 | } |
| 88 | } |
| 89 | } |
| 90 | } else if ((FloatLit1) && (FloatLit2)) { |
| 91 | if (FloatLit1->getValue().bitwiseIsEqual(FloatLit2->getValue())) { |
| 92 | if ((Op == BO_EQ) || (Op == BO_NE)) { |
| 93 | return true; |
| 94 | } |
| 95 | } |
| 96 | } else if (LHS->getType()->hasFloatingRepresentation()) { |
| 97 | // If any side of comparison operator still has floating-point |
| 98 | // representation, then it's an expression. Don't warn. |
| 99 | // Here only LHS is checked since RHS will be implicit casted to float. |
| 100 | return true; |
| 101 | } else { |
| 102 | // No special case with floating-point representation, report as usual. |
| 103 | } |
| 104 | |
| 105 | if (isIdenticalExpr(AC->getASTContext(), B->getLHS(), B->getRHS())) { |
| 106 | PathDiagnosticLocation ELoc = |
| 107 | PathDiagnosticLocation::createOperatorLoc(B, BR.getSourceManager()); |
| 108 | StringRef Message; |
| 109 | if (((Op == BO_EQ) || (Op == BO_LE) || (Op == BO_GE))) |
| 110 | Message = "comparison of identical expressions always evaluates to true"; |
| 111 | else |
| 112 | Message = "comparison of identical expressions always evaluates to false"; |
| 113 | BR.EmitBasicReport(AC->getDecl(), "Compare of identical expressions", |
| 114 | categories::LogicError, Message, ELoc); |
| 115 | } |
| 116 | // We want to visit ALL nodes (subexpressions of binary comparison |
| 117 | // expressions too) that contains comparison operators. |
| 118 | // True is always returned to traverse ALL nodes. |
| 119 | return true; |
| 120 | } |
| 121 | /// \brief Determines whether two expression trees are identical regarding |
| 122 | /// operators and symbols. |
| 123 | /// |
| 124 | /// Exceptions: expressions containing macros or functions with possible side |
| 125 | /// effects are never considered identical. |
| 126 | /// Limitations: (t + u) and (u + t) are not considered identical. |
| 127 | /// t*(u + t) and t*u + t*t are not considered identical. |
| 128 | /// |
| 129 | static bool isIdenticalExpr(const ASTContext &Ctx, const Expr *Expr1, |
| 130 | const Expr *Expr2) { |
| 131 | // If Expr1 & Expr2 are of different class then they are not |
| 132 | // identical expression. |
| 133 | if (Expr1->getStmtClass() != Expr2->getStmtClass()) |
| 134 | return false; |
| 135 | // If Expr1 has side effects then don't warn even if expressions |
| 136 | // are identical. |
| 137 | if (Expr1->HasSideEffects(Ctx)) |
| 138 | return false; |
| 139 | // Is expression is based on macro then don't warn even if |
| 140 | // the expressions are identical. |
| 141 | if ((Expr1->getExprLoc().isMacroID()) || (Expr2->getExprLoc().isMacroID())) |
| 142 | return false; |
| 143 | // If all children of two expressions are identical, return true. |
| 144 | Expr::const_child_iterator I1 = Expr1->child_begin(); |
| 145 | Expr::const_child_iterator I2 = Expr2->child_begin(); |
| 146 | while (I1 != Expr1->child_end() && I2 != Expr2->child_end()) { |
| 147 | const Expr *Child1 = dyn_cast<Expr>(*I1); |
| 148 | const Expr *Child2 = dyn_cast<Expr>(*I2); |
| 149 | if (!Child1 || !Child2 || !isIdenticalExpr(Ctx, Child1, Child2)) |
| 150 | return false; |
| 151 | ++I1; |
| 152 | ++I2; |
| 153 | } |
| 154 | // If there are different number of children in the expressions, return false. |
| 155 | // (TODO: check if this is a redundant condition.) |
| 156 | if (I1 != Expr1->child_end()) |
| 157 | return false; |
| 158 | if (I2 != Expr2->child_end()) |
| 159 | return false; |
| 160 | |
| 161 | switch (Expr1->getStmtClass()) { |
| 162 | default: |
| 163 | return false; |
| 164 | case Stmt::ArraySubscriptExprClass: |
| 165 | case Stmt::CStyleCastExprClass: |
| 166 | case Stmt::ImplicitCastExprClass: |
| 167 | case Stmt::ParenExprClass: |
| 168 | return true; |
| 169 | case Stmt::BinaryOperatorClass: { |
| 170 | const BinaryOperator *BinOp1 = dyn_cast<BinaryOperator>(Expr1); |
| 171 | const BinaryOperator *BinOp2 = dyn_cast<BinaryOperator>(Expr2); |
| 172 | return BinOp1->getOpcode() == BinOp2->getOpcode(); |
| 173 | } |
| 174 | case Stmt::CharacterLiteralClass: { |
| 175 | const CharacterLiteral *CharLit1 = dyn_cast<CharacterLiteral>(Expr1); |
| 176 | const CharacterLiteral *CharLit2 = dyn_cast<CharacterLiteral>(Expr2); |
| 177 | return CharLit1->getValue() == CharLit2->getValue(); |
| 178 | } |
| 179 | case Stmt::DeclRefExprClass: { |
| 180 | const DeclRefExpr *DeclRef1 = dyn_cast<DeclRefExpr>(Expr1); |
| 181 | const DeclRefExpr *DeclRef2 = dyn_cast<DeclRefExpr>(Expr2); |
| 182 | return DeclRef1->getDecl() == DeclRef2->getDecl(); |
| 183 | } |
| 184 | case Stmt::IntegerLiteralClass: { |
| 185 | const IntegerLiteral *IntLit1 = dyn_cast<IntegerLiteral>(Expr1); |
| 186 | const IntegerLiteral *IntLit2 = dyn_cast<IntegerLiteral>(Expr2); |
| 187 | return IntLit1->getValue() == IntLit2->getValue(); |
| 188 | } |
| 189 | case Stmt::FloatingLiteralClass: { |
| 190 | const FloatingLiteral *FloatLit1 = dyn_cast<FloatingLiteral>(Expr1); |
| 191 | const FloatingLiteral *FloatLit2 = dyn_cast<FloatingLiteral>(Expr2); |
| 192 | return FloatLit1->getValue().bitwiseIsEqual(FloatLit2->getValue()); |
| 193 | } |
| 194 | case Stmt::MemberExprClass: { |
| 195 | const MemberExpr *MemberExpr1 = dyn_cast<MemberExpr>(Expr1); |
| 196 | const MemberExpr *MemberExpr2 = dyn_cast<MemberExpr>(Expr2); |
| 197 | return MemberExpr1->getMemberDecl() == MemberExpr2->getMemberDecl(); |
| 198 | } |
| 199 | case Stmt::UnaryOperatorClass: { |
| 200 | const UnaryOperator *UnaryOp1 = dyn_cast<UnaryOperator>(Expr1); |
| 201 | const UnaryOperator *UnaryOp2 = dyn_cast<UnaryOperator>(Expr2); |
| 202 | if (UnaryOp1->getOpcode() != UnaryOp2->getOpcode()) |
| 203 | return false; |
| 204 | return !UnaryOp1->isIncrementDecrementOp(); |
| 205 | } |
| 206 | } |
| 207 | } |
| 208 | |
| 209 | //===----------------------------------------------------------------------===// |
| 210 | // FindIdenticalExprChecker |
| 211 | //===----------------------------------------------------------------------===// |
| 212 | |
Benjamin Kramer | e8a2c18 | 2013-11-14 15:46:10 +0000 | [diff] [blame^] | 213 | namespace { |
Jordan Rose | 51327f9 | 2013-11-08 01:15:39 +0000 | [diff] [blame] | 214 | class FindIdenticalExprChecker : public Checker<check::ASTCodeBody> { |
| 215 | public: |
| 216 | void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr, |
| 217 | BugReporter &BR) const { |
| 218 | FindIdenticalExprVisitor Visitor(BR, Mgr.getAnalysisDeclContext(D)); |
| 219 | Visitor.TraverseDecl(const_cast<Decl *>(D)); |
| 220 | } |
| 221 | }; |
Benjamin Kramer | e8a2c18 | 2013-11-14 15:46:10 +0000 | [diff] [blame^] | 222 | } // end anonymous namespace |
Jordan Rose | 51327f9 | 2013-11-08 01:15:39 +0000 | [diff] [blame] | 223 | |
| 224 | void ento::registerIdenticalExprChecker(CheckerManager &Mgr) { |
| 225 | Mgr.registerChecker<FindIdenticalExprChecker>(); |
| 226 | } |