diff options
Diffstat (limited to 'contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp')
| -rw-r--r-- | contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp | 350 |
1 files changed, 350 insertions, 0 deletions
diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp new file mode 100644 index 000000000000..d1749cfdbe27 --- /dev/null +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp @@ -0,0 +1,350 @@ +//===- NumberObjectConversionChecker.cpp -------------------------*- C++ -*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// 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 "ClangSACheckers.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) {} + virtual void run(const MatchFinder::MatchResult &Result); +}; +} // 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->getLocStart(); + 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) { + llvm::APSInt Result; + if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt( + Result, ACtx, Expr::SE_AllowSideEffects)) { + 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(stmt(forEachDescendant(FinalM)), &CB); + F.match(*D->getBody(), AM.getASTContext()); +} + +void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) { + NumberObjectConversionChecker *Chk = + Mgr.registerChecker<NumberObjectConversionChecker>(); + Chk->Pedantic = + Mgr.getAnalyzerOptions().getBooleanOption("Pedantic", false, Chk); +} |