353 lines · cpp
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-exception6//7//===----------------------------------------------------------------------===//8//9// This file defines NumberObjectConversionChecker, which checks for a10// particular common mistake when dealing with numbers represented as objects11// passed around by pointers. Namely, the language allows to reinterpret the12// 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 latter19// can only hold boolean values.20//21// This checker has an option "Pedantic" (boolean), which enables detection of22// more conversion patterns (which are most likely more harmless, and therefore23// 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 namespace63 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 not75 // a pointer type, we should avoid warning on comparing pointers76 // 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 ? ConvertedObjCObject111 : IsCpp ? ConvertedCppObject112 : 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 else144 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 it158 // when describing object methods we suggest. For consistency, we use159 // "scalar" in the whole sentence when we need to use this word in at least160 // 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 else178 OS << "; did you mean to ";179 180 if (IsComparison)181 OS << "compare the result of calling " << SuggestedApi;182 else183 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 = expr(ignoringParenImpCasts(200 expr(hasType(typedefType(201 hasDeclaration(anyOf(typedefDecl(hasName("CFNumberRef")),202 typedefDecl(hasName("CFBooleanRef")))))))203 .bind("c_object")));204 205 // Currently this matches XNU kernel number-object pointers.206 auto CppSuspiciousNumberObjectExprM =207 expr(ignoringParenImpCasts(208 expr(hasType(hasCanonicalType(209 pointerType(pointee(hasCanonicalType(210 recordType(hasDeclaration(211 anyOf(212 cxxRecordDecl(hasName("OSBoolean")),213 cxxRecordDecl(hasName("OSNumber"))214 .bind("osnumber"))))))))))215 .bind("cpp_object")));216 217 // Currently this matches NeXTSTEP number objects.218 auto ObjCSuspiciousNumberObjectExprM =219 expr(ignoringParenImpCasts(220 expr(hasType(hasCanonicalType(221 objcObjectPointerType(pointee(222 qualType(hasCanonicalType(223 qualType(hasDeclaration(224 objcInterfaceDecl(hasName("NSNumber")))))))))))225 .bind("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("c_object"),236 equalsBoundNode("objc_object"),237 equalsBoundNode("cpp_object")));238 239 // The .bind here is in order to compose the error message more accurately.240 auto ObjCSuspiciousScalarBooleanTypeM =241 qualType(typedefType(hasDeclaration(typedefDecl(hasName("BOOL")))))242 .bind("objc_bool_type");243 244 // The .bind here is in order to compose the error message more accurately.245 auto SuspiciousScalarBooleanTypeM =246 qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"),247 ObjCSuspiciousScalarBooleanTypeM));248 249 // The .bind here is in order to compose the error message more accurately.250 // Also avoid intptr_t and uintptr_t because they were specifically created251 // for storing pointers.252 auto SuspiciousScalarNumberTypeM =253 qualType(hasCanonicalType(isInteger()),254 unless(typedefType(255 hasDeclaration(typedefDecl(matchesName("^::u?intptr_t$"))))))256 .bind("int_type");257 258 auto SuspiciousScalarTypeM =259 qualType(anyOf(SuspiciousScalarBooleanTypeM,260 SuspiciousScalarNumberTypeM));261 262 auto SuspiciousScalarExprM =263 expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM))));264 265 auto ConversionThroughAssignmentM =266 binaryOperator(allOf(hasOperatorName("="),267 hasLHS(SuspiciousScalarExprM),268 hasRHS(SuspiciousNumberObjectExprM)));269 270 auto ConversionThroughBranchingM =271 ifStmt(allOf(272 hasCondition(SuspiciousNumberObjectExprM),273 unless(hasConditionVariableStatement(declStmt())274 ))).bind("pedantic");275 276 auto ConversionThroughCallM =277 callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),278 ignoringParenImpCasts(279 SuspiciousNumberObjectExprM))));280 281 // We bind "check_if_null" to modify the warning message282 // in case it was intended to compare a pointer to 0 with a relatively-ok283 // construct "x == 0" or "x != 0".284 auto ConversionThroughEquivalenceM =285 binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")),286 hasEitherOperand(SuspiciousNumberObjectExprM),287 hasEitherOperand(SuspiciousScalarExprM288 .bind("check_if_null"))))289 .bind("comparison");290 291 auto ConversionThroughComparisonM =292 binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"),293 hasOperatorName("<="), hasOperatorName("<")),294 hasEitherOperand(SuspiciousNumberObjectExprM),295 hasEitherOperand(SuspiciousScalarExprM)))296 .bind("comparison");297 298 auto ConversionThroughConditionalOperatorM =299 conditionalOperator(allOf(300 hasCondition(SuspiciousNumberObjectExprM),301 unless(hasTrueExpression(302 hasDescendant(AnotherSuspiciousNumberObjectExprM))),303 unless(hasFalseExpression(304 hasDescendant(AnotherSuspiciousNumberObjectExprM)))))305 .bind("pedantic");306 307 auto ConversionThroughExclamationMarkM =308 unaryOperator(allOf(hasOperatorName("!"),309 has(expr(SuspiciousNumberObjectExprM))))310 .bind("pedantic");311 312 auto ConversionThroughExplicitBooleanCastM =313 explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM),314 has(expr(SuspiciousNumberObjectExprM))));315 316 auto ConversionThroughExplicitNumberCastM =317 explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM),318 has(expr(SuspiciousNumberObjectExprM))));319 320 auto ConversionThroughInitializerM =321 declStmt(hasSingleDecl(322 varDecl(hasType(SuspiciousScalarTypeM),323 hasInitializer(SuspiciousNumberObjectExprM))));324 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");335 336 MatchFinder F;337 Callback CB(this, BR, AM.getAnalysisDeclContext(D));338 339 F.addMatcher(traverse(TK_AsIs, stmt(forEachDescendant(FinalM))), &CB);340 F.match(*D->getBody(), AM.getASTContext());341}342 343void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {344 NumberObjectConversionChecker *Chk =345 Mgr.registerChecker<NumberObjectConversionChecker>();346 Chk->Pedantic =347 Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic");348}349 350bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) {351 return true;352}353