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 << "' 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 = expr(ignoringParenImpCasts( 200 expr(hasType(elaboratedType(namesType(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(elaboratedType(namesType( 242 typedefType(hasDeclaration(typedefDecl(hasName("BOOL"))))))) 243 .bind("objc_bool_type"); 244 245 // The .bind here is in order to compose the error message more accurately. 246 auto SuspiciousScalarBooleanTypeM = 247 qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"), 248 ObjCSuspiciousScalarBooleanTypeM)); 249 250 // The .bind here is in order to compose the error message more accurately. 251 // Also avoid intptr_t and uintptr_t because they were specifically created 252 // for storing pointers. 253 auto SuspiciousScalarNumberTypeM = 254 qualType(hasCanonicalType(isInteger()), 255 unless(elaboratedType(namesType(typedefType(hasDeclaration( 256 typedefDecl(matchesName("^::u?intptr_t$")))))))) 257 .bind("int_type"); 258 259 auto SuspiciousScalarTypeM = 260 qualType(anyOf(SuspiciousScalarBooleanTypeM, 261 SuspiciousScalarNumberTypeM)); 262 263 auto SuspiciousScalarExprM = 264 expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM)))); 265 266 auto ConversionThroughAssignmentM = 267 binaryOperator(allOf(hasOperatorName("="), 268 hasLHS(SuspiciousScalarExprM), 269 hasRHS(SuspiciousNumberObjectExprM))); 270 271 auto ConversionThroughBranchingM = 272 ifStmt(allOf( 273 hasCondition(SuspiciousNumberObjectExprM), 274 unless(hasConditionVariableStatement(declStmt()) 275 ))).bind("pedantic"); 276 277 auto ConversionThroughCallM = 278 callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM), 279 ignoringParenImpCasts( 280 SuspiciousNumberObjectExprM)))); 281 282 // We bind "check_if_null" to modify the warning message 283 // in case it was intended to compare a pointer to 0 with a relatively-ok 284 // construct "x == 0" or "x != 0". 285 auto ConversionThroughEquivalenceM = 286 binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")), 287 hasEitherOperand(SuspiciousNumberObjectExprM), 288 hasEitherOperand(SuspiciousScalarExprM 289 .bind("check_if_null")))) 290 .bind("comparison"); 291 292 auto ConversionThroughComparisonM = 293 binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"), 294 hasOperatorName("<="), hasOperatorName("<")), 295 hasEitherOperand(SuspiciousNumberObjectExprM), 296 hasEitherOperand(SuspiciousScalarExprM))) 297 .bind("comparison"); 298 299 auto ConversionThroughConditionalOperatorM = 300 conditionalOperator(allOf( 301 hasCondition(SuspiciousNumberObjectExprM), 302 unless(hasTrueExpression( 303 hasDescendant(AnotherSuspiciousNumberObjectExprM))), 304 unless(hasFalseExpression( 305 hasDescendant(AnotherSuspiciousNumberObjectExprM))))) 306 .bind("pedantic"); 307 308 auto ConversionThroughExclamationMarkM = 309 unaryOperator(allOf(hasOperatorName("!"), 310 has(expr(SuspiciousNumberObjectExprM)))) 311 .bind("pedantic"); 312 313 auto ConversionThroughExplicitBooleanCastM = 314 explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM), 315 has(expr(SuspiciousNumberObjectExprM)))); 316 317 auto ConversionThroughExplicitNumberCastM = 318 explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM), 319 has(expr(SuspiciousNumberObjectExprM)))); 320 321 auto ConversionThroughInitializerM = 322 declStmt(hasSingleDecl( 323 varDecl(hasType(SuspiciousScalarTypeM), 324 hasInitializer(SuspiciousNumberObjectExprM)))); 325 326 auto FinalM = stmt(anyOf(ConversionThroughAssignmentM, 327 ConversionThroughBranchingM, 328 ConversionThroughCallM, 329 ConversionThroughComparisonM, 330 ConversionThroughConditionalOperatorM, 331 ConversionThroughEquivalenceM, 332 ConversionThroughExclamationMarkM, 333 ConversionThroughExplicitBooleanCastM, 334 ConversionThroughExplicitNumberCastM, 335 ConversionThroughInitializerM)).bind("conv"); 336 337 MatchFinder F; 338 Callback CB(this, BR, AM.getAnalysisDeclContext(D)); 339 340 F.addMatcher(traverse(TK_AsIs, stmt(forEachDescendant(FinalM))), &CB); 341 F.match(*D->getBody(), AM.getASTContext()); 342 } 343 344 void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) { 345 NumberObjectConversionChecker *Chk = 346 Mgr.registerChecker<NumberObjectConversionChecker>(); 347 Chk->Pedantic = 348 Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic"); 349 } 350 351 bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) { 352 return true; 353 } 354