[clang] [alpha.webkit.UncountedCallArgsChecker] Allow ASSERT and atomic<T> operations in a trivial function (PR #82063)

Ryosuke Niwa via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 16 14:56:47 PST 2024


https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/82063

None

>From ddaf7bf835668b761e82c25232f2e281a002ff78 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 16 Feb 2024 14:54:55 -0800
Subject: [PATCH] [alpha.webkit.UncountedCallArgsChecker] Allow ASSERT and
 atomic<T> operations in a trivial function

---
 .../Checkers/WebKit/ASTUtils.cpp              |  4 ++
 .../Checkers/WebKit/PtrTypesSemantics.cpp     | 18 +++++++++
 .../WebKit/UncountedCallArgsChecker.cpp       |  9 +++++
 .../call-args-protected-return-value.cpp      |  6 ++-
 .../Analysis/Checkers/WebKit/mock-types.h     | 20 +++++++++-
 .../Checkers/WebKit/uncounted-obj-arg.cpp     | 39 +++++++++++++------
 6 files changed, 81 insertions(+), 15 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 94eaa81af51772..1a9d6d3127fb7f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/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 =
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 6f236db0474079..c713293924c45e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -315,6 +315,10 @@ class TrivialFunctionAnalysisVisitor
     return false;
   }
 
+  bool VisitAtomicExpr(const AtomicExpr* E) {
+    return VisitChildren(E);
+  }
+
   bool VisitStaticAssertDecl(const StaticAssertDecl *SAD) {
     // Any static_assert is considered trivial.
     return true;
@@ -330,12 +334,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;
@@ -356,6 +366,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))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 17a64e1b1b8e04..1bed21d18ed74d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -53,6 +53,15 @@ class UncountedCallArgsChecker
       bool shouldVisitTemplateInstantiations() const { return true; }
       bool shouldVisitImplicitCode() const { return false; }
 
+      bool VisitCXXMethodDecl(const CXXMethodDecl *D) {
+        if (auto* Class = D->getParent()) {
+          auto name = safeGetName(Class);
+          if (isRefCounted(Class))
+            return false; // Don't visit contents of Ref/RefPtr methods.
+        }
+        return true;
+      }
+
       bool VisitCallExpr(const CallExpr *CE) {
         Checker->visitCallExpr(CE);
         return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp
index 1c4b3df211b1e3..6a8b7464845a26 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp
@@ -5,12 +5,14 @@
 
 class RefCounted {
 public:
-  void ref();
-  void deref();
+  void ref() const;
+  void deref() const;
 };
 
 class Object {
 public:
+  void ref() const;
+  void deref() const;
   void someFunction(RefCounted&);
 };
 
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index d08a997aa8c043..82db67bb031dd6 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -5,7 +5,15 @@ template <typename T> struct Ref {
   T *t;
 
   Ref() : t{} {};
-  Ref(T *) {}
+  Ref(T &t)
+    : t(t) {
+    if (t)
+      t->ref();
+  }
+  ~Ref() {
+    if (t)
+      t->deref();
+  }
   T *get() { return t; }
   T *ptr() { return t; }
   operator const T &() const { return *t; }
@@ -16,7 +24,15 @@ template <typename T> struct RefPtr {
   T *t;
 
   RefPtr() : t(new T) {}
-  RefPtr(T *t) : t(t) {}
+  RefPtr(T *t)
+    : t(t) {
+    if (t)
+      t->ref();
+  }
+  ~RefPtr() {
+    if (t)
+      t->deref();
+  }
   T *get() { return t; }
   T *operator->() { return t; }
   const T *operator->() const { return t; }
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index 957dff74ffce46..22886102578c16 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -4,6 +4,7 @@
 
 void WTFBreakpointTrap();
 void WTFCrashWithInfo(int, const char*, const char*, int);
+void WTFReportAssertionFailure(const char* file, int line, const char* function, const char* assertion);
 
 inline void compilerFenceForCrash()
 {
@@ -31,21 +32,19 @@ void isIntegralOrPointerType(T, Types... types)
         CRASH_WITH_INFO(__VA_ARGS__); \
 } while (0)
 
-#if !defined(NOT_TAIL_CALLED)
-#if __has_attribute(not_tail_called)
-#define NOT_TAIL_CALLED __attribute__((not_tail_called))
-#else
-#define NOT_TAIL_CALLED
-#endif
-#endif
-#define NO_RETURN_DUE_TO_CRASH
+#define ASSERT(assertion, ...) do { \
+    if (!(assertion)) { \
+        WTFReportAssertionFailure(__FILE__, __LINE__, __PRETTY_FUNCTION__, #assertion); \
+        CRASH_WITH_INFO(__VA_ARGS__); \
+    } \
+} while (0)
 
 #if !defined(ALWAYS_INLINE)
 #define ALWAYS_INLINE inline
 #endif
 
-NO_RETURN_DUE_TO_CRASH NOT_TAIL_CALLED void WTFCrashWithInfoImpl(int line, const char* file, const char* function, int counter, unsigned long reason);
-NO_RETURN_DUE_TO_CRASH NOT_TAIL_CALLED void WTFCrashWithInfo(int line, const char* file, const char* function, int counter);
+void WTFCrashWithInfoImpl(int line, const char* file, const char* function, int counter, unsigned long reason);
+void WTFCrashWithInfo(int line, const char* file, const char* function, int counter);
 
 template<typename T>
 ALWAYS_INLINE unsigned long wtfCrashArg(T* arg) { return reinterpret_cast<unsigned long>(arg); }
@@ -54,7 +53,7 @@ template<typename T>
 ALWAYS_INLINE unsigned long wtfCrashArg(T arg) { return arg; }
 
 template<typename T>
-NO_RETURN_DUE_TO_CRASH ALWAYS_INLINE void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, T reason)
+void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, T reason)
 {
     WTFCrashWithInfoImpl(line, file, function, counter, wtfCrashArg(reason));
 }
@@ -198,6 +197,8 @@ class RefCounted {
   bool trivial22() { return enumValue == Enum::Value1; }
 
   bool trivial23() const { return OptionSet<Flags>::fromRaw(v).contains(Flags::Flag1); }
+  int trivial24() const { ASSERT(v); return v; }
+  unsigned trivial25() const { return __c11_atomic_load((volatile _Atomic(unsigned) *)&v, __ATOMIC_RELAXED); }
 
   static RefCounted& singleton() {
     static RefCounted s_RefCounted;
@@ -256,6 +257,11 @@ class RefCounted {
     }
   }
 
+  static unsigned* another();
+  unsigned nonTrivial10() const {
+    return __c11_atomic_load((volatile _Atomic(unsigned) *)another(), __ATOMIC_RELAXED);
+  }
+
   unsigned v { 0 };
   Number* number { nullptr };
   Enum enumValue { Enum::Value1 };
@@ -301,6 +307,8 @@ class UnrelatedClass {
     getFieldTrivial().trivial21(); // no-warning
     getFieldTrivial().trivial22(); // no-warning
     getFieldTrivial().trivial23(); // no-warning
+    getFieldTrivial().trivial24(); // no-warning
+    getFieldTrivial().trivial25(); // no-warning
     RefCounted::singleton().trivial18(); // no-warning
     RefCounted::singleton().someFunction(); // no-warning
 
@@ -324,6 +332,8 @@ class UnrelatedClass {
     // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
     getFieldTrivial().nonTrivial9();
     // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+    getFieldTrivial().nonTrivial10();
+    // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
   }
 };
 
@@ -342,3 +352,10 @@ class UnrelatedClass2 {
     // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
   }
 };
+
+RefPtr<RefCounted> object();
+void someFunction(const RefCounted&);
+
+void test() {
+    someFunction(*object());
+}



More information about the cfe-commits mailing list