1 //===- NumberObjectConversionChecker.cpp -------------------------*- C++ -*-==//
3 // The LLVM Compiler Infrastructure
5 // This file is distributed under the University of Illinois Open Source
6 // License. See LICENSE.TXT for details.
8 //===----------------------------------------------------------------------===//
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.
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.
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'.
27 //===----------------------------------------------------------------------===//
29 #include "ClangSACheckers.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"
38 using namespace clang;
40 using namespace ast_matchers;
44 class NumberObjectConversionChecker : public Checker<check::ASTCodeBody> {
48 void checkASTCodeBody(const Decl *D, AnalysisManager &AM,
49 BugReporter &BR) const;
52 class Callback : public MatchFinder::MatchCallback {
53 const NumberObjectConversionChecker *C;
55 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);
63 } // end of anonymous namespace
65 void Callback::run(const MatchFinder::MatchResult &Result) {
66 bool IsPedanticMatch =
67 (Result.Nodes.getNodeAs<Stmt>("pedantic") != nullptr);
68 if (IsPedanticMatch && !C->Pedantic)
71 ASTContext &ACtx = ADC->getASTContext();
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->getLocStart();
81 if (Loc.isMacroID()) {
82 StringRef MacroName = Lexer::getImmediateMacroName(
83 Loc, ACtx.getSourceManager(), ACtx.getLangOpts());
84 if (MacroName == "NULL" || MacroName == "nil")
86 if (MacroName == "YES" || MacroName == "NO")
87 MacroIndicatesWeShouldSkipTheCheck = true;
89 if (!MacroIndicatesWeShouldSkipTheCheck) {
91 if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt(
92 Result, ACtx, Expr::SE_AllowSideEffects)) {
96 IsPedanticMatch = true;
102 const Stmt *Conv = Result.Nodes.getNodeAs<Stmt>("conv");
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
116 (Result.Nodes.getNodeAs<Stmt>("comparison") != nullptr);
119 (Result.Nodes.getNodeAs<Decl>("osnumber") != nullptr);
122 (Result.Nodes.getNodeAs<QualType>("int_type") != nullptr);
124 (Result.Nodes.getNodeAs<QualType>("objc_bool_type") != nullptr);
126 (Result.Nodes.getNodeAs<QualType>("cpp_bool_type") != nullptr);
128 llvm::SmallString<64> Msg;
129 llvm::raw_svector_ostream OS(Msg);
131 // Remove ObjC ARC qualifiers.
132 QualType ObjT = Obj->getType().getUnqualifiedType();
134 // Remove consts from pointers.
136 assert(ObjT.getCanonicalType()->isPointerType());
137 ObjT = ACtx.getPointerType(
138 ObjT->getPointeeType().getCanonicalType().getUnqualifiedType());
146 OS << "a pointer value of type '" << ObjT.getAsString() << "' to a ";
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?
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";
165 OS << EuphemismForPlain << " integer value";
167 OS << EuphemismForPlain << " BOOL value";
169 OS << EuphemismForPlain << " bool value";
170 else // Branch condition?
171 OS << EuphemismForPlain << " boolean value";
175 OS << "; instead, either compare the pointer to "
176 << (IsObjC ? "nil" : IsCpp ? "nullptr" : "NULL") << " or ";
178 OS << "; did you mean to ";
181 OS << "compare the result of calling " << SuggestedApi;
183 OS << "call " << SuggestedApi;
185 if (!IsPedanticMatch)
189 ADC->getDecl(), C, "Suspicious number object conversion", "Logic error",
191 PathDiagnosticLocation::createBegin(Obj, BR.getSourceManager(), ADC),
192 Conv->getSourceRange());
195 void NumberObjectConversionChecker::checkASTCodeBody(const Decl *D,
197 BugReporter &BR) const {
198 // Currently this matches CoreFoundation opaque pointer typedefs.
199 auto CSuspiciousNumberObjectExprM =
200 expr(ignoringParenImpCasts(
202 typedefType(hasDeclaration(anyOf(
203 typedefDecl(hasName("CFNumberRef")),
204 typedefDecl(hasName("CFBooleanRef")))))))
207 // Currently this matches XNU kernel number-object pointers.
208 auto CppSuspiciousNumberObjectExprM =
209 expr(ignoringParenImpCasts(
210 expr(hasType(hasCanonicalType(
211 pointerType(pointee(hasCanonicalType(
212 recordType(hasDeclaration(
214 cxxRecordDecl(hasName("OSBoolean")),
215 cxxRecordDecl(hasName("OSNumber"))
216 .bind("osnumber"))))))))))
217 .bind("cpp_object")));
219 // Currently this matches NeXTSTEP number objects.
220 auto ObjCSuspiciousNumberObjectExprM =
221 expr(ignoringParenImpCasts(
222 expr(hasType(hasCanonicalType(
223 objcObjectPointerType(pointee(
224 qualType(hasCanonicalType(
225 qualType(hasDeclaration(
226 objcInterfaceDecl(hasName("NSNumber")))))))))))
227 .bind("objc_object")));
229 auto SuspiciousNumberObjectExprM = anyOf(
230 CSuspiciousNumberObjectExprM,
231 CppSuspiciousNumberObjectExprM,
232 ObjCSuspiciousNumberObjectExprM);
234 // Useful for predicates like "Unless we've seen the same object elsewhere".
235 auto AnotherSuspiciousNumberObjectExprM =
237 equalsBoundNode("c_object"),
238 equalsBoundNode("objc_object"),
239 equalsBoundNode("cpp_object")));
241 // The .bind here is in order to compose the error message more accurately.
242 auto ObjCSuspiciousScalarBooleanTypeM =
243 qualType(typedefType(hasDeclaration(
244 typedefDecl(hasName("BOOL"))))).bind("objc_bool_type");
246 // The .bind here is in order to compose the error message more accurately.
247 auto SuspiciousScalarBooleanTypeM =
248 qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"),
249 ObjCSuspiciousScalarBooleanTypeM));
251 // The .bind here is in order to compose the error message more accurately.
252 // Also avoid intptr_t and uintptr_t because they were specifically created
253 // for storing pointers.
254 auto SuspiciousScalarNumberTypeM =
255 qualType(hasCanonicalType(isInteger()),
256 unless(typedefType(hasDeclaration(
257 typedefDecl(matchesName("^::u?intptr_t$"))))))
260 auto SuspiciousScalarTypeM =
261 qualType(anyOf(SuspiciousScalarBooleanTypeM,
262 SuspiciousScalarNumberTypeM));
264 auto SuspiciousScalarExprM =
265 expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM))));
267 auto ConversionThroughAssignmentM =
268 binaryOperator(allOf(hasOperatorName("="),
269 hasLHS(SuspiciousScalarExprM),
270 hasRHS(SuspiciousNumberObjectExprM)));
272 auto ConversionThroughBranchingM =
273 ifStmt(hasCondition(SuspiciousNumberObjectExprM))
276 auto ConversionThroughCallM =
277 callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),
278 ignoringParenImpCasts(
279 SuspiciousNumberObjectExprM))));
281 // We bind "check_if_null" to modify the warning message
282 // in case it was intended to compare a pointer to 0 with a relatively-ok
283 // construct "x == 0" or "x != 0".
284 auto ConversionThroughEquivalenceM =
285 binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
286 hasEitherOperand(SuspiciousNumberObjectExprM),
287 hasEitherOperand(SuspiciousScalarExprM
288 .bind("check_if_null"))))
291 auto ConversionThroughComparisonM =
292 binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"),
293 hasOperatorName("<="), hasOperatorName("<")),
294 hasEitherOperand(SuspiciousNumberObjectExprM),
295 hasEitherOperand(SuspiciousScalarExprM)))
298 auto ConversionThroughConditionalOperatorM =
299 conditionalOperator(allOf(
300 hasCondition(SuspiciousNumberObjectExprM),
301 unless(hasTrueExpression(
302 hasDescendant(AnotherSuspiciousNumberObjectExprM))),
303 unless(hasFalseExpression(
304 hasDescendant(AnotherSuspiciousNumberObjectExprM)))))
307 auto ConversionThroughExclamationMarkM =
308 unaryOperator(allOf(hasOperatorName("!"),
309 has(expr(SuspiciousNumberObjectExprM))))
312 auto ConversionThroughExplicitBooleanCastM =
313 explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM),
314 has(expr(SuspiciousNumberObjectExprM))));
316 auto ConversionThroughExplicitNumberCastM =
317 explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM),
318 has(expr(SuspiciousNumberObjectExprM))));
320 auto ConversionThroughInitializerM =
321 declStmt(hasSingleDecl(
322 varDecl(hasType(SuspiciousScalarTypeM),
323 hasInitializer(SuspiciousNumberObjectExprM))));
325 auto FinalM = stmt(anyOf(ConversionThroughAssignmentM,
326 ConversionThroughBranchingM,
327 ConversionThroughCallM,
328 ConversionThroughComparisonM,
329 ConversionThroughConditionalOperatorM,
330 ConversionThroughEquivalenceM,
331 ConversionThroughExclamationMarkM,
332 ConversionThroughExplicitBooleanCastM,
333 ConversionThroughExplicitNumberCastM,
334 ConversionThroughInitializerM)).bind("conv");
337 Callback CB(this, BR, AM.getAnalysisDeclContext(D));
339 F.addMatcher(stmt(forEachDescendant(FinalM)), &CB);
340 F.match(*D->getBody(), AM.getASTContext());
343 void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
344 NumberObjectConversionChecker *Chk =
345 Mgr.registerChecker<NumberObjectConversionChecker>();
347 Mgr.getAnalyzerOptions().getBooleanOption("Pedantic", false, Chk);