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 37 using namespace clang; 38 using namespace ento; 39 using namespace ast_matchers; 40 41 namespace { 42 43 class NumberObjectConversionChecker : public Checker<check::ASTCodeBody> { 44 public: 45 bool Pedantic; 46 47 void checkASTCodeBody(const Decl *D, AnalysisManager &AM, 48 BugReporter &BR) const; 49 }; 50 51 class Callback : public MatchFinder::MatchCallback { 52 const NumberObjectConversionChecker *C; 53 BugReporter &BR; 54 AnalysisDeclContext *ADC; 55 56 public: 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 64 void 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 195 void 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 345 void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) { 346 NumberObjectConversionChecker *Chk = 347 Mgr.registerChecker<NumberObjectConversionChecker>(); 348 Chk->Pedantic = 349 Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic"); 350 } 351 352 bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) { 353 return true; 354 } 355