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