[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 17:28:49 PST 2024
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/82063
>From 8d304967acfd0881a4844d630dab5f954772490c 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
This PR permits the use of WebKit's ASSERT macros as well as std::atomic<T> operations to appear
within a trivial function. Also exempt ref() and deref() member function calls.
---
.../Checkers/WebKit/ASTUtils.cpp | 4 ++
.../Checkers/WebKit/PtrTypesSemantics.cpp | 18 +++++++++
.../WebKit/UncountedCallArgsChecker.cpp | 5 +++
.../call-args-protected-return-value.cpp | 6 ++-
.../Analysis/Checkers/WebKit/mock-types.h | 20 +++++++++-
.../Checkers/WebKit/uncounted-obj-arg.cpp | 39 +++++++++++++------
6 files changed, 77 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..8d344f9b63961a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -73,6 +73,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 =
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..ac16a31293f3de 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 test2() {
+ someFunction(*object());
+}
More information about the cfe-commits
mailing list