Thread safety analysis: Support builtin pointer-to-member operators

[aaronpuchert]

Summary

We consider an access to x.*pm as access of the same kind into x, and
an access to px->*pm as access of the same kind into *px. Previously we
missed reads and writes in the .* case, and operations to the pointed-to
data for ->* (we didn't miss accesses to the pointer itself, because
that requires an LValueToRValue cast that we treat independently).

We added support for overloaded operator->* in D124966.

[aaron.ballman]

Good catch, LGTM!

clang/lib/Analysis/ThreadSafety.cpp

void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK,
 
   if (const auto *UO = dyn_cast<UnaryOperator>(Exp)) {
     // For dereferences
     if (UO->getOpcode() == UO_Deref)
       checkPtAccess(UO->getSubExpr(), AK, POK);
     return;
   }
 
+  if (const auto *BO = dyn_cast<BinaryOperator>(Exp)) {
+    switch (BO->getOpcode()) {
+    case BO_PtrMemD: // .*
+      return checkAccess(BO->getLHS(), AK, POK);
+    case BO_PtrMemI: // ->*
+      return checkPtAccess(BO->getLHS(), AK, POK);
+    default:
+      return;
+    }
+  }
+
   if (const auto *AE = dyn_cast<ArraySubscriptExpr>(Exp)) {
     checkPtAccess(AE->getLHS(), AK, POK);
     return;
   }
 
   if (const auto *ME = dyn_cast<MemberExpr>(Exp)) {
     if (ME->isArrow())
       checkPtAccess(ME->getBase(), AK, POK);

clang/test/SemaCXX/warn-thread-safety-analysis.cpp

 class PtGuardedByCorrectnessTest {
   Mutex  mu1;
   Mutex  mu2;
   int*   a GUARDED_BY(mu1) PT_GUARDED_BY(mu2);
   Cell*  c GUARDED_BY(mu1) PT_GUARDED_BY(mu2);
   int    sa[10] GUARDED_BY(mu1);
   Cell   sc[10] GUARDED_BY(mu1);
 
+  static constexpr int Cell::*pa = &Cell::a;
+
   void test1() {
     mu1.Lock();
     if (a == 0) doSomething();  // OK, we don't dereference.
     a = 0;
     c = 0;
     if (sa[0] == 42) doSomething();
     sa[0] = 57;
     if (sc[0].a == 42) doSomething();
     sc[0].a = 57;
     mu1.Unlock();
   }
 
   void test2() {
     mu1.ReaderLock();
     if (*a == 0) doSomething();      // expected-warning {{reading the value pointed to by 'a' requires holding mutex 'mu2'}}
     *a = 0;                          // expected-warning {{writing the value pointed to by 'a' requires holding mutex 'mu2' exclusively}}
 
     if (c->a == 0) doSomething();    // expected-warning {{reading the value pointed to by 'c' requires holding mutex 'mu2'}}
     c->a = 0;                        // expected-warning {{writing the value pointed to by 'c' requires holding mutex 'mu2' exclusively}}
+    c->*pa = 0;                      // expected-warning {{writing the value pointed to by 'c' requires holding mutex 'mu2' exclusively}}
 
     if ((*c).a == 0) doSomething();  // expected-warning {{reading the value pointed to by 'c' requires holding mutex 'mu2'}}
     (*c).a = 0;                      // expected-warning {{writing the value pointed to by 'c' requires holding mutex 'mu2' exclusively}}
+    (*c).*pa = 0;                    // expected-warning {{writing the value pointed to by 'c' requires holding mutex 'mu2' exclusively}}
 
     if (a[0] == 42) doSomething();     // expected-warning {{reading the value pointed to by 'a' requires holding mutex 'mu2'}}
     a[0] = 57;                         // expected-warning {{writing the value pointed to by 'a' requires holding mutex 'mu2' exclusively}}
     if (c[0].a == 42) doSomething();   // expected-warning {{reading the value pointed to by 'c' requires holding mutex 'mu2'}}
     c[0].a = 57;                       // expected-warning {{writing the value pointed to by 'c' requires holding mutex 'mu2' exclusively}}
     mu1.Unlock();
   }
 
void test3() {
     mu2.Unlock();
   }
 
   void test4() {  // Literal arrays
     if (sa[0] == 42) doSomething();     // expected-warning {{reading variable 'sa' requires holding mutex 'mu1'}}
     sa[0] = 57;                         // expected-warning {{writing variable 'sa' requires holding mutex 'mu1' exclusively}}
     if (sc[0].a == 42) doSomething();   // expected-warning {{reading variable 'sc' requires holding mutex 'mu1'}}
     sc[0].a = 57;                       // expected-warning {{writing variable 'sc' requires holding mutex 'mu1' exclusively}}
+    sc[0].*pa = 57;                     // expected-warning {{writing variable 'sc' requires holding mutex 'mu1' exclusively}}
 
     if (*sa == 42) doSomething();       // expected-warning {{reading variable 'sa' requires holding mutex 'mu1'}}
     *sa = 57;                           // expected-warning {{writing variable 'sa' requires holding mutex 'mu1' exclusively}}
     if ((*sc).a == 42) doSomething();   // expected-warning {{reading variable 'sc' requires holding mutex 'mu1'}}
     (*sc).a = 57;                       // expected-warning {{writing variable 'sc' requires holding mutex 'mu1' exclusively}}
     if (sc->a == 42) doSomething();     // expected-warning {{reading variable 'sc' requires holding mutex 'mu1'}}
     sc->a = 57;                         // expected-warning {{writing variable 'sc' requires holding mutex 'mu1' exclusively}}
   }