[clang] [alpha.webkit.UncountedCallArgsChecker] Allow trivial operator++ (PR #91102)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Thu May 9 10:52:28 PDT 2024
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/91102
>From b3ae3d7a2a8885777b691e7fde237f5745e764a1 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sat, 4 May 2024 20:29:03 -0700
Subject: [PATCH 1/4] [alpha.webkit.UncountedCallArgsChecker] Allow trivial
operator++
This PR adds the support for trivial operator++ implementations.
T& operator++() and T operator++(int) are trivial if the calle is trivial.
Also allow incrementing and decrementing of a POD member variable.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 17 ++++++++++-
.../Checkers/WebKit/uncounted-obj-arg.cpp | 28 +++++++++++++++++++
2 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 6901dbb415bf7..c219172477a62 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -316,10 +316,15 @@ class TrivialFunctionAnalysisVisitor
if (UO->isIncrementOp() || UO->isDecrementOp()) {
// Allow increment or decrement of a POD type.
- if (auto *RefExpr = dyn_cast<DeclRefExpr>(UO->getSubExpr())) {
+ auto *SubExpr = UO->getSubExpr();
+ if (auto *RefExpr = dyn_cast<DeclRefExpr>(SubExpr)) {
if (auto *Decl = dyn_cast<VarDecl>(RefExpr->getDecl()))
return Decl->isLocalVarDeclOrParm() &&
Decl->getType().isPODType(Decl->getASTContext());
+ } else if (auto *ME = dyn_cast<MemberExpr>(SubExpr)) {
+ if (auto *Decl = ME->getMemberDecl())
+ return Visit(ME->getBase()) &&
+ Decl->getType().isPODType(Decl->getASTContext());
}
}
// Other operators are non-trivial.
@@ -405,6 +410,16 @@ class TrivialFunctionAnalysisVisitor
return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
}
+ bool VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
+ if (!checkArguments(OCE))
+ return false;
+ auto *Callee = OCE->getCalleeDecl();
+ if (!Callee)
+ return false;
+ // Recursively descend into the callee to confirm that it's trivial as well.
+ return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
+ }
+
bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E) {
if (auto *Expr = E->getExpr()) {
if (!Visit(Expr))
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index 63a68a994a5c6..2816a9cb82364 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -137,11 +137,26 @@ class Number {
Number(int v) : v(v) { }
Number(double);
Number operator+(const Number&);
+ Number& operator++() { ++v; return *this; }
+ Number operator++(int) { Number returnValue(v); ++v; return returnValue; }
const int& value() const { return v; }
+ void someMethod();
+
private:
int v;
};
+class ComplexNumber {
+public:
+ ComplexNumber() : real(0), complex(0) { }
+ ComplexNumber& operator++() { real.someMethod(); return *this; }
+ ComplexNumber operator++(int);
+
+private:
+ Number real;
+ Number complex;
+};
+
class RefCounted {
public:
void ref() const;
@@ -208,6 +223,9 @@ class RefCounted {
unsigned trivial32() { return sizeof(int); }
unsigned trivial33() { return ~0xff; }
template <unsigned v> unsigned trivial34() { return v; }
+ void trivial35() { v++; }
+ void trivial36() { ++(*number); }
+ void trivial37() { (*number)++; }
static RefCounted& singleton() {
static RefCounted s_RefCounted;
@@ -282,9 +300,12 @@ class RefCounted {
int nonTrivial13() { return ~otherFunction(); }
int nonTrivial14() { int r = 0xff; r |= otherFunction(); return r; }
+ void nonTrivial15() { ++complex; }
+ void nonTrivial16() { complex++; }
unsigned v { 0 };
Number* number { nullptr };
+ ComplexNumber complex;
Enum enumValue { Enum::Value1 };
};
@@ -340,6 +361,9 @@ class UnrelatedClass {
getFieldTrivial().trivial32(); // no-warning
getFieldTrivial().trivial33(); // no-warning
getFieldTrivial().trivial34<7>(); // no-warning
+ getFieldTrivial().trivial35(); // no-warning
+ getFieldTrivial().trivial36(); // no-warning
+ getFieldTrivial().trivial37(); // no-warning
RefCounted::singleton().trivial18(); // no-warning
RefCounted::singleton().someFunction(); // no-warning
@@ -374,6 +398,10 @@ class UnrelatedClass {
// expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
getFieldTrivial().nonTrivial14();
// expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial15();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial16();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
}
};
>From b625bebb95bfc224dc299579fd6da1e480f6796c Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Sun, 5 May 2024 12:33:38 -0700
Subject: [PATCH 2/4] Also treat any __builtin_ functions as trivial.
---
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 2 +-
clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index c219172477a62..5837872847bb6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -369,7 +369,7 @@ class TrivialFunctionAnalysisVisitor
if (Name == "WTFCrashWithInfo" || Name == "WTFBreakpointTrap" ||
Name == "WTFReportAssertionFailure" ||
- Name == "compilerFenceForCrash" || Name == "__builtin_unreachable")
+ Name == "compilerFenceForCrash" || Name.find("__builtin") == 0)
return true;
return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index 2816a9cb82364..d4b4916ad7e29 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -226,6 +226,7 @@ class RefCounted {
void trivial35() { v++; }
void trivial36() { ++(*number); }
void trivial37() { (*number)++; }
+ void trivial38() { v++; if (__builtin_expect(!!(number), 1)) (*number)++; }
static RefCounted& singleton() {
static RefCounted s_RefCounted;
@@ -364,6 +365,7 @@ class UnrelatedClass {
getFieldTrivial().trivial35(); // no-warning
getFieldTrivial().trivial36(); // no-warning
getFieldTrivial().trivial37(); // no-warning
+ getFieldTrivial().trivial38(); // no-warning
RefCounted::singleton().trivial18(); // no-warning
RefCounted::singleton().someFunction(); // no-warning
>From 800dc2041b9b36b4a9cccfef7472fd06a75b6f4a Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 7 May 2024 18:59:56 -0700
Subject: [PATCH 3/4] Allow any unary operator so long as its subexpression is
trivial.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 22 ++-----------------
.../Checkers/WebKit/uncounted-obj-arg.cpp | 13 +++++++++++
2 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 5837872847bb6..16d573b46d6a7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -309,26 +309,8 @@ class TrivialFunctionAnalysisVisitor
bool VisitDefaultStmt(const DefaultStmt *DS) { return VisitChildren(DS); }
bool VisitUnaryOperator(const UnaryOperator *UO) {
- // Operator '*' and '!' are allowed as long as the operand is trivial.
- auto op = UO->getOpcode();
- if (op == UO_Deref || op == UO_AddrOf || op == UO_LNot || op == UO_Not)
- return Visit(UO->getSubExpr());
-
- if (UO->isIncrementOp() || UO->isDecrementOp()) {
- // Allow increment or decrement of a POD type.
- auto *SubExpr = UO->getSubExpr();
- if (auto *RefExpr = dyn_cast<DeclRefExpr>(SubExpr)) {
- if (auto *Decl = dyn_cast<VarDecl>(RefExpr->getDecl()))
- return Decl->isLocalVarDeclOrParm() &&
- Decl->getType().isPODType(Decl->getASTContext());
- } else if (auto *ME = dyn_cast<MemberExpr>(SubExpr)) {
- if (auto *Decl = ME->getMemberDecl())
- return Visit(ME->getBase()) &&
- Decl->getType().isPODType(Decl->getASTContext());
- }
- }
- // Other operators are non-trivial.
- return false;
+ // Unary operators are trivial if its operand is trivial.
+ return Visit(UO->getSubExpr());
}
bool VisitBinaryOperator(const BinaryOperator *BO) {
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index d4b4916ad7e29..b8cf25e3ef282 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -149,8 +149,11 @@ class Number {
class ComplexNumber {
public:
ComplexNumber() : real(0), complex(0) { }
+ ComplexNumber(const ComplexNumber&);
ComplexNumber& operator++() { real.someMethod(); return *this; }
ComplexNumber operator++(int);
+ ComplexNumber& operator<<(int);
+ ComplexNumber& operator+();
private:
Number real;
@@ -227,6 +230,8 @@ class RefCounted {
void trivial36() { ++(*number); }
void trivial37() { (*number)++; }
void trivial38() { v++; if (__builtin_expect(!!(number), 1)) (*number)++; }
+ int trivial39() { return -v; }
+ int trivial40() { return v << 2; }
static RefCounted& singleton() {
static RefCounted s_RefCounted;
@@ -303,6 +308,8 @@ class RefCounted {
int nonTrivial14() { int r = 0xff; r |= otherFunction(); return r; }
void nonTrivial15() { ++complex; }
void nonTrivial16() { complex++; }
+ ComplexNumber nonTrivial17() { return complex << 2; }
+ ComplexNumber nonTrivial18() { return +complex; }
unsigned v { 0 };
Number* number { nullptr };
@@ -366,6 +373,8 @@ class UnrelatedClass {
getFieldTrivial().trivial36(); // no-warning
getFieldTrivial().trivial37(); // no-warning
getFieldTrivial().trivial38(); // no-warning
+ getFieldTrivial().trivial39(); // no-warning
+ getFieldTrivial().trivial40(); // no-warning
RefCounted::singleton().trivial18(); // no-warning
RefCounted::singleton().someFunction(); // no-warning
@@ -404,6 +413,10 @@ class UnrelatedClass {
// expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
getFieldTrivial().nonTrivial16();
// expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial17();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial18();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
}
};
>From 3516c098a9a63f43f3f9b0bb41704adcf2e954ad Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Thu, 9 May 2024 10:51:56 -0700
Subject: [PATCH 4/4] Exclude co_await from triviality.
---
.../lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 16d573b46d6a7..3abfa4cbb295d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -309,8 +309,8 @@ class TrivialFunctionAnalysisVisitor
bool VisitDefaultStmt(const DefaultStmt *DS) { return VisitChildren(DS); }
bool VisitUnaryOperator(const UnaryOperator *UO) {
- // Unary operators are trivial if its operand is trivial.
- return Visit(UO->getSubExpr());
+ // Unary operators are trivial if its operand is trivial except co_await.
+ return UO->getOpcode() != UO_Coawait && Visit(UO->getSubExpr());
}
bool VisitBinaryOperator(const BinaryOperator *BO) {
More information about the cfe-commits
mailing list