[clang] 44ae49e - Thread safety analysis: Handle compound assignment and ->* overloads
Aaron Puchert via cfe-commits
cfe-commits at lists.llvm.org
Mon May 9 06:35:52 PDT 2022
Author: Aaron Puchert
Date: 2022-05-09T15:35:43+02:00
New Revision: 44ae49e1a72576ca6aa8835b3f72df9605516403
URL: https://github.com/llvm/llvm-project/commit/44ae49e1a72576ca6aa8835b3f72df9605516403
DIFF: https://github.com/llvm/llvm-project/commit/44ae49e1a72576ca6aa8835b3f72df9605516403.diff
LOG: Thread safety analysis: Handle compound assignment and ->* overloads
Like regular assignment, compound assignment operators can be assumed to
write to their left-hand side operand. So we strengthen the requirements
there. (Previously only the default read access had been required.)
Just like operator->, operator->* can also be assumed to dereference the
left-hand side argument, so we require read access to the pointee. This
will generate new warnings if the left-hand side has a pt_guarded_by
attribute. This overload is rarely used, but it was trivial to add, so
why not. (Supporting the builtin operator requires changes to the TIL.)
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124966
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 73fb5a4f3c1f5..a82ae8845e65c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -204,6 +204,9 @@ Improvements to Clang's diagnostics
``-Wimplicit-int``.
- ``-Wmisexpect`` warns when the branch weights collected during profiling
conflict with those added by ``llvm.expect``.
+- ``-Wthread-safety-analysis`` now considers overloaded compound assignment and
+ increment/decrement operators as writing to their first argument, thus
+ requiring an exclusive lock if the argument is guarded.
Non-comprehensive list of changes in this release
-------------------------------------------------
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index b8fe6000d9716..03bbf078d7e89 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1988,16 +1988,27 @@ void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
examineArguments(CE->getDirectCallee(), CE->arg_begin(), CE->arg_end());
} else if (const auto *OE = dyn_cast<CXXOperatorCallExpr>(Exp)) {
- auto OEop = OE->getOperator();
+ OverloadedOperatorKind OEop = OE->getOperator();
switch (OEop) {
- case OO_Equal: {
- const Expr *Target = OE->getArg(0);
- const Expr *Source = OE->getArg(1);
- checkAccess(Target, AK_Written);
- checkAccess(Source, AK_Read);
+ case OO_Equal:
+ case OO_PlusEqual:
+ case OO_MinusEqual:
+ case OO_StarEqual:
+ case OO_SlashEqual:
+ case OO_PercentEqual:
+ case OO_CaretEqual:
+ case OO_AmpEqual:
+ case OO_PipeEqual:
+ case OO_LessLessEqual:
+ case OO_GreaterGreaterEqual:
+ checkAccess(OE->getArg(1), AK_Read);
+ LLVM_FALLTHROUGH;
+ case OO_PlusPlus:
+ case OO_MinusMinus:
+ checkAccess(OE->getArg(0), AK_Written);
break;
- }
case OO_Star:
+ case OO_ArrowStar:
case OO_Arrow:
case OO_Subscript:
if (!(OEop == OO_Star && OE->getNumArgs() > 1)) {
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 9cd44fb07230d..ea229fef649b9 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -82,6 +82,9 @@ class SmartPtr {
T* ptr_;
};
+template<typename T, typename U>
+U& operator->*(const SmartPtr<T>& ptr, U T::*p) { return ptr->*p; }
+
// For testing destructor calls and cleanup.
class MyString {
@@ -4338,6 +4341,21 @@ class Data {
void operator()() { }
+ Data& operator+=(int);
+ Data& operator-=(int);
+ Data& operator*=(int);
+ Data& operator/=(int);
+ Data& operator%=(int);
+ Data& operator^=(int);
+ Data& operator&=(int);
+ Data& operator|=(int);
+ Data& operator<<=(int);
+ Data& operator>>=(int);
+ Data& operator++();
+ Data& operator++(int);
+ Data& operator--();
+ Data& operator--(int);
+
private:
int dat;
};
@@ -4404,6 +4422,20 @@ class Foo {
// expected-warning {{reading variable 'datap1_' requires holding mutex 'mu_'}}
data_ = *datap2_; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}} \
// expected-warning {{reading the value pointed to by 'datap2_' requires holding mutex 'mu_'}}
+ data_ += 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ -= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ *= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ /= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ %= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ ^= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ &= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ |= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ <<= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_ >>= 1; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ ++data_; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_++; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ --data_; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+ data_--; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
data_[0] = 0; // expected-warning {{reading variable 'data_' requires holding mutex 'mu_'}}
(*datap2_)[0] = 0; // expected-warning {{reading the value pointed to by 'datap2_' requires holding mutex 'mu_'}}
@@ -4923,6 +4955,8 @@ class SmartPtr_PtGuardedBy_Test {
SmartPtr<int> sp GUARDED_BY(mu1) PT_GUARDED_BY(mu2);
SmartPtr<Cell> sq GUARDED_BY(mu1) PT_GUARDED_BY(mu2);
+ static constexpr int Cell::*pa = &Cell::a;
+
void test1() {
mu1.ReaderLock();
mu2.Lock();
@@ -4931,6 +4965,7 @@ class SmartPtr_PtGuardedBy_Test {
if (*sp == 0) doSomething();
*sp = 0;
sq->a = 0;
+ sq->*pa = 0;
if (sp[0] == 0) doSomething();
sp[0] = 0;
@@ -4946,6 +4981,7 @@ class SmartPtr_PtGuardedBy_Test {
if (*sp == 0) doSomething(); // expected-warning {{reading variable 'sp' requires holding mutex 'mu1'}}
*sp = 0; // expected-warning {{reading variable 'sp' requires holding mutex 'mu1'}}
sq->a = 0; // expected-warning {{reading variable 'sq' requires holding mutex 'mu1'}}
+ sq->*pa = 0; // expected-warning {{reading variable 'sq' requires holding mutex 'mu1'}}
if (sp[0] == 0) doSomething(); // expected-warning {{reading variable 'sp' requires holding mutex 'mu1'}}
sp[0] = 0; // expected-warning {{reading variable 'sp' requires holding mutex 'mu1'}}
@@ -4962,6 +4998,7 @@ class SmartPtr_PtGuardedBy_Test {
if (*sp == 0) doSomething(); // expected-warning {{reading the value pointed to by 'sp' requires holding mutex 'mu2'}}
*sp = 0; // expected-warning {{reading the value pointed to by 'sp' requires holding mutex 'mu2'}}
sq->a = 0; // expected-warning {{reading the value pointed to by 'sq' requires holding mutex 'mu2'}}
+ sq->*pa = 0; // expected-warning {{reading the value pointed to by 'sq' requires holding mutex 'mu2'}}
if (sp[0] == 0) doSomething(); // expected-warning {{reading the value pointed to by 'sp' requires holding mutex 'mu2'}}
sp[0] = 0; // expected-warning {{reading the value pointed to by 'sp' requires holding mutex 'mu2'}}
More information about the cfe-commits
mailing list