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