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
37using namespace clang;
38using namespace ento;
39using namespace ast_matchers;
40
41namespace {
42
43class NumberObjectConversionChecker : public Checker<check::ASTCodeBody> {
44public:
45 bool Pedantic;
46
47 void checkASTCodeBody(const Decl *D, AnalysisManager &AM,
48 BugReporter &BR) const;
49};
50
51class Callback : public MatchFinder::MatchCallback {
52 const NumberObjectConversionChecker *C;
53 BugReporter &BR;
54 AnalysisDeclContext *ADC;
55
56public:
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
64void Callback::run(const MatchFinder::MatchResult &Result) {
65 bool IsPedanticMatch =
66 (Result.Nodes.getNodeAs<Stmt>(ID: "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>(ID: "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, SM: ACtx.getSourceManager(), LangOpts: 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 Result&: EVResult, Ctx: ACtx, AllowSideEffects: 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>(ID: "conv");
103 assert(Conv);
104
105 const Expr *ConvertedCObject = Result.Nodes.getNodeAs<Expr>(ID: "c_object");
106 const Expr *ConvertedCppObject = Result.Nodes.getNodeAs<Expr>(ID: "cpp_object");
107 const Expr *ConvertedObjCObject = Result.Nodes.getNodeAs<Expr>(ID: "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>(ID: "comparison") != nullptr);
117
118 bool IsOSNumber =
119 (Result.Nodes.getNodeAs<Decl>(ID: "osnumber") != nullptr);
120
121 bool IsInteger =
122 (Result.Nodes.getNodeAs<QualType>(ID: "int_type") != nullptr);
123 bool IsObjCBool =
124 (Result.Nodes.getNodeAs<QualType>(ID: "objc_bool_type") != nullptr);
125 bool IsCppBool =
126 (Result.Nodes.getNodeAs<QualType>(ID: "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 T: 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 DeclWithIssue: ADC->getDecl(), Checker: C, BugName: "Suspicious number object conversion", BugCategory: "Logic error",
190 BugStr: OS.str(),
191 Loc: PathDiagnosticLocation::createBegin(S: Obj, SM: BR.getSourceManager(), LAC: ADC),
192 Ranges: Conv->getSourceRange());
193}
194
195void 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 InnerMatcher: expr(hasType(InnerMatcher: elaboratedType(namesType(InnerMatcher: typedefType(
201 hasDeclaration(InnerMatcher: anyOf(typedefDecl(hasName(Name: "CFNumberRef")),
202 typedefDecl(hasName(Name: "CFBooleanRef")))))))))
203 .bind(ID: "c_object")));
204
205 // Currently this matches XNU kernel number-object pointers.
206 auto CppSuspiciousNumberObjectExprM =
207 expr(ignoringParenImpCasts(
208 InnerMatcher: expr(hasType(InnerMatcher: hasCanonicalType(
209 InnerMatcher: pointerType(pointee(hasCanonicalType(
210 InnerMatcher: recordType(hasDeclaration(
211 InnerMatcher: anyOf(
212 cxxRecordDecl(hasName(Name: "OSBoolean")),
213 cxxRecordDecl(hasName(Name: "OSNumber"))
214 .bind(ID: "osnumber"))))))))))
215 .bind(ID: "cpp_object")));
216
217 // Currently this matches NeXTSTEP number objects.
218 auto ObjCSuspiciousNumberObjectExprM =
219 expr(ignoringParenImpCasts(
220 InnerMatcher: expr(hasType(InnerMatcher: hasCanonicalType(
221 InnerMatcher: objcObjectPointerType(pointee(
222 qualType(hasCanonicalType(
223 InnerMatcher: qualType(hasDeclaration(
224 InnerMatcher: objcInterfaceDecl(hasName(Name: "NSNumber")))))))))))
225 .bind(ID: "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(ID: "c_object"),
236 equalsBoundNode(ID: "objc_object"),
237 equalsBoundNode(ID: "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 InnerMatcher: typedefType(hasDeclaration(InnerMatcher: typedefDecl(hasName(Name: "BOOL")))))))
243 .bind(ID: "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(ID: "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(InnerMatcher: isInteger()),
255 unless(elaboratedType(namesType(InnerMatcher: typedefType(hasDeclaration(
256 InnerMatcher: typedefDecl(matchesName(RegExp: "^::u?intptr_t$"))))))))
257 .bind(ID: "int_type");
258
259 auto SuspiciousScalarTypeM =
260 qualType(anyOf(SuspiciousScalarBooleanTypeM,
261 SuspiciousScalarNumberTypeM));
262
263 auto SuspiciousScalarExprM =
264 expr(ignoringParenImpCasts(InnerMatcher: expr(hasType(InnerMatcher: SuspiciousScalarTypeM))));
265
266 auto ConversionThroughAssignmentM =
267 binaryOperator(allOf(hasOperatorName(Name: "="),
268 hasLHS(InnerMatcher: SuspiciousScalarExprM),
269 hasRHS(InnerMatcher: SuspiciousNumberObjectExprM)));
270
271 auto ConversionThroughBranchingM =
272 ifStmt(allOf(
273 hasCondition(InnerMatcher: SuspiciousNumberObjectExprM),
274 unless(hasConditionVariableStatement(InnerMatcher: declStmt())
275 ))).bind(ID: "pedantic");
276
277 auto ConversionThroughCallM =
278 callExpr(hasAnyArgument(InnerMatcher: allOf(hasType(InnerMatcher: SuspiciousScalarTypeM),
279 ignoringParenImpCasts(
280 InnerMatcher: 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(Name: "=="), hasOperatorName(Name: "!=")),
287 hasEitherOperand(InnerMatcher: SuspiciousNumberObjectExprM),
288 hasEitherOperand(InnerMatcher: SuspiciousScalarExprM
289 .bind(ID: "check_if_null"))))
290 .bind(ID: "comparison");
291
292 auto ConversionThroughComparisonM =
293 binaryOperator(allOf(anyOf(hasOperatorName(Name: ">="), hasOperatorName(Name: ">"),
294 hasOperatorName(Name: "<="), hasOperatorName(Name: "<")),
295 hasEitherOperand(InnerMatcher: SuspiciousNumberObjectExprM),
296 hasEitherOperand(InnerMatcher: SuspiciousScalarExprM)))
297 .bind(ID: "comparison");
298
299 auto ConversionThroughConditionalOperatorM =
300 conditionalOperator(allOf(
301 hasCondition(InnerMatcher: SuspiciousNumberObjectExprM),
302 unless(hasTrueExpression(
303 InnerMatcher: hasDescendant(AnotherSuspiciousNumberObjectExprM))),
304 unless(hasFalseExpression(
305 InnerMatcher: hasDescendant(AnotherSuspiciousNumberObjectExprM)))))
306 .bind(ID: "pedantic");
307
308 auto ConversionThroughExclamationMarkM =
309 unaryOperator(allOf(hasOperatorName(Name: "!"),
310 has(expr(SuspiciousNumberObjectExprM))))
311 .bind(ID: "pedantic");
312
313 auto ConversionThroughExplicitBooleanCastM =
314 explicitCastExpr(allOf(hasType(InnerMatcher: SuspiciousScalarBooleanTypeM),
315 has(expr(SuspiciousNumberObjectExprM))));
316
317 auto ConversionThroughExplicitNumberCastM =
318 explicitCastExpr(allOf(hasType(InnerMatcher: SuspiciousScalarNumberTypeM),
319 has(expr(SuspiciousNumberObjectExprM))));
320
321 auto ConversionThroughInitializerM =
322 declStmt(hasSingleDecl(
323 InnerMatcher: varDecl(hasType(InnerMatcher: SuspiciousScalarTypeM),
324 hasInitializer(InnerMatcher: 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(ID: "conv");
336
337 MatchFinder F;
338 Callback CB(this, BR, AM.getAnalysisDeclContext(D));
339
340 F.addMatcher(NodeMatch: traverse(TK: TK_AsIs, InnerMatcher: stmt(forEachDescendant(FinalM))), Action: &CB);
341 F.match(Node: *D->getBody(), Context&: AM.getASTContext());
342}
343
344void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
345 NumberObjectConversionChecker *Chk =
346 Mgr.registerChecker<NumberObjectConversionChecker>();
347 Chk->Pedantic =
348 Mgr.getAnalyzerOptions().getCheckerBooleanOption(C: Chk, OptionName: "Pedantic");
349}
350
351bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) {
352 return true;
353}
354