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 llvm::APSInt Result; 91 if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt( 92 Result, ACtx, Expr::SE_AllowSideEffects)) { 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(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("Pedantic", false, Chk); 350 } 351