NumberObjectConversionChecker.cpp 14 KB

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