[analyzer] UncountedCallArgsChecker: Detect & ignore trivial function calls. (#81808)

This PR introduces the concept of a "trivial function" which applies to
a function that only calls other trivial functions and contain literals
and expressions that don't result in heap mutations (specifically it
does not call deref). This is implemented using ConstStmtVisitor and
checking each statement and expression's trivialness.

This PR also introduces the concept of a "ingleton function", which is a
static member function or a free standing function which ends with the
suffix "singleton". Such a function's return value is understood to be
safe to call any function with.

Author: rniwa

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;