Home | History | Annotate | Line # | Download | only in Checkers 
      1 //===- NumberObjectConversionChecker.cpp -------------------------*- C++ -*-==//
      2 //
      3 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
      4 // See https://llvm.org/LICENSE.txt for license information.
      5 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
      6 //
      7 //===----------------------------------------------------------------------===//
      8 //
      9 // This file defines NumberObjectConversionChecker, which checks for a
     10 // particular common mistake when dealing with numbers represented as objects
     11 // passed around by pointers. Namely, the language allows to reinterpret the
     12 // pointer as a number directly, often without throwing any warnings,
     13 // but in most cases the result of such conversion is clearly unexpected,
     14 // as pointer value, rather than number value represented by the pointee object,
     15 // becomes the result of such operation.
     16 //
     17 // Currently the checker supports the Objective-C NSNumber class,
     18 // and the OSBoolean class found in macOS low-level code; the latter
     19 // can only hold boolean values.
     20 //
     21 // This checker has an option "Pedantic" (boolean), which enables detection of
     22 // more conversion patterns (which are most likely more harmless, and therefore
     23 // are more likely to produce false positives) - disabled by default,
     24 // enabled with `-analyzer-config osx.NumberObjectConversion:Pedantic=true'.
     25 //
     26 //===----------------------------------------------------------------------===//
     27 
     28 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
     29 #include "clang/ASTMatchers/ASTMatchFinder.h"
     30 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
     31 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
     32 #include "clang/StaticAnalyzer/Core/Checker.h"
     33 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
     34 #include "clang/Lex/Lexer.h"
     35 #include "llvm/ADT/APSInt.h"
     36 
     37 using namespace clang;
     38 using namespace ento;
     39 using namespace ast_matchers;
     40 
     41 namespace {
     42 
     43 class NumberObjectConversionChecker : public Checker<check::ASTCodeBody> {
     44 public:
     45   bool Pedantic;
     46 
     47   void checkASTCodeBody(const Decl *D, AnalysisManager &AM,
     48                         BugReporter &BR) const;
     49 };
     50 
     51 class Callback : public MatchFinder::MatchCallback {
     52   const NumberObjectConversionChecker *C;
     53   BugReporter &BR;
     54   AnalysisDeclContext *ADC;
     55 
     56 public:
     57   Callback(const NumberObjectConversionChecker *C,
     58            BugReporter &BR, AnalysisDeclContext *ADC)
     59       : C(C), BR(BR), ADC(ADC) {}
     60   void run(const MatchFinder::MatchResult &Result) override;
     61 };
     62 } // end of anonymous namespace
     63 
     64 void Callback::run(const MatchFinder::MatchResult &Result) {
     65   bool IsPedanticMatch =
     66       (Result.Nodes.getNodeAs<Stmt>("pedantic") != nullptr);
     67   if (IsPedanticMatch && !C->Pedantic)
     68     return;
     69 
     70   ASTContext &ACtx = ADC->getASTContext();
     71 
     72   if (const Expr *CheckIfNull =
     73           Result.Nodes.getNodeAs<Expr>("check_if_null")) {
     74     // Unless the macro indicates that the intended type is clearly not
     75     // a pointer type, we should avoid warning on comparing pointers
     76     // to zero literals in non-pedantic mode.
     77     // FIXME: Introduce an AST matcher to implement the macro-related logic?
     78     bool MacroIndicatesWeShouldSkipTheCheck = false;
     79     SourceLocation Loc = CheckIfNull->getBeginLoc();
     80     if (Loc.isMacroID()) {
     81       StringRef MacroName = Lexer::getImmediateMacroName(
     82           Loc, ACtx.getSourceManager(), ACtx.getLangOpts());
     83       if (MacroName == "NULL" || MacroName == "nil")
     84         return;
     85       if (MacroName == "YES" || MacroName == "NO")
     86         MacroIndicatesWeShouldSkipTheCheck = true;
     87     }
     88     if (!MacroIndicatesWeShouldSkipTheCheck) {
     89       Expr::EvalResult EVResult;
     90       if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt(
     91               EVResult, ACtx, Expr::SE_AllowSideEffects)) {
     92         llvm::APSInt Result = EVResult.Val.getInt();
     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(traverse(TK_AsIs, 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(Chk, "Pedantic");
    350 }
    351 
    352 bool ento::shouldRegisterNumberObjectConversionChecker(const CheckerManager &mgr) {
    353   return true;
    354 }
    355