17330f729Sjoerg //===- NumberObjectConversionChecker.cpp -------------------------*- C++ -*-==//
27330f729Sjoerg //
37330f729Sjoerg // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
47330f729Sjoerg // See https://llvm.org/LICENSE.txt for license information.
57330f729Sjoerg // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
67330f729Sjoerg //
77330f729Sjoerg //===----------------------------------------------------------------------===//
87330f729Sjoerg //
97330f729Sjoerg // This file defines NumberObjectConversionChecker, which checks for a
107330f729Sjoerg // particular common mistake when dealing with numbers represented as objects
117330f729Sjoerg // passed around by pointers. Namely, the language allows to reinterpret the
127330f729Sjoerg // pointer as a number directly, often without throwing any warnings,
137330f729Sjoerg // but in most cases the result of such conversion is clearly unexpected,
147330f729Sjoerg // as pointer value, rather than number value represented by the pointee object,
157330f729Sjoerg // becomes the result of such operation.
167330f729Sjoerg //
177330f729Sjoerg // Currently the checker supports the Objective-C NSNumber class,
187330f729Sjoerg // and the OSBoolean class found in macOS low-level code; the latter
197330f729Sjoerg // can only hold boolean values.
207330f729Sjoerg //
217330f729Sjoerg // This checker has an option "Pedantic" (boolean), which enables detection of
227330f729Sjoerg // more conversion patterns (which are most likely more harmless, and therefore
237330f729Sjoerg // are more likely to produce false positives) - disabled by default,
247330f729Sjoerg // enabled with `-analyzer-config osx.NumberObjectConversion:Pedantic=true'.
257330f729Sjoerg //
267330f729Sjoerg //===----------------------------------------------------------------------===//
277330f729Sjoerg
287330f729Sjoerg #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
297330f729Sjoerg #include "clang/ASTMatchers/ASTMatchFinder.h"
307330f729Sjoerg #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
317330f729Sjoerg #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
327330f729Sjoerg #include "clang/StaticAnalyzer/Core/Checker.h"
337330f729Sjoerg #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
347330f729Sjoerg #include "clang/Lex/Lexer.h"
357330f729Sjoerg #include "llvm/ADT/APSInt.h"
367330f729Sjoerg
377330f729Sjoerg using namespace clang;
387330f729Sjoerg using namespace ento;
397330f729Sjoerg using namespace ast_matchers;
407330f729Sjoerg
417330f729Sjoerg namespace {
427330f729Sjoerg
437330f729Sjoerg class NumberObjectConversionChecker : public Checker<check::ASTCodeBody> {
447330f729Sjoerg public:
457330f729Sjoerg bool Pedantic;
467330f729Sjoerg
477330f729Sjoerg void checkASTCodeBody(const Decl *D, AnalysisManager &AM,
487330f729Sjoerg BugReporter &BR) const;
497330f729Sjoerg };
507330f729Sjoerg
517330f729Sjoerg class Callback : public MatchFinder::MatchCallback {
527330f729Sjoerg const NumberObjectConversionChecker *C;
537330f729Sjoerg BugReporter &BR;
547330f729Sjoerg AnalysisDeclContext *ADC;
557330f729Sjoerg
567330f729Sjoerg public:
Callback(const NumberObjectConversionChecker * C,BugReporter & BR,AnalysisDeclContext * ADC)577330f729Sjoerg Callback(const NumberObjectConversionChecker *C,
587330f729Sjoerg BugReporter &BR, AnalysisDeclContext *ADC)
597330f729Sjoerg : C(C), BR(BR), ADC(ADC) {}
60*e038c9c4Sjoerg void run(const MatchFinder::MatchResult &Result) override;
617330f729Sjoerg };
627330f729Sjoerg } // end of anonymous namespace
637330f729Sjoerg
run(const MatchFinder::MatchResult & Result)647330f729Sjoerg void Callback::run(const MatchFinder::MatchResult &Result) {
657330f729Sjoerg bool IsPedanticMatch =
667330f729Sjoerg (Result.Nodes.getNodeAs<Stmt>("pedantic") != nullptr);
677330f729Sjoerg if (IsPedanticMatch && !C->Pedantic)
687330f729Sjoerg return;
697330f729Sjoerg
707330f729Sjoerg ASTContext &ACtx = ADC->getASTContext();
717330f729Sjoerg
727330f729Sjoerg if (const Expr *CheckIfNull =
737330f729Sjoerg Result.Nodes.getNodeAs<Expr>("check_if_null")) {
747330f729Sjoerg // Unless the macro indicates that the intended type is clearly not
757330f729Sjoerg // a pointer type, we should avoid warning on comparing pointers
767330f729Sjoerg // to zero literals in non-pedantic mode.
777330f729Sjoerg // FIXME: Introduce an AST matcher to implement the macro-related logic?
787330f729Sjoerg bool MacroIndicatesWeShouldSkipTheCheck = false;
797330f729Sjoerg SourceLocation Loc = CheckIfNull->getBeginLoc();
807330f729Sjoerg if (Loc.isMacroID()) {
817330f729Sjoerg StringRef MacroName = Lexer::getImmediateMacroName(
827330f729Sjoerg Loc, ACtx.getSourceManager(), ACtx.getLangOpts());
837330f729Sjoerg if (MacroName == "NULL" || MacroName == "nil")
847330f729Sjoerg return;
857330f729Sjoerg if (MacroName == "YES" || MacroName == "NO")
867330f729Sjoerg MacroIndicatesWeShouldSkipTheCheck = true;
877330f729Sjoerg }
887330f729Sjoerg if (!MacroIndicatesWeShouldSkipTheCheck) {
897330f729Sjoerg Expr::EvalResult EVResult;
907330f729Sjoerg if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt(
917330f729Sjoerg EVResult, ACtx, Expr::SE_AllowSideEffects)) {
927330f729Sjoerg llvm::APSInt Result = EVResult.Val.getInt();
937330f729Sjoerg if (Result == 0) {
947330f729Sjoerg if (!C->Pedantic)
957330f729Sjoerg return;
967330f729Sjoerg IsPedanticMatch = true;
977330f729Sjoerg }
987330f729Sjoerg }
997330f729Sjoerg }
1007330f729Sjoerg }
1017330f729Sjoerg
1027330f729Sjoerg const Stmt *Conv = Result.Nodes.getNodeAs<Stmt>("conv");
1037330f729Sjoerg assert(Conv);
1047330f729Sjoerg
1057330f729Sjoerg const Expr *ConvertedCObject = Result.Nodes.getNodeAs<Expr>("c_object");
1067330f729Sjoerg const Expr *ConvertedCppObject = Result.Nodes.getNodeAs<Expr>("cpp_object");
1077330f729Sjoerg const Expr *ConvertedObjCObject = Result.Nodes.getNodeAs<Expr>("objc_object");
1087330f729Sjoerg bool IsCpp = (ConvertedCppObject != nullptr);
1097330f729Sjoerg bool IsObjC = (ConvertedObjCObject != nullptr);
1107330f729Sjoerg const Expr *Obj = IsObjC ? ConvertedObjCObject
1117330f729Sjoerg : IsCpp ? ConvertedCppObject
1127330f729Sjoerg : ConvertedCObject;
1137330f729Sjoerg assert(Obj);
1147330f729Sjoerg
1157330f729Sjoerg bool IsComparison =
1167330f729Sjoerg (Result.Nodes.getNodeAs<Stmt>("comparison") != nullptr);
1177330f729Sjoerg
1187330f729Sjoerg bool IsOSNumber =
1197330f729Sjoerg (Result.Nodes.getNodeAs<Decl>("osnumber") != nullptr);
1207330f729Sjoerg
1217330f729Sjoerg bool IsInteger =
1227330f729Sjoerg (Result.Nodes.getNodeAs<QualType>("int_type") != nullptr);
1237330f729Sjoerg bool IsObjCBool =
1247330f729Sjoerg (Result.Nodes.getNodeAs<QualType>("objc_bool_type") != nullptr);
1257330f729Sjoerg bool IsCppBool =
1267330f729Sjoerg (Result.Nodes.getNodeAs<QualType>("cpp_bool_type") != nullptr);
1277330f729Sjoerg
1287330f729Sjoerg llvm::SmallString<64> Msg;
1297330f729Sjoerg llvm::raw_svector_ostream OS(Msg);
1307330f729Sjoerg
1317330f729Sjoerg // Remove ObjC ARC qualifiers.
1327330f729Sjoerg QualType ObjT = Obj->getType().getUnqualifiedType();
1337330f729Sjoerg
1347330f729Sjoerg // Remove consts from pointers.
1357330f729Sjoerg if (IsCpp) {
1367330f729Sjoerg assert(ObjT.getCanonicalType()->isPointerType());
1377330f729Sjoerg ObjT = ACtx.getPointerType(
1387330f729Sjoerg ObjT->getPointeeType().getCanonicalType().getUnqualifiedType());
1397330f729Sjoerg }
1407330f729Sjoerg
1417330f729Sjoerg if (IsComparison)
1427330f729Sjoerg OS << "Comparing ";
1437330f729Sjoerg else
1447330f729Sjoerg OS << "Converting ";
1457330f729Sjoerg
1467330f729Sjoerg OS << "a pointer value of type '" << ObjT.getAsString() << "' to a ";
1477330f729Sjoerg
1487330f729Sjoerg std::string EuphemismForPlain = "primitive";
1497330f729Sjoerg std::string SuggestedApi = IsObjC ? (IsInteger ? "" : "-boolValue")
1507330f729Sjoerg : IsCpp ? (IsOSNumber ? "" : "getValue()")
1517330f729Sjoerg : "CFNumberGetValue()";
1527330f729Sjoerg if (SuggestedApi.empty()) {
1537330f729Sjoerg // A generic message if we're not sure what API should be called.
1547330f729Sjoerg // FIXME: Pattern-match the integer type to make a better guess?
1557330f729Sjoerg SuggestedApi =
1567330f729Sjoerg "a method on '" + ObjT.getAsString() + "' to get the scalar value";
1577330f729Sjoerg // "scalar" is not quite correct or common, but some documentation uses it
1587330f729Sjoerg // when describing object methods we suggest. For consistency, we use
1597330f729Sjoerg // "scalar" in the whole sentence when we need to use this word in at least
1607330f729Sjoerg // one place, otherwise we use "primitive".
1617330f729Sjoerg EuphemismForPlain = "scalar";
1627330f729Sjoerg }
1637330f729Sjoerg
1647330f729Sjoerg if (IsInteger)
1657330f729Sjoerg OS << EuphemismForPlain << " integer value";
1667330f729Sjoerg else if (IsObjCBool)
1677330f729Sjoerg OS << EuphemismForPlain << " BOOL value";
1687330f729Sjoerg else if (IsCppBool)
1697330f729Sjoerg OS << EuphemismForPlain << " bool value";
1707330f729Sjoerg else // Branch condition?
1717330f729Sjoerg OS << EuphemismForPlain << " boolean value";
1727330f729Sjoerg
1737330f729Sjoerg
1747330f729Sjoerg if (IsPedanticMatch)
1757330f729Sjoerg OS << "; instead, either compare the pointer to "
1767330f729Sjoerg << (IsObjC ? "nil" : IsCpp ? "nullptr" : "NULL") << " or ";
1777330f729Sjoerg else
1787330f729Sjoerg OS << "; did you mean to ";
1797330f729Sjoerg
1807330f729Sjoerg if (IsComparison)
1817330f729Sjoerg OS << "compare the result of calling " << SuggestedApi;
1827330f729Sjoerg else
1837330f729Sjoerg OS << "call " << SuggestedApi;
1847330f729Sjoerg
1857330f729Sjoerg if (!IsPedanticMatch)
1867330f729Sjoerg OS << "?";
1877330f729Sjoerg
1887330f729Sjoerg BR.EmitBasicReport(
1897330f729Sjoerg ADC->getDecl(), C, "Suspicious number object conversion", "Logic error",
1907330f729Sjoerg OS.str(),
1917330f729Sjoerg PathDiagnosticLocation::createBegin(Obj, BR.getSourceManager(), ADC),
1927330f729Sjoerg Conv->getSourceRange());
1937330f729Sjoerg }
1947330f729Sjoerg
checkASTCodeBody(const Decl * D,AnalysisManager & AM,BugReporter & BR) const1957330f729Sjoerg void NumberObjectConversionChecker::checkASTCodeBody(const Decl *D,
1967330f729Sjoerg AnalysisManager &AM,
1977330f729Sjoerg BugReporter &BR) const {
1987330f729Sjoerg // Currently this matches CoreFoundation opaque pointer typedefs.
1997330f729Sjoerg auto CSuspiciousNumberObjectExprM =
2007330f729Sjoerg expr(ignoringParenImpCasts(
2017330f729Sjoerg expr(hasType(
2027330f729Sjoerg typedefType(hasDeclaration(anyOf(
2037330f729Sjoerg typedefDecl(hasName("CFNumberRef")),
2047330f729Sjoerg typedefDecl(hasName("CFBooleanRef")))))))
2057330f729Sjoerg .bind("c_object")));
2067330f729Sjoerg
2077330f729Sjoerg // Currently this matches XNU kernel number-object pointers.
2087330f729Sjoerg auto CppSuspiciousNumberObjectExprM =
2097330f729Sjoerg expr(ignoringParenImpCasts(
2107330f729Sjoerg expr(hasType(hasCanonicalType(
2117330f729Sjoerg pointerType(pointee(hasCanonicalType(
2127330f729Sjoerg recordType(hasDeclaration(
2137330f729Sjoerg anyOf(
2147330f729Sjoerg cxxRecordDecl(hasName("OSBoolean")),
2157330f729Sjoerg cxxRecordDecl(hasName("OSNumber"))
2167330f729Sjoerg .bind("osnumber"))))))))))
2177330f729Sjoerg .bind("cpp_object")));
2187330f729Sjoerg
2197330f729Sjoerg // Currently this matches NeXTSTEP number objects.
2207330f729Sjoerg auto ObjCSuspiciousNumberObjectExprM =
2217330f729Sjoerg expr(ignoringParenImpCasts(
2227330f729Sjoerg expr(hasType(hasCanonicalType(
2237330f729Sjoerg objcObjectPointerType(pointee(
2247330f729Sjoerg qualType(hasCanonicalType(
2257330f729Sjoerg qualType(hasDeclaration(
2267330f729Sjoerg objcInterfaceDecl(hasName("NSNumber")))))))))))
2277330f729Sjoerg .bind("objc_object")));
2287330f729Sjoerg
2297330f729Sjoerg auto SuspiciousNumberObjectExprM = anyOf(
2307330f729Sjoerg CSuspiciousNumberObjectExprM,
2317330f729Sjoerg CppSuspiciousNumberObjectExprM,
2327330f729Sjoerg ObjCSuspiciousNumberObjectExprM);
2337330f729Sjoerg
2347330f729Sjoerg // Useful for predicates like "Unless we've seen the same object elsewhere".
2357330f729Sjoerg auto AnotherSuspiciousNumberObjectExprM =
2367330f729Sjoerg expr(anyOf(
2377330f729Sjoerg equalsBoundNode("c_object"),
2387330f729Sjoerg equalsBoundNode("objc_object"),
2397330f729Sjoerg equalsBoundNode("cpp_object")));
2407330f729Sjoerg
2417330f729Sjoerg // The .bind here is in order to compose the error message more accurately.
2427330f729Sjoerg auto ObjCSuspiciousScalarBooleanTypeM =
2437330f729Sjoerg qualType(typedefType(hasDeclaration(
2447330f729Sjoerg typedefDecl(hasName("BOOL"))))).bind("objc_bool_type");
2457330f729Sjoerg
2467330f729Sjoerg // The .bind here is in order to compose the error message more accurately.
2477330f729Sjoerg auto SuspiciousScalarBooleanTypeM =
2487330f729Sjoerg qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"),
2497330f729Sjoerg ObjCSuspiciousScalarBooleanTypeM));
2507330f729Sjoerg
2517330f729Sjoerg // The .bind here is in order to compose the error message more accurately.
2527330f729Sjoerg // Also avoid intptr_t and uintptr_t because they were specifically created
2537330f729Sjoerg // for storing pointers.
2547330f729Sjoerg auto SuspiciousScalarNumberTypeM =
2557330f729Sjoerg qualType(hasCanonicalType(isInteger()),
2567330f729Sjoerg unless(typedefType(hasDeclaration(
2577330f729Sjoerg typedefDecl(matchesName("^::u?intptr_t$"))))))
2587330f729Sjoerg .bind("int_type");
2597330f729Sjoerg
2607330f729Sjoerg auto SuspiciousScalarTypeM =
2617330f729Sjoerg qualType(anyOf(SuspiciousScalarBooleanTypeM,
2627330f729Sjoerg SuspiciousScalarNumberTypeM));
2637330f729Sjoerg
2647330f729Sjoerg auto SuspiciousScalarExprM =
2657330f729Sjoerg expr(ignoringParenImpCasts(expr(hasType(SuspiciousScalarTypeM))));
2667330f729Sjoerg
2677330f729Sjoerg auto ConversionThroughAssignmentM =
2687330f729Sjoerg binaryOperator(allOf(hasOperatorName("="),
2697330f729Sjoerg hasLHS(SuspiciousScalarExprM),
2707330f729Sjoerg hasRHS(SuspiciousNumberObjectExprM)));
2717330f729Sjoerg
2727330f729Sjoerg auto ConversionThroughBranchingM =
2737330f729Sjoerg ifStmt(allOf(
2747330f729Sjoerg hasCondition(SuspiciousNumberObjectExprM),
2757330f729Sjoerg unless(hasConditionVariableStatement(declStmt())
2767330f729Sjoerg ))).bind("pedantic");
2777330f729Sjoerg
2787330f729Sjoerg auto ConversionThroughCallM =
2797330f729Sjoerg callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),
2807330f729Sjoerg ignoringParenImpCasts(
2817330f729Sjoerg SuspiciousNumberObjectExprM))));
2827330f729Sjoerg
2837330f729Sjoerg // We bind "check_if_null" to modify the warning message
2847330f729Sjoerg // in case it was intended to compare a pointer to 0 with a relatively-ok
2857330f729Sjoerg // construct "x == 0" or "x != 0".
2867330f729Sjoerg auto ConversionThroughEquivalenceM =
2877330f729Sjoerg binaryOperator(allOf(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
2887330f729Sjoerg hasEitherOperand(SuspiciousNumberObjectExprM),
2897330f729Sjoerg hasEitherOperand(SuspiciousScalarExprM
2907330f729Sjoerg .bind("check_if_null"))))
2917330f729Sjoerg .bind("comparison");
2927330f729Sjoerg
2937330f729Sjoerg auto ConversionThroughComparisonM =
2947330f729Sjoerg binaryOperator(allOf(anyOf(hasOperatorName(">="), hasOperatorName(">"),
2957330f729Sjoerg hasOperatorName("<="), hasOperatorName("<")),
2967330f729Sjoerg hasEitherOperand(SuspiciousNumberObjectExprM),
2977330f729Sjoerg hasEitherOperand(SuspiciousScalarExprM)))
2987330f729Sjoerg .bind("comparison");
2997330f729Sjoerg
3007330f729Sjoerg auto ConversionThroughConditionalOperatorM =
3017330f729Sjoerg conditionalOperator(allOf(
3027330f729Sjoerg hasCondition(SuspiciousNumberObjectExprM),
3037330f729Sjoerg unless(hasTrueExpression(
3047330f729Sjoerg hasDescendant(AnotherSuspiciousNumberObjectExprM))),
3057330f729Sjoerg unless(hasFalseExpression(
3067330f729Sjoerg hasDescendant(AnotherSuspiciousNumberObjectExprM)))))
3077330f729Sjoerg .bind("pedantic");
3087330f729Sjoerg
3097330f729Sjoerg auto ConversionThroughExclamationMarkM =
3107330f729Sjoerg unaryOperator(allOf(hasOperatorName("!"),
3117330f729Sjoerg has(expr(SuspiciousNumberObjectExprM))))
3127330f729Sjoerg .bind("pedantic");
3137330f729Sjoerg
3147330f729Sjoerg auto ConversionThroughExplicitBooleanCastM =
3157330f729Sjoerg explicitCastExpr(allOf(hasType(SuspiciousScalarBooleanTypeM),
3167330f729Sjoerg has(expr(SuspiciousNumberObjectExprM))));
3177330f729Sjoerg
3187330f729Sjoerg auto ConversionThroughExplicitNumberCastM =
3197330f729Sjoerg explicitCastExpr(allOf(hasType(SuspiciousScalarNumberTypeM),
3207330f729Sjoerg has(expr(SuspiciousNumberObjectExprM))));
3217330f729Sjoerg
3227330f729Sjoerg auto ConversionThroughInitializerM =
3237330f729Sjoerg declStmt(hasSingleDecl(
3247330f729Sjoerg varDecl(hasType(SuspiciousScalarTypeM),
3257330f729Sjoerg hasInitializer(SuspiciousNumberObjectExprM))));
3267330f729Sjoerg
3277330f729Sjoerg auto FinalM = stmt(anyOf(ConversionThroughAssignmentM,
3287330f729Sjoerg ConversionThroughBranchingM,
3297330f729Sjoerg ConversionThroughCallM,
3307330f729Sjoerg ConversionThroughComparisonM,
3317330f729Sjoerg ConversionThroughConditionalOperatorM,
3327330f729Sjoerg ConversionThroughEquivalenceM,
3337330f729Sjoerg ConversionThroughExclamationMarkM,
3347330f729Sjoerg ConversionThroughExplicitBooleanCastM,
3357330f729Sjoerg ConversionThroughExplicitNumberCastM,
3367330f729Sjoerg ConversionThroughInitializerM)).bind("conv");
3377330f729Sjoerg
3387330f729Sjoerg MatchFinder F;
3397330f729Sjoerg Callback CB(this, BR, AM.getAnalysisDeclContext(D));
3407330f729Sjoerg
341*e038c9c4Sjoerg F.addMatcher(traverse(TK_AsIs, stmt(forEachDescendant(FinalM))), &CB);
3427330f729Sjoerg F.match(*D->getBody(), AM.getASTContext());
3437330f729Sjoerg }
3447330f729Sjoerg
registerNumberObjectConversionChecker(CheckerManager & Mgr)3457330f729Sjoerg void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
3467330f729Sjoerg NumberObjectConversionChecker *Chk =
3477330f729Sjoerg Mgr.registerChecker<NumberObjectConversionChecker>();
3487330f729Sjoerg Chk->Pedantic =
3497330f729Sjoerg Mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk, "Pedantic");
3507330f729Sjoerg }
3517330f729Sjoerg
shouldRegisterNumberObjectConversionChecker(const CheckerManager & mgr)352*e038c9c4Sjoerg bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) {
3537330f729Sjoerg return true;
3547330f729Sjoerg }
355