[alpha.webkit.UncountedCallArgsChecker] Allow ASSERT and atomic<T> operations in a trivial function

This PR permits the use of WebKit's ASSERT macros as well as std::atomic<T> operations to appear
within a trivial function. It also skips checkers on methods of Ref/RefPtr types.

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

@@ -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))

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;

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&);
 };
 

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; }

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@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
     getFieldTrivial().nonTrivial9();
     // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
+    getFieldTrivial().nonTrivial10();
+    // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
   }
 };
 
@@ -342,3 +352,10 @@ class UnrelatedClass2 {
     // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}}
   }
 };
+
+RefPtr<RefCounted> object();
+void someFunction(const RefCounted&);
+
+void test() {
+    someFunction(*object());
+}