NumberObjectConversionChecker.cpp 14 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354
  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.getAsString() << "' 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 =
  172. expr(ignoringParenImpCasts(
  173. expr(hasType(
  174. typedefType(hasDeclaration(anyOf(
  175. typedefDecl(hasName("CFNumberRef")),
  176. typedefDecl(hasName("CFBooleanRef")))))))
  177. .bind("c_object")));
  178. // Currently this matches XNU kernel number-object pointers.
  179. auto CppSuspiciousNumberObjectExprM =
  180. expr(ignoringParenImpCasts(
  181. expr(hasType(hasCanonicalType(
  182. pointerType(pointee(hasCanonicalType(
  183. recordType(hasDeclaration(
  184. anyOf(
  185. cxxRecordDecl(hasName("OSBoolean")),
  186. cxxRecordDecl(hasName("OSNumber"))
  187. .bind("osnumber"))))))))))
  188. .bind("cpp_object")));
  189. // Currently this matches NeXTSTEP number objects.
  190. auto ObjCSuspiciousNumberObjectExprM =
  191. expr(ignoringParenImpCasts(
  192. expr(hasType(hasCanonicalType(
  193. objcObjectPointerType(pointee(
  194. qualType(hasCanonicalType(
  195. qualType(hasDeclaration(
  196. objcInterfaceDecl(hasName("NSNumber")))))))))))
  197. .bind("objc_object")));
  198. auto SuspiciousNumberObjectExprM = anyOf(
  199. CSuspiciousNumberObjectExprM,
  200. CppSuspiciousNumberObjectExprM,
  201. ObjCSuspiciousNumberObjectExprM);
  202. // Useful for predicates like "Unless we've seen the same object elsewhere".
  203. auto AnotherSuspiciousNumberObjectExprM =
  204. expr(anyOf(
  205. equalsBoundNode("c_object"),
  206. equalsBoundNode("objc_object"),
  207. equalsBoundNode("cpp_object")));
  208. // The .bind here is in order to compose the error message more accurately.
  209. auto ObjCSuspiciousScalarBooleanTypeM =
  210. qualType(typedefType(hasDeclaration(
  211. typedefDecl(hasName("BOOL"))))).bind("objc_bool_type");
  212. // The .bind here is in order to compose the error message more accurately.
  213. auto SuspiciousScalarBooleanTypeM =
  214. qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"),
  215. ObjCSuspiciousScalarBooleanTypeM));
  216. // The .bind here is in order to compose the error message more accurately.
  217. // Also avoid intptr_t and uintptr_t because they were specifically created
  218. // for storing pointers.
  219. auto SuspiciousScalarNumberTypeM =
  220. qualType(hasCanonicalType(isInteger()),
  221. unless(typedefType(hasDeclaration(
  222. typedefDecl(matchesName("^::u?intptr_t$"))))))
  223. .bind("int_type");
  224. auto SuspiciousScalarTypeM =
  225. qualType(anyOf(SuspiciousScalarBooleanTypeM,
  226. SuspiciousScalarNumberTypeM));
  227. auto SuspiciousScalarExprM =
  228. expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM))));
  229. auto ConversionThroughAssignmentM =
  230. binaryOperator(allOf(hasOperatorName("="),
  231. hasLHS(SuspiciousScalarExprM),
  232. hasRHS(SuspiciousNumberObjectExprM)));
  233. auto ConversionThroughBranchingM =
  234. ifStmt(allOf(
  235. hasCondition(SuspiciousNumberObjectExprM),
  236. unless(hasConditionVariableStatement(declStmt())
  237. ))).bind("pedantic");
  238. auto ConversionThroughCallM =
  239. callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),
  240. ignoringParenImpCasts(
  241. SuspiciousNumberObjectExprM))));
  242. // We bind "check_if_null" to modify the warning message
  243. // in case it was intended to compare a pointer to 0 with a relatively-ok
  244. // construct "x == 0" or "x != 0".
  245. auto ConversionThroughEquivalenceM =
  246. binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
  247. hasEitherOperand(SuspiciousNumberObjectExprM),
  248. hasEitherOperand(SuspiciousScalarExprM
  249. .bind("check_if_null"))))
  250. .bind("comparison");
  251. auto ConversionThroughComparisonM =
  252. binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"),
  253. hasOperatorName("<="), hasOperatorName("<")),
  254. hasEitherOperand(SuspiciousNumberObjectExprM),
  255. hasEitherOperand(SuspiciousScalarExprM)))
  256. .bind("comparison");
  257. auto ConversionThroughConditionalOperatorM =
  258. conditionalOperator(allOf(
  259. hasCondition(SuspiciousNumberObjectExprM),
  260. unless(hasTrueExpression(
  261. hasDescendant(AnotherSuspiciousNumberObjectExprM))),
  262. unless(hasFalseExpression(
  263. hasDescendant(AnotherSuspiciousNumberObjectExprM)))))
  264. .bind("pedantic");
  265. auto ConversionThroughExclamationMarkM =
  266. unaryOperator(allOf(hasOperatorName("!"),
  267. has(expr(SuspiciousNumberObjectExprM))))
  268. .bind("pedantic");
  269. auto ConversionThroughExplicitBooleanCastM =
  270. explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM),
  271. has(expr(SuspiciousNumberObjectExprM))));
  272. auto ConversionThroughExplicitNumberCastM =
  273. explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM),
  274. has(expr(SuspiciousNumberObjectExprM))));
  275. auto ConversionThroughInitializerM =
  276. declStmt(hasSingleDecl(
  277. varDecl(hasType(SuspiciousScalarTypeM),
  278. hasInitializer(SuspiciousNumberObjectExprM))));
  279. auto FinalM = stmt(anyOf(ConversionThroughAssignmentM,
  280. ConversionThroughBranchingM,
  281. ConversionThroughCallM,
  282. ConversionThroughComparisonM,
  283. ConversionThroughConditionalOperatorM,
  284. ConversionThroughEquivalenceM,
  285. ConversionThroughExclamationMarkM,
  286. ConversionThroughExplicitBooleanCastM,
  287. ConversionThroughExplicitNumberCastM,
  288. ConversionThroughInitializerM)).bind("conv");
  289. MatchFinder F;
  290. Callback CB(this, BR, AM.getAnalysisDeclContext(D));
  291. F.addMatcher(traverse(TK_AsIs, stmt(forEachDescendant(FinalM))), &CB);
  292. F.match(*D->getBody(), AM.getASTContext());
  293. }
  294. void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
  295. NumberObjectConversionChecker *Chk =
  296. Mgr.registerChecker<NumberObjectConversionChecker>();
  297. Chk->Pedantic =
  298. Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic");
  299. }
  300. bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) {
  301. return true;
  302. }