208 lines · cpp
1//===----------------------------------------------------------------------===//2//3// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.4// See https://llvm.org/LICENSE.txt for license information.5// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception6//7//===----------------------------------------------------------------------===//8 9#include "SuspiciousStringCompareCheck.h"10#include "../utils/OptionsUtils.h"11#include "clang/AST/ASTContext.h"12#include "clang/ASTMatchers/ASTMatchFinder.h"13#include "clang/Lex/Lexer.h"14 15using namespace clang::ast_matchers;16 17namespace clang::tidy::bugprone {18 19// Semicolon separated list of known string compare-like functions. The list20// must ends with a semicolon.21static const char KnownStringCompareFunctions[] = "__builtin_memcmp;"22 "__builtin_strcasecmp;"23 "__builtin_strcmp;"24 "__builtin_strncasecmp;"25 "__builtin_strncmp;"26 "_mbscmp;"27 "_mbscmp_l;"28 "_mbsicmp;"29 "_mbsicmp_l;"30 "_mbsnbcmp;"31 "_mbsnbcmp_l;"32 "_mbsnbicmp;"33 "_mbsnbicmp_l;"34 "_mbsncmp;"35 "_mbsncmp_l;"36 "_mbsnicmp;"37 "_mbsnicmp_l;"38 "_memicmp;"39 "_memicmp_l;"40 "_stricmp;"41 "_stricmp_l;"42 "_strnicmp;"43 "_strnicmp_l;"44 "_wcsicmp;"45 "_wcsicmp_l;"46 "_wcsnicmp;"47 "_wcsnicmp_l;"48 "lstrcmp;"49 "lstrcmpi;"50 "memcmp;"51 "memicmp;"52 "strcasecmp;"53 "strcmp;"54 "strcmpi;"55 "stricmp;"56 "strncasecmp;"57 "strncmp;"58 "strnicmp;"59 "wcscasecmp;"60 "wcscmp;"61 "wcsicmp;"62 "wcsncmp;"63 "wcsnicmp;"64 "wmemcmp;";65 66SuspiciousStringCompareCheck::SuspiciousStringCompareCheck(67 StringRef Name, ClangTidyContext *Context)68 : ClangTidyCheck(Name, Context),69 WarnOnImplicitComparison(Options.get("WarnOnImplicitComparison", true)),70 WarnOnLogicalNotComparison(71 Options.get("WarnOnLogicalNotComparison", false)),72 StringCompareLikeFunctions(73 Options.get("StringCompareLikeFunctions", "")) {}74 75void SuspiciousStringCompareCheck::storeOptions(76 ClangTidyOptions::OptionMap &Opts) {77 Options.store(Opts, "WarnOnImplicitComparison", WarnOnImplicitComparison);78 Options.store(Opts, "WarnOnLogicalNotComparison", WarnOnLogicalNotComparison);79 Options.store(Opts, "StringCompareLikeFunctions", StringCompareLikeFunctions);80}81 82void SuspiciousStringCompareCheck::registerMatchers(MatchFinder *Finder) {83 // Match relational operators.84 const auto ComparisonUnaryOperator = unaryOperator(hasOperatorName("!"));85 const auto ComparisonBinaryOperator = binaryOperator(isComparisonOperator());86 const auto ComparisonOperator =87 expr(anyOf(ComparisonUnaryOperator, ComparisonBinaryOperator));88 89 // Add the list of known string compare-like functions and add user-defined90 // functions.91 const std::vector<StringRef> FunctionNames = utils::options::parseListPair(92 KnownStringCompareFunctions, StringCompareLikeFunctions);93 94 // Match a call to a string compare functions.95 const auto FunctionCompareDecl =96 functionDecl(hasAnyName(FunctionNames)).bind("decl");97 const auto DirectStringCompareCallExpr =98 callExpr(hasDeclaration(FunctionCompareDecl)).bind("call");99 const auto MacroStringCompareCallExpr = conditionalOperator(anyOf(100 hasTrueExpression(ignoringParenImpCasts(DirectStringCompareCallExpr)),101 hasFalseExpression(ignoringParenImpCasts(DirectStringCompareCallExpr))));102 // The implicit cast is not present in C.103 const auto StringCompareCallExpr = ignoringParenImpCasts(104 anyOf(DirectStringCompareCallExpr, MacroStringCompareCallExpr));105 106 if (WarnOnImplicitComparison) {107 // Detect suspicious calls to string compare:108 // 'if (strcmp())' -> 'if (strcmp() != 0)'109 Finder->addMatcher(110 stmt(anyOf(mapAnyOf(ifStmt, whileStmt, doStmt, forStmt)111 .with(hasCondition(StringCompareCallExpr)),112 binaryOperator(hasAnyOperatorName("&&", "||"),113 hasEitherOperand(StringCompareCallExpr))))114 .bind("missing-comparison"),115 this);116 }117 118 if (WarnOnLogicalNotComparison) {119 // Detect suspicious calls to string compared with '!' operator:120 // 'if (!strcmp())' -> 'if (strcmp() == 0)'121 Finder->addMatcher(unaryOperator(hasOperatorName("!"),122 hasUnaryOperand(ignoringParenImpCasts(123 StringCompareCallExpr)))124 .bind("logical-not-comparison"),125 this);126 }127 128 // Detect suspicious cast to an inconsistent type (i.e. not integer type).129 Finder->addMatcher(130 traverse(TK_AsIs,131 implicitCastExpr(unless(hasType(isInteger())),132 hasSourceExpression(StringCompareCallExpr))133 .bind("invalid-conversion")),134 this);135 136 // Detect suspicious operator with string compare function as operand.137 Finder->addMatcher(138 binaryOperator(unless(anyOf(isComparisonOperator(), hasOperatorName("&&"),139 hasOperatorName("||"), hasOperatorName("="))),140 hasEitherOperand(StringCompareCallExpr))141 .bind("suspicious-operator"),142 this);143 144 // Detect comparison to invalid constant: 'strcmp() == -1'.145 const auto InvalidLiteral = ignoringParenImpCasts(146 anyOf(integerLiteral(unless(equals(0))),147 unaryOperator(148 hasOperatorName("-"),149 has(ignoringParenImpCasts(integerLiteral(unless(equals(0)))))),150 characterLiteral(), cxxBoolLiteral()));151 152 Finder->addMatcher(153 binaryOperator(isComparisonOperator(),154 hasOperands(StringCompareCallExpr, InvalidLiteral))155 .bind("invalid-comparison"),156 this);157}158 159void SuspiciousStringCompareCheck::check(160 const MatchFinder::MatchResult &Result) {161 const auto *Decl = Result.Nodes.getNodeAs<FunctionDecl>("decl");162 const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");163 assert(Decl != nullptr && Call != nullptr);164 165 if (Result.Nodes.getNodeAs<Stmt>("missing-comparison")) {166 const SourceLocation EndLoc = Lexer::getLocForEndOfToken(167 Call->getRParenLoc(), 0, Result.Context->getSourceManager(),168 getLangOpts());169 170 diag(Call->getBeginLoc(),171 "function %0 is called without explicitly comparing result")172 << Decl << FixItHint::CreateInsertion(EndLoc, " != 0");173 }174 175 if (const auto *E = Result.Nodes.getNodeAs<Expr>("logical-not-comparison")) {176 const SourceLocation EndLoc = Lexer::getLocForEndOfToken(177 Call->getRParenLoc(), 0, Result.Context->getSourceManager(),178 getLangOpts());179 const SourceLocation NotLoc = E->getBeginLoc();180 181 diag(Call->getBeginLoc(),182 "function %0 is compared using logical not operator")183 << Decl184 << FixItHint::CreateRemoval(185 CharSourceRange::getTokenRange(NotLoc, NotLoc))186 << FixItHint::CreateInsertion(EndLoc, " == 0");187 }188 189 if (Result.Nodes.getNodeAs<Stmt>("invalid-comparison")) {190 diag(Call->getBeginLoc(),191 "function %0 is compared to a suspicious constant")192 << Decl;193 }194 195 if (const auto *BinOp =196 Result.Nodes.getNodeAs<BinaryOperator>("suspicious-operator")) {197 diag(Call->getBeginLoc(), "results of function %0 used by operator '%1'")198 << Decl << BinOp->getOpcodeStr();199 }200 201 if (Result.Nodes.getNodeAs<Stmt>("invalid-conversion")) {202 diag(Call->getBeginLoc(), "function %0 has suspicious implicit cast")203 << Decl;204 }205}206 207} // namespace clang::tidy::bugprone208