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