[alpha.webkit.UncountedCallArgsChecker] Detect & ignore trivial function calls.

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

@@ -66,9 +66,13 @@ tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) {
           E = call->getArg(0);
           continue;
         }
+
         if (isReturnValueRefCounted(callee))
           return {E, true};
 
+        if (isSingleton(callee))
+          return {E, true};
+
         if (isPtrConversion(callee)) {
           E = call->getArg(0);
           continue;

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

@@ -12,6 +12,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/StmtVisitor.h"
 #include <optional>
 
 using namespace clang;
@@ -222,4 +223,216 @@ bool isPtrConversion(const FunctionDecl *F) {
   return false;
 }
 
+bool isSingleton(const FunctionDecl *F) {
+  assert(F);
+  // FIXME: check # of params == 1
+  if (auto *MethodDecl = dyn_cast<CXXMethodDecl>(F)) {
+    if (!MethodDecl->isStatic())
+      return false;
+  }
+  const auto &Name = safeGetName(F);
+  std::string SingletonStr = "singleton";
+  auto index = Name.find(SingletonStr);
+  return index != std::string::npos &&
+         index == Name.size() - SingletonStr.size();
+}
+
+// We only care about statements so let's use the simple
+// (non-recursive) visitor.
+class TrivialFunctionAnalysisVisitor
+    : public ConstStmtVisitor<TrivialFunctionAnalysisVisitor, bool> {
+
+  // Returns false if at least one child is non-trivial.
+  bool VisitChildren(const Stmt *S) {
+    for (const Stmt *Child : S->children()) {
+      if (Child && !Visit(Child))
+        return false;
+    }
+
+    return true;
+  }
+
+public:
+  using CacheTy = TrivialFunctionAnalysis::CacheTy;
+
+  TrivialFunctionAnalysisVisitor(CacheTy &Cache) : Cache(Cache) {}
+
+  bool VisitStmt(const Stmt *S) {
+    // All statements are non-trivial unless overriden later.
+    // Don't even recurse into children by default.
+    return false;
+  }
+
+  bool VisitCompoundStmt(const CompoundStmt *CS) {
+    // A compound statement is allowed as long each individual sub-statement
+    // is trivial.
+    return VisitChildren(CS);
+  }
+
+  bool VisitReturnStmt(const ReturnStmt *RS) {
+    // A return statement is allowed as long as the return value is trivial.
+    if (auto *RV = RS->getRetValue())
+      return Visit(RV);
+    return true;
+  }
+
+  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 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)
+      return Visit(UO->getSubExpr());
+
+    // Other operators are non-trivial.
+    return false;
+  }
+
+  bool VisitBinaryOperator(const BinaryOperator *BO) {
+    // Binary operators are trivial if their operands are trivial.
+    return Visit(BO->getLHS()) && Visit(BO->getRHS());
+  }
+
+  bool VisitConditionalOperator(const ConditionalOperator *CO) {
+    // Ternary operators are trivial if their conditions & values are trivial.
+    return VisitChildren(CO);
+  }
+
+  bool VisitDeclRefExpr(const DeclRefExpr *DRE) {
+    if (auto *decl = DRE->getDecl()) {
+      if (isa<ParmVarDecl>(decl))
+        return true;
+    }
+    return false;
+  }
+
+  bool VisitStaticAssertDecl(const StaticAssertDecl *SAD) {
+    // Any static_assert is considered trivial.
+    return true;
+  }
+
+  bool VisitCallExpr(const CallExpr *CE) {
+    if (!checkArguments(CE))
+      return false;
+
+    auto *Callee = CE->getDirectCallee();
+    if (!Callee)
+      return false;
+    const auto &Name = safeGetName(Callee);
+
+    if (Name == "WTFCrashWithInfo" || Name == "WTFBreakpointTrap" ||
+        Name == "compilerFenceForCrash" || Name == "__builtin_unreachable")
+      return true;
+
+    return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
+  }
+
+  bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
+    if (!checkArguments(MCE))
+      return false;
+
+    bool TrivialThis = Visit(MCE->getImplicitObjectArgument());
+    if (!TrivialThis)
+      return false;
+
+    auto *Callee = MCE->getMethodDecl();
+    if (!Callee)
+      return false;
+
+    std::optional<bool> IsGetterOfRefCounted = isGetterOfRefCounted(Callee);
+    if (IsGetterOfRefCounted && *IsGetterOfRefCounted)
+      return true;
+
+    // Recursively descend into the callee to confirm that it's trivial as well.
+    return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
+  }
+
+  bool checkArguments(const CallExpr *CE) {
+    for (const Expr *Arg : CE->arguments()) {
+      if (Arg && !Visit(Arg))
+        return false;
+    }
+    return true;
+  }
+
+  bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
+    for (const Expr *Arg : CE->arguments()) {
+      if (Arg && !Visit(Arg))
+        return false;
+    }
+
+    // Recursively descend into the callee to confirm that it's trivial.
+    return TrivialFunctionAnalysis::isTrivialImpl(CE->getConstructor(), Cache);
+  }
+
+  bool VisitImplicitCastExpr(const ImplicitCastExpr *ICE) {
+    return Visit(ICE->getSubExpr());
+  }
+
+  bool VisitExplicitCastExpr(const ExplicitCastExpr *ECE) {
+    return Visit(ECE->getSubExpr());
+  }
+
+  bool VisitParenExpr(const ParenExpr *PE) { return Visit(PE->getSubExpr()); }
+
+  bool VisitInitListExpr(const InitListExpr *ILE) {
+    for (const Expr *Child : ILE->inits()) {
+      if (Child && !Visit(Child))
+        return false;
+    }
+    return true;
+  }
+
+  bool VisitMemberExpr(const MemberExpr *ME) {
+    // Field access is allowed but the base pointer may itself be non-trivial.
+    return Visit(ME->getBase());
+  }
+
+  bool VisitCXXThisExpr(const CXXThisExpr *CTE) {
+    // The expression 'this' is always trivial, be it explicit or implicit.
+    return true;
+  }
+
+  // Constant literal expressions are always trivial
+  bool VisitIntegerLiteral(const IntegerLiteral *E) { return true; }
+  bool VisitFloatingLiteral(const FloatingLiteral *E) { return true; }
+  bool VisitFixedPointLiteral(const FixedPointLiteral *E) { return true; }
+  bool VisitCharacterLiteral(const CharacterLiteral *E) { return true; }
+  bool VisitStringLiteral(const StringLiteral *E) { return true; }
+
+  bool VisitConstantExpr(const ConstantExpr *CE) {
+    // Constant expressions are trivial.
+    return true;
+  }
+
+private:
+  CacheTy Cache;
+};
+
+bool TrivialFunctionAnalysis::isTrivialImpl(
+    const Decl *D, TrivialFunctionAnalysis::CacheTy &Cache) {
+  // If the function isn't in the cache, conservatively assume that
+  // it's not trivial until analysis completes. This makes every recursive
+  // function non-trivial. This also guarantees that each function
+  // will be scanned at most once.
+  auto [It, IsNew] = Cache.insert(std::make_pair(D, false));
+  if (!IsNew)
+    return It->second;
+
+  const Stmt *Body = D->getBody();
+  if (!Body)
+    return false;
+
+  TrivialFunctionAnalysisVisitor V(Cache);
+  bool Result = V.Visit(Body);
+  if (Result)
+    Cache[D] = true;
+
+  return Result;
+}
+
 } // namespace clang

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

@@ -10,12 +10,14 @@
 #define LLVM_CLANG_ANALYZER_WEBKIT_PTRTYPESEMANTICS_H
 
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/DenseMap.h"
 #include <optional>
 
 namespace clang {
 class CXXBaseSpecifier;
 class CXXMethodDecl;
 class CXXRecordDecl;
+class Decl;
 class FunctionDecl;
 class Type;
 
@@ -60,6 +62,25 @@ std::optional<bool> isGetterOfRefCounted(const clang::CXXMethodDecl* Method);
 /// pointer types.
 bool isPtrConversion(const FunctionDecl *F);
 
+/// \returns true if \p F is a static singleton function.
+bool isSingleton(const FunctionDecl *F);
+
+/// An inter-procedural analysis facility that detects functions with "trivial"
+/// behavior with respect to reference counting, such as simple field getters.
+class TrivialFunctionAnalysis {
+public:
+  /// \returns true if \p D is a "trivial" function.
+  bool isTrivial(const Decl *D) const { return isTrivialImpl(D, TheCache); }
+
+private:
+  friend class TrivialFunctionAnalysisVisitor;
+
+  using CacheTy = llvm::DenseMap<const Decl *, bool>;
+  mutable CacheTy TheCache{};
+
+  static bool isTrivialImpl(const Decl *D, CacheTy &Cache);
+};
+
 } // namespace clang
 
 #endif

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

@@ -32,6 +32,8 @@ class UncountedCallArgsChecker
             "WebKit coding guidelines"};
   mutable BugReporter *BR;
 
+  TrivialFunctionAnalysis TFA;
+
 public:
 
   void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -134,6 +136,11 @@ class UncountedCallArgsChecker
   }
 
   bool shouldSkipCall(const CallExpr *CE) const {
+    const auto *Callee = CE->getDirectCallee();
+
+    if (Callee && TFA.isTrivial(Callee))
+      return true;
+
     if (CE->getNumArgs() == 0)
       return false;
 
@@ -155,7 +162,6 @@ class UncountedCallArgsChecker
         return false;
     }
 
-    const auto *Callee = CE->getDirectCallee();
     if (!Callee)
       return false;