[alpha.webkit.ForwardDeclChecker] Add a new WebKit checker for forward declarations (#130554)

Add a new static analyzer which emits warnings for function call
arguments, local variables, and member variables that are only forward
declared. These forward declaration prevents other WebKit checkers from
checking the safety of code.

Author: rniwa

clang/docs/analyzer/checkers.rst

@@ -3476,6 +3476,24 @@ Limitations:
 alpha.WebKit
 ^^^^^^^^^^^^
 
+alpha.webkit.ForwardDeclChecker
+"""""""""""""""""""""""""""""""
+Check for local variables, member variables, and function arguments that are forward declared.
+
+.. code-block:: cpp
+
+ struct Obj;
+ Obj* provide();
+
+ struct Foo {
+   Obj* ptr; // warn
+ };
+
+  void foo() {
+    Obj* obj = provide(); // warn
+    consume(obj); // warn
+  }
+
 .. _alpha-webkit-NoUncheckedPtrMemberChecker:
 
 alpha.webkit.MemoryUnsafeCastChecker

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -1761,6 +1761,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">,
 
 let ParentPackage = WebKitAlpha in {
 
+def ForwardDeclChecker : Checker<"ForwardDeclChecker">,
+  HelpText<"Check for forward declared local or member variables and function arguments">,
+  Documentation<HasDocumentation>;
+
 def MemoryUnsafeCastChecker : Checker<"MemoryUnsafeCastChecker">,
   HelpText<"Check for memory unsafe casts from base type to derived type.">,
   Documentation<HasDocumentation>;

clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

@@ -130,6 +130,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   ValistChecker.cpp
   VirtualCallChecker.cpp
   WebKit/ASTUtils.cpp
+  WebKit/ForwardDeclChecker.cpp
   WebKit/MemoryUnsafeCastChecker.cpp
   WebKit/PtrTypesSemantics.cpp
   WebKit/RefCntblBaseVirtualDtorChecker.cpp

clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp

@@ -0,0 +1,327 @@
+//=======- ForwardDeclChecker.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
+//
+//===----------------------------------------------------------------------===//
+
+#include "ASTUtils.h"
+#include "DiagOutputUtils.h"
+#include "PtrTypesSemantics.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Analysis/DomainSpecific/CocoaConventions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/SaveAndRestore.h"
+#include <optional>
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
+  BugType Bug;
+  mutable BugReporter *BR;
+  mutable RetainTypeChecker RTC;
+  mutable llvm::DenseSet<const Type *> SystemTypes;
+
+public:
+  ForwardDeclChecker()
+      : Bug(this, "Forward declared member or local variable or parameter",
+            "WebKit coding guidelines") {}
+
+  void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
+                    BugReporter &BRArg) const {
+    BR = &BRArg;
+
+    // The calls to checkAST* from AnalysisConsumer don't
+    // visit template instantiations or lambda classes. We
+    // want to visit those, so we make our own RecursiveASTVisitor.
+    struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
+      using Base = RecursiveASTVisitor<LocalVisitor>;
+
+      const ForwardDeclChecker *Checker;
+      Decl *DeclWithIssue{nullptr};
+
+      explicit LocalVisitor(const ForwardDeclChecker *Checker)
+          : Checker(Checker) {
+        assert(Checker);
+      }
+
+      bool shouldVisitTemplateInstantiations() const { return true; }
+      bool shouldVisitImplicitCode() const { return false; }
+
+      bool VisitTypedefDecl(TypedefDecl *TD) {
+        Checker->visitTypedef(TD);
+        return true;
+      }
+
+      bool VisitRecordDecl(const RecordDecl *RD) {
+        Checker->visitRecordDecl(RD, DeclWithIssue);
+        return true;
+      }
+
+      bool TraverseDecl(Decl *D) {
+        llvm::SaveAndRestore SavedDecl(DeclWithIssue);
+        if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)))
+          DeclWithIssue = D;
+        return Base::TraverseDecl(D);
+      }
+
+      bool VisitVarDecl(VarDecl *V) {
+        if (V->isLocalVarDecl())
+          Checker->visitVarDecl(V, DeclWithIssue);
+        return true;
+      }
+
+      bool VisitCallExpr(const CallExpr *CE) {
+        Checker->visitCallExpr(CE, DeclWithIssue);
+        return true;
+      }
+
+      bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
+        Checker->visitConstructExpr(CE, DeclWithIssue);
+        return true;
+      }
+
+      bool VisitObjCMessageExpr(const ObjCMessageExpr *ObjCMsgExpr) {
+        Checker->visitObjCMessageExpr(ObjCMsgExpr, DeclWithIssue);
+        return true;
+      }
+    };
+
+    LocalVisitor visitor(this);
+    RTC.visitTranslationUnitDecl(TUD);
+    visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
+  }
+
+  void visitTypedef(const TypedefDecl *TD) const {
+    RTC.visitTypedef(TD);
+    auto QT = TD->getUnderlyingType().getCanonicalType();
+    if (BR->getSourceManager().isInSystemHeader(TD->getBeginLoc())) {
+      if (auto *Type = QT.getTypePtrOrNull(); Type && QT->isPointerType())
+        SystemTypes.insert(Type);
+    }
+  }
+
+  bool isUnknownType(QualType QT) const {
+    auto *Type = QT.getTypePtrOrNull();
+    if (!Type)
+      return false;
+    auto *CanonicalType = QT.getCanonicalType().getTypePtrOrNull();
+    auto PointeeQT = Type->getPointeeType();
+    auto *PointeeType = PointeeQT.getTypePtrOrNull();
+    if (!PointeeType)
+      return false;
+    auto *R = PointeeType->getAsCXXRecordDecl();
+    if (!R) // Forward declaration of a Objective-C interface is safe.
+      return false;
+    auto Name = R->getName();
+    return !R->hasDefinition() && !RTC.isUnretained(QT) &&
+           !SystemTypes.contains(CanonicalType) &&
+           !Name.starts_with("Opaque") && Name != "_NSZone";
+  }
+
+  void visitRecordDecl(const RecordDecl *RD, const Decl *DeclWithIssue) const {
+    if (!RD->isThisDeclarationADefinition())
+      return;
+
+    if (RD->isImplicit() || RD->isLambda())
+      return;
+
+    const auto RDLocation = RD->getLocation();
+    if (!RDLocation.isValid())
+      return;
+
+    const auto Kind = RD->getTagKind();
+    if (Kind != TagTypeKind::Struct && Kind != TagTypeKind::Class)
+      return;
+
+    if (BR->getSourceManager().isInSystemHeader(RDLocation))
+      return;
+
+    // Ref-counted smartpointers actually have raw-pointer to uncounted type as
+    // a member but we trust them to handle it correctly.
+    auto R = llvm::dyn_cast_or_null<CXXRecordDecl>(RD);
+    if (!R || isRefCounted(R) || isCheckedPtr(R) || isRetainPtr(R))
+      return;
+
+    for (auto *Member : RD->fields()) {
+      auto QT = Member->getType();
+      if (isUnknownType(QT)) {
+        SmallString<100> Buf;
+        llvm::raw_svector_ostream Os(Buf);
+
+        const std::string TypeName = QT.getAsString();
+        Os << "Member variable ";
+        printQuotedName(Os, Member);
+        Os << " uses a forward declared type '" << TypeName << "'";
+
+        const SourceLocation SrcLocToReport = Member->getBeginLoc();
+        PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
+        auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+        Report->addRange(Member->getSourceRange());
+        Report->setDeclWithIssue(DeclWithIssue);
+        BR->emitReport(std::move(Report));
+      }
+    }
+  }
+
+  void visitVarDecl(const VarDecl *V, const Decl *DeclWithIssue) const {
+    if (BR->getSourceManager().isInSystemHeader(V->getBeginLoc()))
+      return;
+
+    auto QT = V->getType();
+    if (!isUnknownType(QT))
+      return;
+
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+    Os << "Local variable ";
+    printQuotedQualifiedName(Os, V);
+
+    reportBug(V->getBeginLoc(), V->getSourceRange(), DeclWithIssue, Os.str(),
+              QT);
+  }
+
+  void visitCallExpr(const CallExpr *CE, const Decl *DeclWithIssue) const {
+    if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
+      return;
+
+    if (auto *F = CE->getDirectCallee()) {
+      // Skip the first argument for overloaded member operators (e. g. lambda
+      // or std::function call operator).
+      unsigned ArgIdx =
+          isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F);
+
+      for (auto P = F->param_begin();
+           P < F->param_end() && ArgIdx < CE->getNumArgs(); ++P, ++ArgIdx)
+        visitCallArg(CE->getArg(ArgIdx), *P, DeclWithIssue);
+    }
+  }
+
+  void visitConstructExpr(const CXXConstructExpr *CE,
+                          const Decl *DeclWithIssue) const {
+    if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
+      return;
+
+    if (auto *F = CE->getConstructor()) {
+      // Skip the first argument for overloaded member operators (e. g. lambda
+      // or std::function call operator).
+      unsigned ArgIdx =
+          isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F);
+
+      for (auto P = F->param_begin();
+           P < F->param_end() && ArgIdx < CE->getNumArgs(); ++P, ++ArgIdx)
+        visitCallArg(CE->getArg(ArgIdx), *P, DeclWithIssue);
+    }
+  }
+
+  void visitObjCMessageExpr(const ObjCMessageExpr *E,
+                            const Decl *DeclWithIssue) const {
+    if (BR->getSourceManager().isInSystemHeader(E->getExprLoc()))
+      return;
+
+    if (auto *Receiver = E->getInstanceReceiver()->IgnoreParenCasts()) {
+      if (isUnknownType(E->getReceiverType()))
+        reportUnknownRecieverType(Receiver, DeclWithIssue);
+    }
+
+    auto *MethodDecl = E->getMethodDecl();
+    if (!MethodDecl)
+      return;
+
+    auto ArgCount = E->getNumArgs();
+    for (unsigned i = 0; i < ArgCount && i < MethodDecl->param_size(); ++i)
+      visitCallArg(E->getArg(i), MethodDecl->getParamDecl(i), DeclWithIssue);
+  }
+
+  void visitCallArg(const Expr *Arg, const ParmVarDecl *Param,
+                    const Decl *DeclWithIssue) const {
+    auto *ArgExpr = Arg->IgnoreParenCasts();
+    if (auto *InnerCE = dyn_cast<CallExpr>(Arg)) {
+      auto *InnerCallee = InnerCE->getDirectCallee();
+      if (InnerCallee && InnerCallee->isInStdNamespace() &&
+          safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) {
+        ArgExpr = InnerCE->getArg(0);
+        if (ArgExpr)
+          ArgExpr = ArgExpr->IgnoreParenCasts();
+      }
+    }
+    if (isa<CXXNullPtrLiteralExpr>(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
+        isa<CXXDefaultArgExpr>(ArgExpr))
+      return;
+    if (auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr)) {
+      if (auto *ValDecl = DRE->getDecl()) {
+        if (isa<ParmVarDecl>(ValDecl))
+          return;
+      }
+    }
+
+    QualType ArgType = Param->getType();
+    if (!isUnknownType(ArgType))
+      return;
+
+    reportUnknownArgType(Arg, Param, DeclWithIssue);
+  }
+
+  void reportUnknownArgType(const Expr *CA, const ParmVarDecl *Param,
+                            const Decl *DeclWithIssue) const {
+    assert(CA);
+
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    const std::string paramName = safeGetName(Param);
+    Os << "Call argument";
+    if (!paramName.empty()) {
+      Os << " for parameter ";
+      printQuotedQualifiedName(Os, Param);
+    }
+
+    reportBug(CA->getExprLoc(), CA->getSourceRange(), DeclWithIssue, Os.str(),
+              Param->getType());
+  }
+
+  void reportUnknownRecieverType(const Expr *Receiver,
+                                 const Decl *DeclWithIssue) const {
+    assert(Receiver);
+    reportBug(Receiver->getExprLoc(), Receiver->getSourceRange(), DeclWithIssue,
+              "Receiver", Receiver->getType());
+  }
+
+  void reportBug(const SourceLocation &SrcLoc, const SourceRange &SrcRange,
+                 const Decl *DeclWithIssue, const StringRef &Description,
+                 QualType Type) const {
+    SmallString<100> Buf;
+    llvm::raw_svector_ostream Os(Buf);
+
+    const std::string TypeName = Type.getAsString();
+    Os << Description << " uses a forward declared type '" << TypeName << "'";
+
+    PathDiagnosticLocation BSLoc(SrcLoc, BR->getSourceManager());
+    auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+    Report->addRange(SrcRange);
+    Report->setDeclWithIssue(DeclWithIssue);
+    BR->emitReport(std::move(Report));
+  }
+};
+
+} // namespace
+
+void ento::registerForwardDeclChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<ForwardDeclChecker>();
+}
+
+bool ento::shouldRegisterForwardDeclChecker(const CheckerManager &) {
+  return true;
+}