[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
Herald added a project: All.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124966

Files:
  clang/lib/Analysis/ThreadSafety.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp


Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===================================================================
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -82,6 +82,9 @@
   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,8 @@
 
   void operator()() { }
 
+  Data& operator+=(int);
+
 private:
   int dat;
 };
@@ -4404,6 +4409,7 @@
                           // 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_ += 0;           // 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 +4929,8 @@
   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 +4939,7 @@
     if (*sp == 0) doSomething();
     *sp = 0;
     sq->a = 0;
+    sq->*pa = 0;
 
     if (sp[0] == 0) doSomething();
     sp[0] = 0;
@@ -4946,6 +4955,7 @@
     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 +4972,7 @@
     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'}}
Index: clang/lib/Analysis/ThreadSafety.cpp
===================================================================
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -1990,7 +1990,17 @@
   } else if (const auto *OE = dyn_cast<CXXOperatorCallExpr>(Exp)) {
     auto OEop = OE->getOperator();
     switch (OEop) {
-      case OO_Equal: {
+      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: {
         const Expr *Target = OE->getArg(0);
         const Expr *Source = OE->getArg(1);
         checkAccess(Target, AK_Written);
@@ -1998,6 +2008,7 @@
         break;
       }
       case OO_Star:
+      case OO_ArrowStar:
       case OO_Arrow:
       case OO_Subscript:
         if (!(OEop == OO_Star && OE->getNumArgs() > 1)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124966.427147.patch
Type: text/x-patch
Size: 4144 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220504/c0015d57/attachment-0001.bin>