Merge pull request #8349 from rniwa/merge-webkit-checker-improvements

Merge WebKit checker improvements

Author: rniwa

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

@@ -23,6 +23,10 @@ tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) {
       E = tempExpr->getSubExpr();
       continue;
     }
+    if (auto *tempExpr = dyn_cast<CXXBindTemporaryExpr>(E)) {
+      E = tempExpr->getSubExpr();
+      continue;
+    }
     if (auto *cast = dyn_cast<CastExpr>(E)) {
       if (StopAtFirstRefCountedObj) {
         if (auto *ConversionFunc =

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

@@ -103,15 +103,18 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
   return hasRef && hasDeref;
 }
 
+bool isRefType(const std::string &Name) {
+  return Name == "Ref" || Name == "RefAllowingPartiallyDestroyed" ||
+         Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed";
+}
+
 bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
   assert(F);
-  const auto &FunctionName = safeGetName(F);
-
-  return FunctionName == "Ref" || FunctionName == "makeRef"
+  const std::string &FunctionName = safeGetName(F);
 
-         || FunctionName == "RefPtr" || FunctionName == "makeRefPtr"
-
-         || FunctionName == "UniqueRef" || FunctionName == "makeUniqueRef" ||
+  return isRefType(FunctionName) || FunctionName == "makeRef" ||
+         FunctionName == "makeRefPtr" || FunctionName == "UniqueRef" ||
+         FunctionName == "makeUniqueRef" ||
          FunctionName == "makeUniqueRefWithoutFastMallocCheck"
 
          || FunctionName == "String" || FunctionName == "AtomString" ||
@@ -131,7 +134,7 @@ bool isReturnValueRefCounted(const clang::FunctionDecl *F) {
     if (auto *specialT = type->getAs<TemplateSpecializationType>()) {
       if (auto *decl = specialT->getTemplateName().getAsTemplateDecl()) {
         auto name = decl->getNameAsString();
-        return name == "Ref" || name == "RefPtr";
+        return isRefType(name);
       }
       return false;
     }
@@ -172,20 +175,18 @@ std::optional<bool> isGetterOfRefCounted(const CXXMethodDecl* M)
   if (isa<CXXMethodDecl>(M)) {
     const CXXRecordDecl *calleeMethodsClass = M->getParent();
     auto className = safeGetName(calleeMethodsClass);
-    auto methodName = safeGetName(M);
+    auto method = safeGetName(M);
 
-    if (((className == "Ref" || className == "RefPtr") &&
-         methodName == "get") ||
-        (className == "Ref" && methodName == "ptr") ||
+    if ((isRefType(className) && (method == "get" || method == "ptr")) ||
         ((className == "String" || className == "AtomString" ||
           className == "AtomStringImpl" || className == "UniqueString" ||
           className == "UniqueStringImpl" || className == "Identifier") &&
-         methodName == "impl"))
+         method == "impl"))
       return true;
 
     // Ref<T> -> T conversion
     // FIXME: Currently allowing any Ref<T> -> whatever cast.
-    if (className == "Ref" || className == "RefPtr") {
+    if (isRefType(className)) {
       if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
         if (auto *targetConversionType =
                 maybeRefToRawOperator->getConversionType().getTypePtrOrNull()) {
@@ -202,7 +203,7 @@ bool isRefCounted(const CXXRecordDecl *R) {
   if (auto *TmplR = R->getTemplateInstantiationPattern()) {
     // FIXME: String/AtomString/UniqueString
     const auto &ClassName = safeGetName(TmplR);
-    return ClassName == "RefPtr" || ClassName == "Ref";
+    return isRefType(ClassName);
   }
   return false;
 }
@@ -252,6 +253,19 @@ class TrivialFunctionAnalysisVisitor
     return true;
   }
 
+  template <typename CheckFunction>
+  bool WithCachedResult(const Stmt *S, CheckFunction Function) {
+    // If the statement isn't in the cache, conservatively assume that
+    // it's not trivial until analysis completes. Insert false to the cache
+    // first to avoid infinite recursion.
+    auto [It, IsNew] = Cache.insert(std::make_pair(S, false));
+    if (!IsNew)
+      return It->second;
+    bool Result = Function();
+    Cache[S] = Result;
+    return Result;
+  }
+
 public:
   using CacheTy = TrivialFunctionAnalysis::CacheTy;
 
@@ -266,7 +280,7 @@ class TrivialFunctionAnalysisVisitor
   bool VisitCompoundStmt(const CompoundStmt *CS) {
     // A compound statement is allowed as long each individual sub-statement
     // is trivial.
-    return VisitChildren(CS);
+    return WithCachedResult(CS, [&]() { return VisitChildren(CS); });
   }
 
   bool VisitReturnStmt(const ReturnStmt *RS) {
@@ -278,16 +292,36 @@ class TrivialFunctionAnalysisVisitor
 
   bool VisitDeclStmt(const DeclStmt *DS) { return VisitChildren(DS); }
   bool VisitDoStmt(const DoStmt *DS) { return VisitChildren(DS); }
-  bool VisitIfStmt(const IfStmt *IS) { return VisitChildren(IS); }
+  bool VisitIfStmt(const IfStmt *IS) {
+    return WithCachedResult(IS, [&]() { return VisitChildren(IS); });
+  }
+  bool VisitForStmt(const ForStmt *FS) {
+    return WithCachedResult(FS, [&]() { return VisitChildren(FS); });
+  }
+  bool VisitCXXForRangeStmt(const CXXForRangeStmt *FS) {
+    return WithCachedResult(FS, [&]() { return VisitChildren(FS); });
+  }
+  bool VisitWhileStmt(const WhileStmt *WS) {
+    return WithCachedResult(WS, [&]() { return VisitChildren(WS); });
+  }
   bool VisitSwitchStmt(const SwitchStmt *SS) { return VisitChildren(SS); }
   bool VisitCaseStmt(const CaseStmt *CS) { return VisitChildren(CS); }
   bool VisitDefaultStmt(const DefaultStmt *DS) { return VisitChildren(DS); }
 
   bool VisitUnaryOperator(const UnaryOperator *UO) {
     // Operator '*' and '!' are allowed as long as the operand is trivial.
-    if (UO->getOpcode() == UO_Deref || UO->getOpcode() == UO_LNot)
+    auto op = UO->getOpcode();
+    if (op == UO_Deref || op == UO_AddrOf || op == UO_LNot)
       return Visit(UO->getSubExpr());
 
+    if (UO->isIncrementOp() || UO->isDecrementOp()) {
+      // Allow increment or decrement of a POD type.
+      if (auto *RefExpr = dyn_cast<DeclRefExpr>(UO->getSubExpr())) {
+        if (auto *Decl = dyn_cast<VarDecl>(RefExpr->getDecl()))
+          return Decl->isLocalVarDeclOrParm() &&
+                 Decl->getType().isPODType(Decl->getASTContext());
+      }
+    }
     // Other operators are non-trivial.
     return false;
   }
@@ -302,13 +336,7 @@ class TrivialFunctionAnalysisVisitor
     return VisitChildren(CO);
   }
 
-  bool VisitDeclRefExpr(const DeclRefExpr *DRE) {
-    if (auto *decl = DRE->getDecl()) {
-      if (isa<ParmVarDecl>(decl))
-        return true;
-    }
-    return false;
-  }
+  bool VisitAtomicExpr(const AtomicExpr *E) { return VisitChildren(E); }
 
   bool VisitStaticAssertDecl(const StaticAssertDecl *SAD) {
     // Any static_assert is considered trivial.
@@ -325,12 +353,18 @@ class TrivialFunctionAnalysisVisitor
     const auto &Name = safeGetName(Callee);
 
     if (Name == "WTFCrashWithInfo" || Name == "WTFBreakpointTrap" ||
+        Name == "WTFReportAssertionFailure" ||
         Name == "compilerFenceForCrash" || Name == "__builtin_unreachable")
       return true;
 
     return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
   }
 
+  bool VisitPredefinedExpr(const PredefinedExpr *E) {
+    // A predefined identifier such as "func" is considered trivial.
+    return true;
+  }
+
   bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
     if (!checkArguments(MCE))
       return false;
@@ -351,6 +385,14 @@ class TrivialFunctionAnalysisVisitor
     return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
   }
 
+  bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E) {
+    if (auto *Expr = E->getExpr()) {
+      if (!Visit(Expr))
+        return false;
+    }
+    return true;
+  }
+
   bool checkArguments(const CallExpr *CE) {
     for (const Expr *Arg : CE->arguments()) {
       if (Arg && !Visit(Arg))
@@ -377,6 +419,14 @@ class TrivialFunctionAnalysisVisitor
     return Visit(ECE->getSubExpr());
   }
 
+  bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *VMT) {
+    return Visit(VMT->getSubExpr());
+  }
+
+  bool VisitExprWithCleanups(const ExprWithCleanups *EWC) {
+    return Visit(EWC->getSubExpr());
+  }
+
   bool VisitParenExpr(const ParenExpr *PE) { return Visit(PE->getSubExpr()); }
 
   bool VisitInitListExpr(const InitListExpr *ILE) {
@@ -397,6 +447,16 @@ class TrivialFunctionAnalysisVisitor
     return true;
   }
 
+  bool VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E) {
+    // nullptr is trivial.
+    return true;
+  }
+
+  bool VisitDeclRefExpr(const DeclRefExpr *DRE) {
+    // The use of a variable is trivial.
+    return true;
+  }
+
   // Constant literal expressions are always trivial
   bool VisitIntegerLiteral(const IntegerLiteral *E) { return true; }
   bool VisitFloatingLiteral(const FloatingLiteral *E) { return true; }
@@ -410,7 +470,7 @@ class TrivialFunctionAnalysisVisitor
   }
 
 private:
-  CacheTy Cache;
+  CacheTy &Cache;
 };
 
 bool TrivialFunctionAnalysis::isTrivialImpl(
@@ -435,4 +495,17 @@ bool TrivialFunctionAnalysis::isTrivialImpl(
   return Result;
 }
 
+bool TrivialFunctionAnalysis::isTrivialImpl(
+    const Stmt *S, TrivialFunctionAnalysis::CacheTy &Cache) {
+  // If the statement isn't in the cache, conservatively assume that
+  // it's not trivial until analysis completes. Unlike a function case,
+  // we don't insert an entry into the cache until Visit returns
+  // since Visit* functions themselves make use of the cache.
+
+  TrivialFunctionAnalysisVisitor V(Cache);
+  bool Result = V.Visit(S);
+  assert(Cache.contains(S) && "Top-level statement not properly cached!");
+  return Result;
+}
+
 } // namespace clang

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h

@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/PointerUnion.h"
 #include <optional>
 
 namespace clang {
@@ -19,6 +20,7 @@ class CXXMethodDecl;
 class CXXRecordDecl;
 class Decl;
 class FunctionDecl;
+class Stmt;
 class Type;
 
 // Ref-countability of a type is implicitly defined by Ref<T> and RefPtr<T>
@@ -71,14 +73,17 @@ class TrivialFunctionAnalysis {
 public:
   /// \returns true if \p D is a "trivial" function.
   bool isTrivial(const Decl *D) const { return isTrivialImpl(D, TheCache); }
+  bool isTrivial(const Stmt *S) const { return isTrivialImpl(S, TheCache); }
 
 private:
   friend class TrivialFunctionAnalysisVisitor;
 
-  using CacheTy = llvm::DenseMap<const Decl *, bool>;
+  using CacheTy =
+      llvm::DenseMap<llvm::PointerUnion<const Decl *, const Stmt *>, bool>;
   mutable CacheTy TheCache{};
 
   static bool isTrivialImpl(const Decl *D, CacheTy &Cache);
+  static bool isTrivialImpl(const Stmt *S, CacheTy &Cache);
 };
 
 } // namespace clang

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

@@ -74,6 +74,11 @@ class UncountedCallArgsChecker
       unsigned ArgIdx = isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F);
 
       if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) {
+        if (auto *MD = MemberCallExpr->getMethodDecl()) {
+          auto name = safeGetName(MD);
+          if (name == "ref" || name == "deref")
+            return;
+        }
         auto *E = MemberCallExpr->getImplicitObjectArgument();
         QualType ArgType = MemberCallExpr->getObjectType();
         std::optional<bool> IsUncounted =
@@ -166,6 +171,9 @@ class UncountedCallArgsChecker
     if (!Callee)
       return false;
 
+    if (isMethodOnWTFContainerType(Callee))
+      return true;
+
     auto overloadedOperatorType = Callee->getOverloadedOperator();
     if (overloadedOperatorType == OO_EqualEqual ||
         overloadedOperatorType == OO_ExclaimEqual ||
@@ -194,6 +202,31 @@ class UncountedCallArgsChecker
     return false;
   }
 
+  bool isMethodOnWTFContainerType(const FunctionDecl *Decl) const {
+    if (!isa<CXXMethodDecl>(Decl))
+      return false;
+    auto *ClassDecl = Decl->getParent();
+    if (!ClassDecl || !isa<CXXRecordDecl>(ClassDecl))
+      return false;
+
+    auto *NsDecl = ClassDecl->getParent();
+    if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
+      return false;
+
+    auto MethodName = safeGetName(Decl);
+    auto ClsNameStr = safeGetName(ClassDecl);
+    StringRef ClsName = ClsNameStr; // FIXME: Make safeGetName return StringRef.
+    auto NamespaceName = safeGetName(NsDecl);
+    // FIXME: These should be implemented via attributes.
+    return NamespaceName == "WTF" &&
+           (MethodName == "find" || MethodName == "findIf" ||
+            MethodName == "reverseFind" || MethodName == "reverseFindIf" ||
+            MethodName == "get" || MethodName == "inlineGet" ||
+            MethodName == "contains" || MethodName == "containsIf") &&
+           (ClsName.ends_with("Vector") || ClsName.ends_with("Set") ||
+            ClsName.ends_with("Map"));
+  }
+
   void reportBug(const Expr *CallArg, const ParmVarDecl *Param) const {
     assert(CallArg);