[clang] [alpha.webkit.UncountedCallArgsChecker] Allow ASSERT and atomic<T> operations in a trivial function (PR #82063)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 16 14:57:18 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Ryosuke Niwa (rniwa)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/82063.diff
6 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+4)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+18)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp (+9)
- (modified) clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp (+4-2)
- (modified) clang/test/Analysis/Checkers/WebKit/mock-types.h (+18-2)
- (modified) clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp (+28-11)
``````````diff
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());
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/82063
More information about the cfe-commits
mailing list