1 //===- NumberObjectConversionChecker.cpp -------------------------*- C++ -*-==// 2 // 3 // The LLVM Compiler Infrastructure 4 // 5 // This file is distributed under the University of Illinois Open Source 6 // License. See LICENSE.TXT for details. 7 // 8 //===----------------------------------------------------------------------===// 9 // 10 // This file defines NumberObjectConversionChecker, which checks for a 11 // particular common mistake when dealing with numbers represented as objects 12 // passed around by pointers. Namely, the language allows to reinterpret the 13 // pointer as a number directly, often without throwing any warnings, 14 // but in most cases the result of such conversion is clearly unexpected, 15 // as pointer value, rather than number value represented by the pointee object, 16 // becomes the result of such operation. 17 // 18 // Currently the checker supports the Objective-C NSNumber class, 19 // and the OSBoolean class found in macOS low-level code; the latter 20 // can only hold boolean values. 21 // 22 // This checker has an option "Pedantic" (boolean), which enables detection of 23 // more conversion patterns (which are most likely more harmless, and therefore 24 // are more likely to produce false positives) - disabled by default, 25 // enabled with `-analyzer-config osx.NumberObjectConversion:Pedantic=true'. 26 // 27 //===----------------------------------------------------------------------===// 28 29 #include "ClangSACheckers.h" 30 #include "clang/ASTMatchers/ASTMatchFinder.h" 31 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" 32 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" 33 #include "clang/StaticAnalyzer/Core/Checker.h" 34 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" 35 #include "clang/Lex/Lexer.h" 36 #include "llvm/ADT/APSInt.h" 37 38 using namespace clang; 39 using namespace ento; 40 using namespace ast_matchers; 41 42 namespace { 43 44 class NumberObjectConversionChecker : public Checker<check::ASTCodeBody> { 45 public: 46 bool Pedantic; 47 48 void checkASTCodeBody(const Decl *D, AnalysisManager &AM, 49 BugReporter &BR) const; 50 }; 51 52 class Callback : public MatchFinder::MatchCallback { 53 const NumberObjectConversionChecker *C; 54 BugReporter &BR; 55 AnalysisDeclContext *ADC; 56 57 public: 58 Callback(const NumberObjectConversionChecker *C, 59 BugReporter &BR, AnalysisDeclContext *ADC) 60 : C(C), BR(BR), ADC(ADC) {} 61 virtual void run(const MatchFinder::MatchResult &Result); 62 }; 63 } // end of anonymous namespace 64 65 void Callback::run(const MatchFinder::MatchResult &Result) { 66 bool IsPedanticMatch = 67 (Result.Nodes.getNodeAs<Stmt>("pedantic") != nullptr); 68 if (IsPedanticMatch && !C->Pedantic) 69 return; 70 71 ASTContext &ACtx = ADC->getASTContext(); 72 73 if (const Expr *CheckIfNull = 74 Result.Nodes.getNodeAs<Expr>("check_if_null")) { 75 // Unless the macro indicates that the intended type is clearly not 76 // a pointer type, we should avoid warning on comparing pointers 77 // to zero literals in non-pedantic mode. 78 // FIXME: Introduce an AST matcher to implement the macro-related logic? 79 bool MacroIndicatesWeShouldSkipTheCheck = false; 80 SourceLocation Loc = CheckIfNull->getBeginLoc(); 81 if (Loc.isMacroID()) { 82 StringRef MacroName = Lexer::getImmediateMacroName( 83 Loc, ACtx.getSourceManager(), ACtx.getLangOpts()); 84 if (MacroName == "NULL" || MacroName == "nil") 85 return; 86 if (MacroName == "YES" || MacroName == "NO") 87 MacroIndicatesWeShouldSkipTheCheck = true; 88 } 89 if (!MacroIndicatesWeShouldSkipTheCheck) { 90 Expr::EvalResult EVResult; 91 if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt( 92 EVResult, ACtx, Expr::SE_AllowSideEffects)) { 93 llvm::APSInt Result = EVResult.Val.getInt(); 94 if (Result == 0) { 95 if (!C->Pedantic) 96 return; 97 IsPedanticMatch = true; 98 } 99 } 100 } 101 } 102 103 const Stmt *Conv = Result.Nodes.getNodeAs<Stmt>("conv"); 104 assert(Conv); 105 106 const Expr *ConvertedCObject = Result.Nodes.getNodeAs<Expr>("c_object"); 107 const Expr *ConvertedCppObject = Result.Nodes.getNodeAs<Expr>("cpp_object"); 108 const Expr *ConvertedObjCObject = Result.Nodes.getNodeAs<Expr>("objc_object"); 109 bool IsCpp = (ConvertedCppObject != nullptr); 110 bool IsObjC = (ConvertedObjCObject != nullptr); 111 const Expr *Obj = IsObjC ? ConvertedObjCObject 112 : IsCpp ? ConvertedCppObject 113 : ConvertedCObject; 114 assert(Obj); 115 116 bool IsComparison = 117 (Result.Nodes.getNodeAs<Stmt>("comparison") != nullptr); 118 119 bool IsOSNumber = 120 (Result.Nodes.getNodeAs<Decl>("osnumber") != nullptr); 121 122 bool IsInteger = 123 (Result.Nodes.getNodeAs<QualType>("int_type") != nullptr); 124 bool IsObjCBool = 125 (Result.Nodes.getNodeAs<QualType>("objc_bool_type") != nullptr); 126 bool IsCppBool = 127 (Result.Nodes.getNodeAs<QualType>("cpp_bool_type") != nullptr); 128 129 llvm::SmallString<64> Msg; 130 llvm::raw_svector_ostream OS(Msg); 131 132 // Remove ObjC ARC qualifiers. 133 QualType ObjT = Obj->getType().getUnqualifiedType(); 134 135 // Remove consts from pointers. 136 if (IsCpp) { 137 assert(ObjT.getCanonicalType()->isPointerType()); 138 ObjT = ACtx.getPointerType( 139 ObjT->getPointeeType().getCanonicalType().getUnqualifiedType()); 140 } 141 142 if (IsComparison) 143 OS << "Comparing "; 144 else 145 OS << "Converting "; 146 147 OS << "a pointer value of type '" << ObjT.getAsString() << "' to a "; 148 149 std::string EuphemismForPlain = "primitive"; 150 std::string SuggestedApi = IsObjC ? (IsInteger ? "" : "-boolValue") 151 : IsCpp ? (IsOSNumber ? "" : "getValue()") 152 : "CFNumberGetValue()"; 153 if (SuggestedApi.empty()) { 154 // A generic message if we're not sure what API should be called. 155 // FIXME: Pattern-match the integer type to make a better guess? 156 SuggestedApi = 157 "a method on '" + ObjT.getAsString() + "' to get the scalar value"; 158 // "scalar" is not quite correct or common, but some documentation uses it 159 // when describing object methods we suggest. For consistency, we use 160 // "scalar" in the whole sentence when we need to use this word in at least 161 // one place, otherwise we use "primitive". 162 EuphemismForPlain = "scalar"; 163 } 164 165 if (IsInteger) 166 OS << EuphemismForPlain << " integer value"; 167 else if (IsObjCBool) 168 OS << EuphemismForPlain << " BOOL value"; 169 else if (IsCppBool) 170 OS << EuphemismForPlain << " bool value"; 171 else // Branch condition? 172 OS << EuphemismForPlain << " boolean value"; 173 174 175 if (IsPedanticMatch) 176 OS << "; instead, either compare the pointer to " 177 << (IsObjC ? "nil" : IsCpp ? "nullptr" : "NULL") << " or "; 178 else 179 OS << "; did you mean to "; 180 181 if (IsComparison) 182 OS << "compare the result of calling " << SuggestedApi; 183 else 184 OS << "call " << SuggestedApi; 185 186 if (!IsPedanticMatch) 187 OS << "?"; 188 189 BR.EmitBasicReport( 190 ADC->getDecl(), C, "Suspicious number object conversion", "Logic error", 191 OS.str(), 192 PathDiagnosticLocation::createBegin(Obj, BR.getSourceManager(), ADC), 193 Conv->getSourceRange()); 194 } 195 196 void NumberObjectConversionChecker::checkASTCodeBody(const Decl *D, 197 AnalysisManager &AM, 198 BugReporter &BR) const { 199 // Currently this matches CoreFoundation opaque pointer typedefs. 200 auto CSuspiciousNumberObjectExprM = 201 expr(ignoringParenImpCasts( 202 expr(hasType( 203 typedefType(hasDeclaration(anyOf( 204 typedefDecl(hasName("CFNumberRef")), 205 typedefDecl(hasName("CFBooleanRef"))))))) 206 .bind("c_object"))); 207 208 // Currently this matches XNU kernel number-object pointers. 209 auto CppSuspiciousNumberObjectExprM = 210 expr(ignoringParenImpCasts( 211 expr(hasType(hasCanonicalType( 212 pointerType(pointee(hasCanonicalType( 213 recordType(hasDeclaration( 214 anyOf( 215 cxxRecordDecl(hasName("OSBoolean")), 216 cxxRecordDecl(hasName("OSNumber")) 217 .bind("osnumber")))))))))) 218 .bind("cpp_object"))); 219 220 // Currently this matches NeXTSTEP number objects. 221 auto ObjCSuspiciousNumberObjectExprM = 222 expr(ignoringParenImpCasts( 223 expr(hasType(hasCanonicalType( 224 objcObjectPointerType(pointee( 225 qualType(hasCanonicalType( 226 qualType(hasDeclaration( 227 objcInterfaceDecl(hasName("NSNumber"))))))))))) 228 .bind("objc_object"))); 229 230 auto SuspiciousNumberObjectExprM = anyOf( 231 CSuspiciousNumberObjectExprM, 232 CppSuspiciousNumberObjectExprM, 233 ObjCSuspiciousNumberObjectExprM); 234 235 // Useful for predicates like "Unless we've seen the same object elsewhere". 236 auto AnotherSuspiciousNumberObjectExprM = 237 expr(anyOf( 238 equalsBoundNode("c_object"), 239 equalsBoundNode("objc_object"), 240 equalsBoundNode("cpp_object"))); 241 242 // The .bind here is in order to compose the error message more accurately. 243 auto ObjCSuspiciousScalarBooleanTypeM = 244 qualType(typedefType(hasDeclaration( 245 typedefDecl(hasName("BOOL"))))).bind("objc_bool_type"); 246 247 // The .bind here is in order to compose the error message more accurately. 248 auto SuspiciousScalarBooleanTypeM = 249 qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"), 250 ObjCSuspiciousScalarBooleanTypeM)); 251 252 // The .bind here is in order to compose the error message more accurately. 253 // Also avoid intptr_t and uintptr_t because they were specifically created 254 // for storing pointers. 255 auto SuspiciousScalarNumberTypeM = 256 qualType(hasCanonicalType(isInteger()), 257 unless(typedefType(hasDeclaration( 258 typedefDecl(matchesName("^::u?intptr_t$")))))) 259 .bind("int_type"); 260 261 auto SuspiciousScalarTypeM = 262 qualType(anyOf(SuspiciousScalarBooleanTypeM, 263 SuspiciousScalarNumberTypeM)); 264 265 auto SuspiciousScalarExprM = 266 expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM)))); 267 268 auto ConversionThroughAssignmentM = 269 binaryOperator(allOf(hasOperatorName("="), 270 hasLHS(SuspiciousScalarExprM), 271 hasRHS(SuspiciousNumberObjectExprM))); 272 273 auto ConversionThroughBranchingM = 274 ifStmt(allOf( 275 hasCondition(SuspiciousNumberObjectExprM), 276 unless(hasConditionVariableStatement(declStmt()) 277 ))).bind("pedantic"); 278 279 auto ConversionThroughCallM = 280 callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM), 281 ignoringParenImpCasts( 282 SuspiciousNumberObjectExprM)))); 283 284 // We bind "check_if_null" to modify the warning message 285 // in case it was intended to compare a pointer to 0 with a relatively-ok 286 // construct "x == 0" or "x != 0". 287 auto ConversionThroughEquivalenceM = 288 binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")), 289 hasEitherOperand(SuspiciousNumberObjectExprM), 290 hasEitherOperand(SuspiciousScalarExprM 291 .bind("check_if_null")))) 292 .bind("comparison"); 293 294 auto ConversionThroughComparisonM = 295 binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"), 296 hasOperatorName("<="), hasOperatorName("<")), 297 hasEitherOperand(SuspiciousNumberObjectExprM), 298 hasEitherOperand(SuspiciousScalarExprM))) 299 .bind("comparison"); 300 301 auto ConversionThroughConditionalOperatorM = 302 conditionalOperator(allOf( 303 hasCondition(SuspiciousNumberObjectExprM), 304 unless(hasTrueExpression( 305 hasDescendant(AnotherSuspiciousNumberObjectExprM))), 306 unless(hasFalseExpression( 307 hasDescendant(AnotherSuspiciousNumberObjectExprM))))) 308 .bind("pedantic"); 309 310 auto ConversionThroughExclamationMarkM = 311 unaryOperator(allOf(hasOperatorName("!"), 312 has(expr(SuspiciousNumberObjectExprM)))) 313 .bind("pedantic"); 314 315 auto ConversionThroughExplicitBooleanCastM = 316 explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM), 317 has(expr(SuspiciousNumberObjectExprM)))); 318 319 auto ConversionThroughExplicitNumberCastM = 320 explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM), 321 has(expr(SuspiciousNumberObjectExprM)))); 322 323 auto ConversionThroughInitializerM = 324 declStmt(hasSingleDecl( 325 varDecl(hasType(SuspiciousScalarTypeM), 326 hasInitializer(SuspiciousNumberObjectExprM)))); 327 328 auto FinalM = stmt(anyOf(ConversionThroughAssignmentM, 329 ConversionThroughBranchingM, 330 ConversionThroughCallM, 331 ConversionThroughComparisonM, 332 ConversionThroughConditionalOperatorM, 333 ConversionThroughEquivalenceM, 334 ConversionThroughExclamationMarkM, 335 ConversionThroughExplicitBooleanCastM, 336 ConversionThroughExplicitNumberCastM, 337 ConversionThroughInitializerM)).bind("conv"); 338 339 MatchFinder F; 340 Callback CB(this, BR, AM.getAnalysisDeclContext(D)); 341 342 F.addMatcher(stmt(forEachDescendant(FinalM)), &CB); 343 F.match(*D->getBody(), AM.getASTContext()); 344 } 345 346 void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) { 347 NumberObjectConversionChecker *Chk = 348 Mgr.registerChecker<NumberObjectConversionChecker>(); 349 Chk->Pedantic = 350 Mgr.getAnalyzerOptions().getCheckerBooleanOption("Pedantic", false, Chk); 351 } 352