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>("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.getAsString() << "' 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
195void NumberObjectConversionChecker::checkASTCodeBody(const Decl *D,
196                                                     AnalysisManager &AM,
197                                                     BugReporter &BR) const {
198  // Currently this matches CoreFoundation opaque pointer typedefs.
199  auto CSuspiciousNumberObjectExprM =
200      expr(ignoringParenImpCasts(
201          expr(hasType(
202              typedefType(hasDeclaration(anyOf(
203                  typedefDecl(hasName("CFNumberRef")),
204                  typedefDecl(hasName("CFBooleanRef")))))))
205          .bind("c_object")));
206
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(
213                      anyOf(
214                        cxxRecordDecl(hasName("OSBoolean")),
215                        cxxRecordDecl(hasName("OSNumber"))
216                            .bind("osnumber"))))))))))
217          .bind("cpp_object")));
218
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")));
228
229  auto SuspiciousNumberObjectExprM = anyOf(
230      CSuspiciousNumberObjectExprM,
231      CppSuspiciousNumberObjectExprM,
232      ObjCSuspiciousNumberObjectExprM);
233
234  // Useful for predicates like "Unless we've seen the same object elsewhere".
235  auto AnotherSuspiciousNumberObjectExprM =
236      expr(anyOf(
237          equalsBoundNode("c_object"),
238          equalsBoundNode("objc_object"),
239          equalsBoundNode("cpp_object")));
240
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");
245
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));
250
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$"))))))
258      .bind("int_type");
259
260  auto SuspiciousScalarTypeM =
261      qualType(anyOf(SuspiciousScalarBooleanTypeM,
262                     SuspiciousScalarNumberTypeM));
263
264  auto SuspiciousScalarExprM =
265      expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM))));
266
267  auto ConversionThroughAssignmentM =
268      binaryOperator(allOf(hasOperatorName("="),
269                           hasLHS(SuspiciousScalarExprM),
270                           hasRHS(SuspiciousNumberObjectExprM)));
271
272  auto ConversionThroughBranchingM =
273      ifStmt(allOf(
274          hasCondition(SuspiciousNumberObjectExprM),
275          unless(hasConditionVariableStatement(declStmt())
276      ))).bind("pedantic");
277
278  auto ConversionThroughCallM =
279      callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),
280                                    ignoringParenImpCasts(
281                                        SuspiciousNumberObjectExprM))));
282
283  // We bind "check_if_null" to modify the warning message
284  // in case it was intended to compare a pointer to 0 with a relatively-ok
285  // construct "x == 0" or "x != 0".
286  auto ConversionThroughEquivalenceM =
287      binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
288                           hasEitherOperand(SuspiciousNumberObjectExprM),
289                           hasEitherOperand(SuspiciousScalarExprM
290                                            .bind("check_if_null"))))
291      .bind("comparison");
292
293  auto ConversionThroughComparisonM =
294      binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"),
295                                 hasOperatorName("<="), hasOperatorName("<")),
296                           hasEitherOperand(SuspiciousNumberObjectExprM),
297                           hasEitherOperand(SuspiciousScalarExprM)))
298      .bind("comparison");
299
300  auto ConversionThroughConditionalOperatorM =
301      conditionalOperator(allOf(
302          hasCondition(SuspiciousNumberObjectExprM),
303          unless(hasTrueExpression(
304              hasDescendant(AnotherSuspiciousNumberObjectExprM))),
305          unless(hasFalseExpression(
306              hasDescendant(AnotherSuspiciousNumberObjectExprM)))))
307      .bind("pedantic");
308
309  auto ConversionThroughExclamationMarkM =
310      unaryOperator(allOf(hasOperatorName("!"),
311                          has(expr(SuspiciousNumberObjectExprM))))
312      .bind("pedantic");
313
314  auto ConversionThroughExplicitBooleanCastM =
315      explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM),
316                             has(expr(SuspiciousNumberObjectExprM))));
317
318  auto ConversionThroughExplicitNumberCastM =
319      explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM),
320                             has(expr(SuspiciousNumberObjectExprM))));
321
322  auto ConversionThroughInitializerM =
323      declStmt(hasSingleDecl(
324          varDecl(hasType(SuspiciousScalarTypeM),
325                  hasInitializer(SuspiciousNumberObjectExprM))));
326
327  auto FinalM = stmt(anyOf(ConversionThroughAssignmentM,
328                           ConversionThroughBranchingM,
329                           ConversionThroughCallM,
330                           ConversionThroughComparisonM,
331                           ConversionThroughConditionalOperatorM,
332                           ConversionThroughEquivalenceM,
333                           ConversionThroughExclamationMarkM,
334                           ConversionThroughExplicitBooleanCastM,
335                           ConversionThroughExplicitNumberCastM,
336                           ConversionThroughInitializerM)).bind("conv");
337
338  MatchFinder F;
339  Callback CB(this, BR, AM.getAnalysisDeclContext(D));
340
341  F.addMatcher(traverse(TK_AsIs, stmt(forEachDescendant(FinalM))), &CB);
342  F.match(*D->getBody(), AM.getASTContext());
343}
344
345void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
346  NumberObjectConversionChecker *Chk =
347      Mgr.registerChecker<NumberObjectConversionChecker>();
348  Chk->Pedantic =
349      Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic");
350}
351
352bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) {
353  return true;
354}
355