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/OptionsUtils.h" |
11 | #include "clang/AST/ASTContext.h" |
12 | #include "clang/ASTMatchers/ASTMatchFinder.h" |
13 | #include "clang/Lex/Lexer.h" |
14 | |
15 | using namespace clang::ast_matchers; |
16 | |
17 | namespace clang::tidy::bugprone { |
18 | |
19 | // Semicolon separated list of known string compare-like functions. The list |
20 | // must ends with a semicolon. |
21 | static 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 | |
66 | SuspiciousStringCompareCheck::SuspiciousStringCompareCheck( |
67 | StringRef Name, ClangTidyContext *Context) |
68 | : ClangTidyCheck(Name, Context), |
69 | WarnOnImplicitComparison(Options.get(LocalName: "WarnOnImplicitComparison" , Default: true)), |
70 | WarnOnLogicalNotComparison( |
71 | Options.get(LocalName: "WarnOnLogicalNotComparison" , Default: false)), |
72 | StringCompareLikeFunctions( |
73 | Options.get(LocalName: "StringCompareLikeFunctions" , Default: "" )) {} |
74 | |
75 | void SuspiciousStringCompareCheck::storeOptions( |
76 | ClangTidyOptions::OptionMap &Opts) { |
77 | Options.store(Options&: Opts, LocalName: "WarnOnImplicitComparison" , Value: WarnOnImplicitComparison); |
78 | Options.store(Options&: Opts, LocalName: "WarnOnLogicalNotComparison" , Value: WarnOnLogicalNotComparison); |
79 | Options.store(Options&: Opts, LocalName: "StringCompareLikeFunctions" , Value: StringCompareLikeFunctions); |
80 | } |
81 | |
82 | void SuspiciousStringCompareCheck::registerMatchers(MatchFinder *Finder) { |
83 | // Match relational operators. |
84 | const auto ComparisonUnaryOperator = unaryOperator(hasOperatorName(Name: "!" )); |
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-defined |
90 | // functions. |
91 | std::vector<StringRef> FunctionNames = utils::options::parseListPair( |
92 | L: KnownStringCompareFunctions, R: StringCompareLikeFunctions); |
93 | |
94 | // Match a call to a string compare functions. |
95 | const auto FunctionCompareDecl = |
96 | functionDecl(hasAnyName(FunctionNames)).bind(ID: "decl" ); |
97 | const auto DirectStringCompareCallExpr = |
98 | callExpr(hasDeclaration(InnerMatcher: FunctionCompareDecl)).bind(ID: "call" ); |
99 | const auto MacroStringCompareCallExpr = conditionalOperator(anyOf( |
100 | hasTrueExpression(InnerMatcher: ignoringParenImpCasts(InnerMatcher: DirectStringCompareCallExpr)), |
101 | hasFalseExpression(InnerMatcher: ignoringParenImpCasts(InnerMatcher: DirectStringCompareCallExpr)))); |
102 | // The implicit cast is not present in C. |
103 | const auto StringCompareCallExpr = ignoringParenImpCasts( |
104 | InnerMatcher: anyOf(DirectStringCompareCallExpr, MacroStringCompareCallExpr)); |
105 | |
106 | if (WarnOnImplicitComparison) { |
107 | // Detect suspicious calls to string compare: |
108 | // 'if (strcmp())' -> 'if (strcmp() != 0)' |
109 | Finder->addMatcher( |
110 | NodeMatch: stmt(anyOf(mapAnyOf(ifStmt, whileStmt, doStmt, forStmt) |
111 | .with(hasCondition(InnerMatcher: StringCompareCallExpr)), |
112 | binaryOperator(hasAnyOperatorName("&&" , "||" ), |
113 | hasEitherOperand(InnerMatcher: StringCompareCallExpr)))) |
114 | .bind(ID: "missing-comparison" ), |
115 | Action: this); |
116 | } |
117 | |
118 | if (WarnOnLogicalNotComparison) { |
119 | // Detect suspicious calls to string compared with '!' operator: |
120 | // 'if (!strcmp())' -> 'if (strcmp() == 0)' |
121 | Finder->addMatcher(NodeMatch: unaryOperator(hasOperatorName(Name: "!" ), |
122 | hasUnaryOperand(InnerMatcher: ignoringParenImpCasts( |
123 | InnerMatcher: StringCompareCallExpr))) |
124 | .bind(ID: "logical-not-comparison" ), |
125 | Action: this); |
126 | } |
127 | |
128 | // Detect suspicious cast to an inconsistent type (i.e. not integer type). |
129 | Finder->addMatcher( |
130 | NodeMatch: traverse(TK: TK_AsIs, |
131 | InnerMatcher: implicitCastExpr(unless(hasType(InnerMatcher: isInteger())), |
132 | hasSourceExpression(InnerMatcher: StringCompareCallExpr)) |
133 | .bind(ID: "invalid-conversion" )), |
134 | Action: this); |
135 | |
136 | // Detect suspicious operator with string compare function as operand. |
137 | Finder->addMatcher( |
138 | NodeMatch: binaryOperator(unless(anyOf(isComparisonOperator(), hasOperatorName(Name: "&&" ), |
139 | hasOperatorName(Name: "||" ), hasOperatorName(Name: "=" ))), |
140 | hasEitherOperand(InnerMatcher: StringCompareCallExpr)) |
141 | .bind(ID: "suspicious-operator" ), |
142 | Action: this); |
143 | |
144 | // Detect comparison to invalid constant: 'strcmp() == -1'. |
145 | const auto InvalidLiteral = ignoringParenImpCasts( |
146 | InnerMatcher: anyOf(integerLiteral(unless(equals(Value: 0))), |
147 | unaryOperator( |
148 | hasOperatorName(Name: "-" ), |
149 | has(ignoringParenImpCasts(InnerMatcher: integerLiteral(unless(equals(Value: 0)))))), |
150 | characterLiteral(), cxxBoolLiteral())); |
151 | |
152 | Finder->addMatcher( |
153 | NodeMatch: binaryOperator(isComparisonOperator(), |
154 | hasOperands(Matcher1: StringCompareCallExpr, Matcher2: InvalidLiteral)) |
155 | .bind(ID: "invalid-comparison" ), |
156 | Action: this); |
157 | } |
158 | |
159 | void SuspiciousStringCompareCheck::check( |
160 | const MatchFinder::MatchResult &Result) { |
161 | const auto *Decl = Result.Nodes.getNodeAs<FunctionDecl>(ID: "decl" ); |
162 | const auto *Call = Result.Nodes.getNodeAs<CallExpr>(ID: "call" ); |
163 | assert(Decl != nullptr && Call != nullptr); |
164 | |
165 | if (Result.Nodes.getNodeAs<Stmt>(ID: "missing-comparison" )) { |
166 | SourceLocation EndLoc = Lexer::getLocForEndOfToken( |
167 | Loc: Call->getRParenLoc(), Offset: 0, SM: Result.Context->getSourceManager(), |
168 | LangOpts: getLangOpts()); |
169 | |
170 | diag(Loc: Call->getBeginLoc(), |
171 | Description: "function %0 is called without explicitly comparing result" ) |
172 | << Decl << FixItHint::CreateInsertion(InsertionLoc: EndLoc, Code: " != 0" ); |
173 | } |
174 | |
175 | if (const auto *E = Result.Nodes.getNodeAs<Expr>(ID: "logical-not-comparison" )) { |
176 | SourceLocation EndLoc = Lexer::getLocForEndOfToken( |
177 | Loc: Call->getRParenLoc(), Offset: 0, SM: Result.Context->getSourceManager(), |
178 | LangOpts: getLangOpts()); |
179 | SourceLocation NotLoc = E->getBeginLoc(); |
180 | |
181 | diag(Loc: Call->getBeginLoc(), |
182 | Description: "function %0 is compared using logical not operator" ) |
183 | << Decl |
184 | << FixItHint::CreateRemoval( |
185 | RemoveRange: CharSourceRange::getTokenRange(B: NotLoc, E: NotLoc)) |
186 | << FixItHint::CreateInsertion(InsertionLoc: EndLoc, Code: " == 0" ); |
187 | } |
188 | |
189 | if (Result.Nodes.getNodeAs<Stmt>(ID: "invalid-comparison" )) { |
190 | diag(Loc: Call->getBeginLoc(), |
191 | Description: "function %0 is compared to a suspicious constant" ) |
192 | << Decl; |
193 | } |
194 | |
195 | if (const auto *BinOp = |
196 | Result.Nodes.getNodeAs<BinaryOperator>(ID: "suspicious-operator" )) { |
197 | diag(Loc: Call->getBeginLoc(), Description: "results of function %0 used by operator '%1'" ) |
198 | << Decl << BinOp->getOpcodeStr(); |
199 | } |
200 | |
201 | if (Result.Nodes.getNodeAs<Stmt>(ID: "invalid-conversion" )) { |
202 | diag(Loc: Call->getBeginLoc(), Description: "function %0 has suspicious implicit cast" ) |
203 | << Decl; |
204 | } |
205 | } |
206 | |
207 | } // namespace clang::tidy::bugprone |
208 | |