| //===- NumberObjectConversionChecker.cpp -------------------------*- C++ -*-==// |
| // |
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. |
| // See https://llvm.org/LICENSE.txt for license information. |
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception |
| // |
| //===----------------------------------------------------------------------===// |
| // |
| // This file defines NumberObjectConversionChecker, which checks for a |
| // particular common mistake when dealing with numbers represented as objects |
| // passed around by pointers. Namely, the language allows to reinterpret the |
| // pointer as a number directly, often without throwing any warnings, |
| // but in most cases the result of such conversion is clearly unexpected, |
| // as pointer value, rather than number value represented by the pointee object, |
| // becomes the result of such operation. |
| // |
| // Currently the checker supports the Objective-C NSNumber class, |
| // and the OSBoolean class found in macOS low-level code; the latter |
| // can only hold boolean values. |
| // |
| // This checker has an option "Pedantic" (boolean), which enables detection of |
| // more conversion patterns (which are most likely more harmless, and therefore |
| // are more likely to produce false positives) - disabled by default, |
| // enabled with `-analyzer-config osx.NumberObjectConversion:Pedantic=true'. |
| // |
| //===----------------------------------------------------------------------===// |
| |
| #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" |
| #include "clang/ASTMatchers/ASTMatchFinder.h" |
| #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" |
| #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" |
| #include "clang/StaticAnalyzer/Core/Checker.h" |
| #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" |
| #include "clang/Lex/Lexer.h" |
| #include "llvm/ADT/APSInt.h" |
| |
| using namespace clang; |
| using namespace ento; |
| using namespace ast_matchers; |
| |
| namespace { |
| |
| class NumberObjectConversionChecker : public Checker<check::ASTCodeBody> { |
| public: |
| bool Pedantic; |
| |
| void checkASTCodeBody(const Decl *D, AnalysisManager &AM, |
| BugReporter &BR) const; |
| }; |
| |
| class Callback : public MatchFinder::MatchCallback { |
| const NumberObjectConversionChecker *C; |
| BugReporter &BR; |
| AnalysisDeclContext *ADC; |
| |
| public: |
| Callback(const NumberObjectConversionChecker *C, |
| BugReporter &BR, AnalysisDeclContext *ADC) |
| : C(C), BR(BR), ADC(ADC) {} |
| void run(const MatchFinder::MatchResult &Result) override; |
| }; |
| } // end of anonymous namespace |
| |
| void Callback::run(const MatchFinder::MatchResult &Result) { |
| bool IsPedanticMatch = |
| (Result.Nodes.getNodeAs<Stmt>("pedantic") != nullptr); |
| if (IsPedanticMatch && !C->Pedantic) |
| return; |
| |
| ASTContext &ACtx = ADC->getASTContext(); |
| |
| if (const Expr *CheckIfNull = |
| Result.Nodes.getNodeAs<Expr>("check_if_null")) { |
| // Unless the macro indicates that the intended type is clearly not |
| // a pointer type, we should avoid warning on comparing pointers |
| // to zero literals in non-pedantic mode. |
| // FIXME: Introduce an AST matcher to implement the macro-related logic? |
| bool MacroIndicatesWeShouldSkipTheCheck = false; |
| SourceLocation Loc = CheckIfNull->getBeginLoc(); |
| if (Loc.isMacroID()) { |
| StringRef MacroName = Lexer::getImmediateMacroName( |
| Loc, ACtx.getSourceManager(), ACtx.getLangOpts()); |
| if (MacroName == "NULL" || MacroName == "nil") |
| return; |
| if (MacroName == "YES" || MacroName == "NO") |
| MacroIndicatesWeShouldSkipTheCheck = true; |
| } |
| if (!MacroIndicatesWeShouldSkipTheCheck) { |
| Expr::EvalResult EVResult; |
| if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt( |
| EVResult, ACtx, Expr::SE_AllowSideEffects)) { |
| llvm::APSInt Result = EVResult.Val.getInt(); |
| if (Result == 0) { |
| if (!C->Pedantic) |
| return; |
| IsPedanticMatch = true; |
| } |
| } |
| } |
| } |
| |
| const Stmt *Conv = Result.Nodes.getNodeAs<Stmt>("conv"); |
| assert(Conv); |
| |
| const Expr *ConvertedCObject = Result.Nodes.getNodeAs<Expr>("c_object"); |
| const Expr *ConvertedCppObject = Result.Nodes.getNodeAs<Expr>("cpp_object"); |
| const Expr *ConvertedObjCObject = Result.Nodes.getNodeAs<Expr>("objc_object"); |
| bool IsCpp = (ConvertedCppObject != nullptr); |
| bool IsObjC = (ConvertedObjCObject != nullptr); |
| const Expr *Obj = IsObjC ? ConvertedObjCObject |
| : IsCpp ? ConvertedCppObject |
| : ConvertedCObject; |
| assert(Obj); |
| |
| bool IsComparison = |
| (Result.Nodes.getNodeAs<Stmt>("comparison") != nullptr); |
| |
| bool IsOSNumber = |
| (Result.Nodes.getNodeAs<Decl>("osnumber") != nullptr); |
| |
| bool IsInteger = |
| (Result.Nodes.getNodeAs<QualType>("int_type") != nullptr); |
| bool IsObjCBool = |
| (Result.Nodes.getNodeAs<QualType>("objc_bool_type") != nullptr); |
| bool IsCppBool = |
| (Result.Nodes.getNodeAs<QualType>("cpp_bool_type") != nullptr); |
| |
| llvm::SmallString<64> Msg; |
| llvm::raw_svector_ostream OS(Msg); |
| |
| // Remove ObjC ARC qualifiers. |
| QualType ObjT = Obj->getType().getUnqualifiedType(); |
| |
| // Remove consts from pointers. |
| if (IsCpp) { |
| assert(ObjT.getCanonicalType()->isPointerType()); |
| ObjT = ACtx.getPointerType( |
| ObjT->getPointeeType().getCanonicalType().getUnqualifiedType()); |
| } |
| |
| if (IsComparison) |
| OS << "Comparing "; |
| else |
| OS << "Converting "; |
| |
| OS << "a pointer value of type '" << ObjT.getAsString() << "' to a "; |
| |
| std::string EuphemismForPlain = "primitive"; |
| std::string SuggestedApi = IsObjC ? (IsInteger ? "" : "-boolValue") |
| : IsCpp ? (IsOSNumber ? "" : "getValue()") |
| : "CFNumberGetValue()"; |
| if (SuggestedApi.empty()) { |
| // A generic message if we're not sure what API should be called. |
| // FIXME: Pattern-match the integer type to make a better guess? |
| SuggestedApi = |
| "a method on '" + ObjT.getAsString() + "' to get the scalar value"; |
| // "scalar" is not quite correct or common, but some documentation uses it |
| // when describing object methods we suggest. For consistency, we use |
| // "scalar" in the whole sentence when we need to use this word in at least |
| // one place, otherwise we use "primitive". |
| EuphemismForPlain = "scalar"; |
| } |
| |
| if (IsInteger) |
| OS << EuphemismForPlain << " integer value"; |
| else if (IsObjCBool) |
| OS << EuphemismForPlain << " BOOL value"; |
| else if (IsCppBool) |
| OS << EuphemismForPlain << " bool value"; |
| else // Branch condition? |
| OS << EuphemismForPlain << " boolean value"; |
| |
| |
| if (IsPedanticMatch) |
| OS << "; instead, either compare the pointer to " |
| << (IsObjC ? "nil" : IsCpp ? "nullptr" : "NULL") << " or "; |
| else |
| OS << "; did you mean to "; |
| |
| if (IsComparison) |
| OS << "compare the result of calling " << SuggestedApi; |
| else |
| OS << "call " << SuggestedApi; |
| |
| if (!IsPedanticMatch) |
| OS << "?"; |
| |
| BR.EmitBasicReport( |
| ADC->getDecl(), C, "Suspicious number object conversion", "Logic error", |
| OS.str(), |
| PathDiagnosticLocation::createBegin(Obj, BR.getSourceManager(), ADC), |
| Conv->getSourceRange()); |
| } |
| |
| void NumberObjectConversionChecker::checkASTCodeBody(const Decl *D, |
| AnalysisManager &AM, |
| BugReporter &BR) const { |
| // Currently this matches CoreFoundation opaque pointer typedefs. |
| auto CSuspiciousNumberObjectExprM = |
| expr(ignoringParenImpCasts( |
| expr(hasType( |
| typedefType(hasDeclaration(anyOf( |
| typedefDecl(hasName("CFNumberRef")), |
| typedefDecl(hasName("CFBooleanRef"))))))) |
| .bind("c_object"))); |
| |
| // Currently this matches XNU kernel number-object pointers. |
| auto CppSuspiciousNumberObjectExprM = |
| expr(ignoringParenImpCasts( |
| expr(hasType(hasCanonicalType( |
| pointerType(pointee(hasCanonicalType( |
| recordType(hasDeclaration( |
| anyOf( |
| cxxRecordDecl(hasName("OSBoolean")), |
| cxxRecordDecl(hasName("OSNumber")) |
| .bind("osnumber")))))))))) |
| .bind("cpp_object"))); |
| |
| // Currently this matches NeXTSTEP number objects. |
| auto ObjCSuspiciousNumberObjectExprM = |
| expr(ignoringParenImpCasts( |
| expr(hasType(hasCanonicalType( |
| objcObjectPointerType(pointee( |
| qualType(hasCanonicalType( |
| qualType(hasDeclaration( |
| objcInterfaceDecl(hasName("NSNumber"))))))))))) |
| .bind("objc_object"))); |
| |
| auto SuspiciousNumberObjectExprM = anyOf( |
| CSuspiciousNumberObjectExprM, |
| CppSuspiciousNumberObjectExprM, |
| ObjCSuspiciousNumberObjectExprM); |
| |
| // Useful for predicates like "Unless we've seen the same object elsewhere". |
| auto AnotherSuspiciousNumberObjectExprM = |
| expr(anyOf( |
| equalsBoundNode("c_object"), |
| equalsBoundNode("objc_object"), |
| equalsBoundNode("cpp_object"))); |
| |
| // The .bind here is in order to compose the error message more accurately. |
| auto ObjCSuspiciousScalarBooleanTypeM = |
| qualType(typedefType(hasDeclaration( |
| typedefDecl(hasName("BOOL"))))).bind("objc_bool_type"); |
| |
| // The .bind here is in order to compose the error message more accurately. |
| auto SuspiciousScalarBooleanTypeM = |
| qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"), |
| ObjCSuspiciousScalarBooleanTypeM)); |
| |
| // The .bind here is in order to compose the error message more accurately. |
| // Also avoid intptr_t and uintptr_t because they were specifically created |
| // for storing pointers. |
| auto SuspiciousScalarNumberTypeM = |
| qualType(hasCanonicalType(isInteger()), |
| unless(typedefType(hasDeclaration( |
| typedefDecl(matchesName("^::u?intptr_t$")))))) |
| .bind("int_type"); |
| |
| auto SuspiciousScalarTypeM = |
| qualType(anyOf(SuspiciousScalarBooleanTypeM, |
| SuspiciousScalarNumberTypeM)); |
| |
| auto SuspiciousScalarExprM = |
| expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM)))); |
| |
| auto ConversionThroughAssignmentM = |
| binaryOperator(allOf(hasOperatorName("="), |
| hasLHS(SuspiciousScalarExprM), |
| hasRHS(SuspiciousNumberObjectExprM))); |
| |
| auto ConversionThroughBranchingM = |
| ifStmt(allOf( |
| hasCondition(SuspiciousNumberObjectExprM), |
| unless(hasConditionVariableStatement(declStmt()) |
| ))).bind("pedantic"); |
| |
| auto ConversionThroughCallM = |
| callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM), |
| ignoringParenImpCasts( |
| SuspiciousNumberObjectExprM)))); |
| |
| // We bind "check_if_null" to modify the warning message |
| // in case it was intended to compare a pointer to 0 with a relatively-ok |
| // construct "x == 0" or "x != 0". |
| auto ConversionThroughEquivalenceM = |
| binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")), |
| hasEitherOperand(SuspiciousNumberObjectExprM), |
| hasEitherOperand(SuspiciousScalarExprM |
| .bind("check_if_null")))) |
| .bind("comparison"); |
| |
| auto ConversionThroughComparisonM = |
| binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"), |
| hasOperatorName("<="), hasOperatorName("<")), |
| hasEitherOperand(SuspiciousNumberObjectExprM), |
| hasEitherOperand(SuspiciousScalarExprM))) |
| .bind("comparison"); |
| |
| auto ConversionThroughConditionalOperatorM = |
| conditionalOperator(allOf( |
| hasCondition(SuspiciousNumberObjectExprM), |
| unless(hasTrueExpression( |
| hasDescendant(AnotherSuspiciousNumberObjectExprM))), |
| unless(hasFalseExpression( |
| hasDescendant(AnotherSuspiciousNumberObjectExprM))))) |
| .bind("pedantic"); |
| |
| auto ConversionThroughExclamationMarkM = |
| unaryOperator(allOf(hasOperatorName("!"), |
| has(expr(SuspiciousNumberObjectExprM)))) |
| .bind("pedantic"); |
| |
| auto ConversionThroughExplicitBooleanCastM = |
| explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM), |
| has(expr(SuspiciousNumberObjectExprM)))); |
| |
| auto ConversionThroughExplicitNumberCastM = |
| explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM), |
| has(expr(SuspiciousNumberObjectExprM)))); |
| |
| auto ConversionThroughInitializerM = |
| declStmt(hasSingleDecl( |
| varDecl(hasType(SuspiciousScalarTypeM), |
| hasInitializer(SuspiciousNumberObjectExprM)))); |
| |
| auto FinalM = stmt(anyOf(ConversionThroughAssignmentM, |
| ConversionThroughBranchingM, |
| ConversionThroughCallM, |
| ConversionThroughComparisonM, |
| ConversionThroughConditionalOperatorM, |
| ConversionThroughEquivalenceM, |
| ConversionThroughExclamationMarkM, |
| ConversionThroughExplicitBooleanCastM, |
| ConversionThroughExplicitNumberCastM, |
| ConversionThroughInitializerM)).bind("conv"); |
| |
| MatchFinder F; |
| Callback CB(this, BR, AM.getAnalysisDeclContext(D)); |
| |
| F.addMatcher(traverse(TK_AsIs, stmt(forEachDescendant(FinalM))), &CB); |
| F.match(*D->getBody(), AM.getASTContext()); |
| } |
| |
| void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) { |
| NumberObjectConversionChecker *Chk = |
| Mgr.registerChecker<NumberObjectConversionChecker>(); |
| Chk->Pedantic = |
| Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic"); |
| } |
| |
| bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) { |
| return true; |
| } |
