1 //===- NumberObjectConversionChecker.cpp -------------------------*- C++ -*-==//
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 // This file defines NumberObjectConversionChecker, which checks for a
10 // particular common mistake when dealing with numbers represented as objects
11 // passed around by pointers. Namely, the language allows to reinterpret the
12 // pointer as a number directly, often without throwing any warnings,
13 // but in most cases the result of such conversion is clearly unexpected,
14 // as pointer value, rather than number value represented by the pointee object,
15 // becomes the result of such operation.
16 //
17 // Currently the checker supports the Objective-C NSNumber class,
18 // and the OSBoolean class found in macOS low-level code; the latter
19 // can only hold boolean values.
20 //
21 // This checker has an option "Pedantic" (boolean), which enables detection of
22 // more conversion patterns (which are most likely more harmless, and therefore
23 // are more likely to produce false positives) - disabled by default,
24 // enabled with `-analyzer-config osx.NumberObjectConversion:Pedantic=true'.
25 //
26 //===----------------------------------------------------------------------===//
27
28 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
29 #include "clang/ASTMatchers/ASTMatchFinder.h"
30 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
31 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
32 #include "clang/StaticAnalyzer/Core/Checker.h"
33 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
34 #include "clang/Lex/Lexer.h"
35 #include "llvm/ADT/APSInt.h"
36
37 using namespace clang;
38 using namespace ento;
39 using namespace ast_matchers;
40
41 namespace {
42
43 class NumberObjectConversionChecker : public Checker<check::ASTCodeBody> {
44 public:
45 bool Pedantic;
46
47 void checkASTCodeBody(const Decl *D, AnalysisManager &AM,
48 BugReporter &BR) const;
49 };
50
51 class Callback : public MatchFinder::MatchCallback {
52 const NumberObjectConversionChecker *C;
53 BugReporter &BR;
54 AnalysisDeclContext *ADC;
55
56 public:
Callback(const NumberObjectConversionChecker * C,BugReporter & BR,AnalysisDeclContext * ADC)57 Callback(const NumberObjectConversionChecker *C,
58 BugReporter &BR, AnalysisDeclContext *ADC)
59 : C(C), BR(BR), ADC(ADC) {}
60 void run(const MatchFinder::MatchResult &Result) override;
61 };
62 } // end of anonymous namespace
63
run(const MatchFinder::MatchResult & Result)64 void Callback::run(const MatchFinder::MatchResult &Result) {
65 bool IsPedanticMatch =
66 (Result.Nodes.getNodeAs<Stmt>("pedantic") != nullptr);
67 if (IsPedanticMatch && !C->Pedantic)
68 return;
69
70 ASTContext &ACtx = ADC->getASTContext();
71
72 if (const Expr *CheckIfNull =
73 Result.Nodes.getNodeAs<Expr>("check_if_null")) {
74 // Unless the macro indicates that the intended type is clearly not
75 // a pointer type, we should avoid warning on comparing pointers
76 // to zero literals in non-pedantic mode.
77 // FIXME: Introduce an AST matcher to implement the macro-related logic?
78 bool MacroIndicatesWeShouldSkipTheCheck = false;
79 SourceLocation Loc = CheckIfNull->getBeginLoc();
80 if (Loc.isMacroID()) {
81 StringRef MacroName = Lexer::getImmediateMacroName(
82 Loc, ACtx.getSourceManager(), ACtx.getLangOpts());
83 if (MacroName == "NULL" || MacroName == "nil")
84 return;
85 if (MacroName == "YES" || MacroName == "NO")
86 MacroIndicatesWeShouldSkipTheCheck = true;
87 }
88 if (!MacroIndicatesWeShouldSkipTheCheck) {
89 Expr::EvalResult EVResult;
90 if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt(
91 EVResult, ACtx, Expr::SE_AllowSideEffects)) {
92 llvm::APSInt Result = EVResult.Val.getInt();
93 if (Result == 0) {
94 if (!C->Pedantic)
95 return;
96 IsPedanticMatch = true;
97 }
98 }
99 }
100 }
101
102 const Stmt *Conv = Result.Nodes.getNodeAs<Stmt>("conv");
103 assert(Conv);
104
105 const Expr *ConvertedCObject = Result.Nodes.getNodeAs<Expr>("c_object");
106 const Expr *ConvertedCppObject = Result.Nodes.getNodeAs<Expr>("cpp_object");
107 const Expr *ConvertedObjCObject = Result.Nodes.getNodeAs<Expr>("objc_object");
108 bool IsCpp = (ConvertedCppObject != nullptr);
109 bool IsObjC = (ConvertedObjCObject != nullptr);
110 const Expr *Obj = IsObjC ? ConvertedObjCObject
111 : IsCpp ? ConvertedCppObject
112 : ConvertedCObject;
113 assert(Obj);
114
115 bool IsComparison =
116 (Result.Nodes.getNodeAs<Stmt>("comparison") != nullptr);
117
118 bool IsOSNumber =
119 (Result.Nodes.getNodeAs<Decl>("osnumber") != nullptr);
120
121 bool IsInteger =
122 (Result.Nodes.getNodeAs<QualType>("int_type") != nullptr);
123 bool IsObjCBool =
124 (Result.Nodes.getNodeAs<QualType>("objc_bool_type") != nullptr);
125 bool IsCppBool =
126 (Result.Nodes.getNodeAs<QualType>("cpp_bool_type") != nullptr);
127
128 llvm::SmallString<64> Msg;
129 llvm::raw_svector_ostream OS(Msg);
130
131 // Remove ObjC ARC qualifiers.
132 QualType ObjT = Obj->getType().getUnqualifiedType();
133
134 // Remove consts from pointers.
135 if (IsCpp) {
136 assert(ObjT.getCanonicalType()->isPointerType());
137 ObjT = ACtx.getPointerType(
138 ObjT->getPointeeType().getCanonicalType().getUnqualifiedType());
139 }
140
141 if (IsComparison)
142 OS << "Comparing ";
143 else
144 OS << "Converting ";
145
146 OS << "a pointer value of type '" << ObjT << "' to a ";
147
148 std::string EuphemismForPlain = "primitive";
149 std::string SuggestedApi = IsObjC ? (IsInteger ? "" : "-boolValue")
150 : IsCpp ? (IsOSNumber ? "" : "getValue()")
151 : "CFNumberGetValue()";
152 if (SuggestedApi.empty()) {
153 // A generic message if we're not sure what API should be called.
154 // FIXME: Pattern-match the integer type to make a better guess?
155 SuggestedApi =
156 "a method on '" + ObjT.getAsString() + "' to get the scalar value";
157 // "scalar" is not quite correct or common, but some documentation uses it
158 // when describing object methods we suggest. For consistency, we use
159 // "scalar" in the whole sentence when we need to use this word in at least
160 // one place, otherwise we use "primitive".
161 EuphemismForPlain = "scalar";
162 }
163
164 if (IsInteger)
165 OS << EuphemismForPlain << " integer value";
166 else if (IsObjCBool)
167 OS << EuphemismForPlain << " BOOL value";
168 else if (IsCppBool)
169 OS << EuphemismForPlain << " bool value";
170 else // Branch condition?
171 OS << EuphemismForPlain << " boolean value";
172
173
174 if (IsPedanticMatch)
175 OS << "; instead, either compare the pointer to "
176 << (IsObjC ? "nil" : IsCpp ? "nullptr" : "NULL") << " or ";
177 else
178 OS << "; did you mean to ";
179
180 if (IsComparison)
181 OS << "compare the result of calling " << SuggestedApi;
182 else
183 OS << "call " << SuggestedApi;
184
185 if (!IsPedanticMatch)
186 OS << "?";
187
188 BR.EmitBasicReport(
189 ADC->getDecl(), C, "Suspicious number object conversion", "Logic error",
190 OS.str(),
191 PathDiagnosticLocation::createBegin(Obj, BR.getSourceManager(), ADC),
192 Conv->getSourceRange());
193 }
194
checkASTCodeBody(const Decl * D,AnalysisManager & AM,BugReporter & BR) const195 void NumberObjectConversionChecker::checkASTCodeBody(const Decl *D,
196 AnalysisManager &AM,
197 BugReporter &BR) const {
198 // Currently this matches CoreFoundation opaque pointer typedefs.
199 auto CSuspiciousNumberObjectExprM = expr(ignoringParenImpCasts(
200 expr(hasType(elaboratedType(namesType(typedefType(
201 hasDeclaration(anyOf(typedefDecl(hasName("CFNumberRef")),
202 typedefDecl(hasName("CFBooleanRef")))))))))
203 .bind("c_object")));
204
205 // Currently this matches XNU kernel number-object pointers.
206 auto CppSuspiciousNumberObjectExprM =
207 expr(ignoringParenImpCasts(
208 expr(hasType(hasCanonicalType(
209 pointerType(pointee(hasCanonicalType(
210 recordType(hasDeclaration(
211 anyOf(
212 cxxRecordDecl(hasName("OSBoolean")),
213 cxxRecordDecl(hasName("OSNumber"))
214 .bind("osnumber"))))))))))
215 .bind("cpp_object")));
216
217 // Currently this matches NeXTSTEP number objects.
218 auto ObjCSuspiciousNumberObjectExprM =
219 expr(ignoringParenImpCasts(
220 expr(hasType(hasCanonicalType(
221 objcObjectPointerType(pointee(
222 qualType(hasCanonicalType(
223 qualType(hasDeclaration(
224 objcInterfaceDecl(hasName("NSNumber")))))))))))
225 .bind("objc_object")));
226
227 auto SuspiciousNumberObjectExprM = anyOf(
228 CSuspiciousNumberObjectExprM,
229 CppSuspiciousNumberObjectExprM,
230 ObjCSuspiciousNumberObjectExprM);
231
232 // Useful for predicates like "Unless we've seen the same object elsewhere".
233 auto AnotherSuspiciousNumberObjectExprM =
234 expr(anyOf(
235 equalsBoundNode("c_object"),
236 equalsBoundNode("objc_object"),
237 equalsBoundNode("cpp_object")));
238
239 // The .bind here is in order to compose the error message more accurately.
240 auto ObjCSuspiciousScalarBooleanTypeM =
241 qualType(elaboratedType(namesType(
242 typedefType(hasDeclaration(typedefDecl(hasName("BOOL")))))))
243 .bind("objc_bool_type");
244
245 // The .bind here is in order to compose the error message more accurately.
246 auto SuspiciousScalarBooleanTypeM =
247 qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"),
248 ObjCSuspiciousScalarBooleanTypeM));
249
250 // The .bind here is in order to compose the error message more accurately.
251 // Also avoid intptr_t and uintptr_t because they were specifically created
252 // for storing pointers.
253 auto SuspiciousScalarNumberTypeM =
254 qualType(hasCanonicalType(isInteger()),
255 unless(elaboratedType(namesType(typedefType(hasDeclaration(
256 typedefDecl(matchesName("^::u?intptr_t$"))))))))
257 .bind("int_type");
258
259 auto SuspiciousScalarTypeM =
260 qualType(anyOf(SuspiciousScalarBooleanTypeM,
261 SuspiciousScalarNumberTypeM));
262
263 auto SuspiciousScalarExprM =
264 expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM))));
265
266 auto ConversionThroughAssignmentM =
267 binaryOperator(allOf(hasOperatorName("="),
268 hasLHS(SuspiciousScalarExprM),
269 hasRHS(SuspiciousNumberObjectExprM)));
270
271 auto ConversionThroughBranchingM =
272 ifStmt(allOf(
273 hasCondition(SuspiciousNumberObjectExprM),
274 unless(hasConditionVariableStatement(declStmt())
275 ))).bind("pedantic");
276
277 auto ConversionThroughCallM =
278 callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),
279 ignoringParenImpCasts(
280 SuspiciousNumberObjectExprM))));
281
282 // We bind "check_if_null" to modify the warning message
283 // in case it was intended to compare a pointer to 0 with a relatively-ok
284 // construct "x == 0" or "x != 0".
285 auto ConversionThroughEquivalenceM =
286 binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
287 hasEitherOperand(SuspiciousNumberObjectExprM),
288 hasEitherOperand(SuspiciousScalarExprM
289 .bind("check_if_null"))))
290 .bind("comparison");
291
292 auto ConversionThroughComparisonM =
293 binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"),
294 hasOperatorName("<="), hasOperatorName("<")),
295 hasEitherOperand(SuspiciousNumberObjectExprM),
296 hasEitherOperand(SuspiciousScalarExprM)))
297 .bind("comparison");
298
299 auto ConversionThroughConditionalOperatorM =
300 conditionalOperator(allOf(
301 hasCondition(SuspiciousNumberObjectExprM),
302 unless(hasTrueExpression(
303 hasDescendant(AnotherSuspiciousNumberObjectExprM))),
304 unless(hasFalseExpression(
305 hasDescendant(AnotherSuspiciousNumberObjectExprM)))))
306 .bind("pedantic");
307
308 auto ConversionThroughExclamationMarkM =
309 unaryOperator(allOf(hasOperatorName("!"),
310 has(expr(SuspiciousNumberObjectExprM))))
311 .bind("pedantic");
312
313 auto ConversionThroughExplicitBooleanCastM =
314 explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM),
315 has(expr(SuspiciousNumberObjectExprM))));
316
317 auto ConversionThroughExplicitNumberCastM =
318 explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM),
319 has(expr(SuspiciousNumberObjectExprM))));
320
321 auto ConversionThroughInitializerM =
322 declStmt(hasSingleDecl(
323 varDecl(hasType(SuspiciousScalarTypeM),
324 hasInitializer(SuspiciousNumberObjectExprM))));
325
326 auto FinalM = stmt(anyOf(ConversionThroughAssignmentM,
327 ConversionThroughBranchingM,
328 ConversionThroughCallM,
329 ConversionThroughComparisonM,
330 ConversionThroughConditionalOperatorM,
331 ConversionThroughEquivalenceM,
332 ConversionThroughExclamationMarkM,
333 ConversionThroughExplicitBooleanCastM,
334 ConversionThroughExplicitNumberCastM,
335 ConversionThroughInitializerM)).bind("conv");
336
337 MatchFinder F;
338 Callback CB(this, BR, AM.getAnalysisDeclContext(D));
339
340 F.addMatcher(traverse(TK_AsIs, stmt(forEachDescendant(FinalM))), &CB);
341 F.match(*D->getBody(), AM.getASTContext());
342 }
343
registerNumberObjectConversionChecker(CheckerManager & Mgr)344 void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
345 NumberObjectConversionChecker *Chk =
346 Mgr.registerChecker<NumberObjectConversionChecker>();
347 Chk->Pedantic =
348 Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic");
349 }
350
shouldRegisterNumberObjectConversionChecker(const CheckerManager & mgr)351 bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) {
352 return true;
353 }
354