clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp

//===- 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;

  const Stmt *Conv = Result.Nodes.getNodeAs<Stmt>("conv");
  assert(Conv);
  const Expr *Osboolean = Result.Nodes.getNodeAs<Expr>("osboolean");
  const Expr *Nsnumber = Result.Nodes.getNodeAs<Expr>("nsnumber");
  bool IsObjC = (bool)Nsnumber;
  const Expr *Obj = IsObjC ? Nsnumber : Osboolean;
  assert(Obj);

  ASTContext &ACtx = ADC->getASTContext();

  if (const Expr *CheckIfNull =
          Result.Nodes.getNodeAs<Expr>("check_if_null")) {
    // We consider NULL to be a pointer, even if it is defined as a plain 0.
    // FIXME: Introduce a matcher to implement this logic?
    SourceLocation Loc = CheckIfNull->getLocStart();
    if (Loc.isMacroID()) {
      StringRef MacroName = Lexer::getImmediateMacroName(
          Loc, ACtx.getSourceManager(), ACtx.getLangOpts());
      if (MacroName != "YES" && MacroName != "NO")
        return;
    } else {
      // Otherwise, comparison of pointers to 0 might still be intentional.
      // See if this is the case.
      llvm::APSInt Result;
      if (CheckIfNull->IgnoreParenCasts()->EvaluateAsInt(
              Result, ACtx, Expr::SE_AllowSideEffects)) {
        if (Result == 0) {
          if (!C->Pedantic)
            return;
          IsPedanticMatch = true;
        }
      }
    }
  }

  llvm::SmallString<64> Msg;
  llvm::raw_svector_ostream OS(Msg);
  OS << "Converting '"
     << Obj->getType().getCanonicalType().getUnqualifiedType().getAsString()
     << "' to a plain ";

  if (Result.Nodes.getNodeAs<QualType>("int_type") != nullptr)
    OS << "integer value";
  else if (Result.Nodes.getNodeAs<QualType>("objc_bool_type") != nullptr)
    OS << "BOOL value";
  else if (Result.Nodes.getNodeAs<QualType>("cpp_bool_type") != nullptr)
    OS << "bool value";
  else
    OS << "boolean value for branching";

  if (IsPedanticMatch) {
    if (IsObjC) {
      OS << "; please compare the pointer to nil instead "
            "to suppress this warning";
    } else {
      OS << "; please compare the pointer to NULL or nullptr instead "
            "to suppress this warning";
    }
  } else {
    OS << "; pointer value is being used instead";
  }

  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 {
  MatchFinder F;
  Callback CB(this, BR, AM.getAnalysisDeclContext(D));

  auto OSBooleanExprM =
      expr(ignoringParenImpCasts(
          expr(hasType(hasCanonicalType(
              pointerType(pointee(hasCanonicalType(
                  recordType(hasDeclaration(
                      cxxRecordDecl(hasName(
                          "OSBoolean")))))))))).bind("osboolean")));

  auto NSNumberExprM =
      expr(ignoringParenImpCasts(expr(hasType(hasCanonicalType(
          objcObjectPointerType(pointee(
              qualType(hasCanonicalType(
                  qualType(hasDeclaration(
                      objcInterfaceDecl(hasName(
                          "NSNumber"))))))))))).bind("nsnumber")));

  auto SuspiciousExprM =
      anyOf(OSBooleanExprM, NSNumberExprM);

  auto AnotherNSNumberExprM =
      expr(equalsBoundNode("nsnumber"));

  // The .bind here is in order to compose the error message more accurately.
  auto ObjCBooleanTypeM =
      qualType(typedefType(hasDeclaration(
                   typedefDecl(hasName("BOOL"))))).bind("objc_bool_type");

  // The .bind here is in order to compose the error message more accurately.
  auto AnyBooleanTypeM =
      qualType(anyOf(qualType(booleanType()).bind("cpp_bool_type"),
                     ObjCBooleanTypeM));


  // The .bind here is in order to compose the error message more accurately.
  auto AnyNumberTypeM =
      qualType(hasCanonicalType(isInteger()),
               unless(typedefType(hasDeclaration(
                   typedefDecl(matchesName("^::u?intptr_t$"))))))
      .bind("int_type");

  auto AnyBooleanOrNumberTypeM =
      qualType(anyOf(AnyBooleanTypeM, AnyNumberTypeM));

  auto AnyBooleanOrNumberExprM =
      expr(ignoringParenImpCasts(expr(hasType(AnyBooleanOrNumberTypeM))));

  auto ConversionThroughAssignmentM =
      binaryOperator(hasOperatorName("="),
                     hasLHS(AnyBooleanOrNumberExprM),
                     hasRHS(SuspiciousExprM));

  auto ConversionThroughBranchingM =
      ifStmt(hasCondition(SuspiciousExprM))
      .bind("pedantic");

  auto ConversionThroughCallM =
      callExpr(hasAnyArgument(allOf(hasType(AnyBooleanOrNumberTypeM),
                                    ignoringParenImpCasts(SuspiciousExprM))));

  // 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(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
                     hasEitherOperand(SuspiciousExprM),
                     hasEitherOperand(AnyBooleanOrNumberExprM
                                      .bind("check_if_null")));

  auto ConversionThroughComparisonM =
      binaryOperator(anyOf(hasOperatorName(">="), hasOperatorName(">"),
                           hasOperatorName("<="), hasOperatorName("<")),
                     hasEitherOperand(SuspiciousExprM),
                     hasEitherOperand(AnyBooleanOrNumberExprM));

  auto ConversionThroughConditionalOperatorM =
      conditionalOperator(
          hasCondition(SuspiciousExprM),
          unless(hasTrueExpression(hasDescendant(AnotherNSNumberExprM))),
          unless(hasFalseExpression(hasDescendant(AnotherNSNumberExprM))))
      .bind("pedantic");

  auto ConversionThroughExclamationMarkM =
      unaryOperator(hasOperatorName("!"), has(expr(SuspiciousExprM)))
      .bind("pedantic");

  auto ConversionThroughExplicitBooleanCastM =
      explicitCastExpr(hasType(AnyBooleanTypeM),
                       has(expr(SuspiciousExprM)))
      .bind("pedantic");

  auto ConversionThroughExplicitNumberCastM =
      explicitCastExpr(hasType(AnyNumberTypeM),
                       has(expr(SuspiciousExprM)));

  auto ConversionThroughInitializerM =
      declStmt(hasSingleDecl(
          varDecl(hasType(AnyBooleanOrNumberTypeM),
                  hasInitializer(SuspiciousExprM))));

  auto FinalM = stmt(anyOf(ConversionThroughAssignmentM,
                           ConversionThroughBranchingM,
                           ConversionThroughCallM,
                           ConversionThroughComparisonM,
                           ConversionThroughConditionalOperatorM,
                           ConversionThroughEquivalenceM,
                           ConversionThroughExclamationMarkM,
                           ConversionThroughExplicitBooleanCastM,
                           ConversionThroughExplicitNumberCastM,
                           ConversionThroughInitializerM)).bind("conv");

  F.addMatcher(stmt(forEachDescendant(FinalM)), &CB);
  F.match(*D->getBody(), AM.getASTContext());
}

void ento::registerNumberObjectConversionChecker(CheckerManager &Mgr) {
  const LangOptions &LO = Mgr.getLangOpts();
  if (LO.CPlusPlus || LO.ObjC2) {
    NumberObjectConversionChecker *Chk =
        Mgr.registerChecker<NumberObjectConversionChecker>();
    Chk->Pedantic =
        Mgr.getAnalyzerOptions().getBooleanOption("Pedantic", false, Chk);
  }
}