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