Revert "Thread Safety Analysis: Drop call-based alias invalidation (#187691)" (#190041)

This reverts commit 873d6bc3b415f1c2d942bbf4e4219c4bdcd4f2f8.

This causes Linux kernel build to fail because it relied on
alias-invalidation in kernel/core/sched.c.

clang/docs/ReleaseNotes.rst

@@ -353,11 +353,6 @@ Improvements to Clang's diagnostics
 - Improved ``-Wgnu-zero-variadic-macro-arguments`` to suggest using
   ``__VA_OPT__`` if the current language version supports it(#GH188624)
 
-- The :doc:`ThreadSafetyAnalysis` no longer performs call-based alias
-  invalidation (alias analysis only with ``-Wthread-safety-beta``), eliminating
-  false positives when passing local variable aliases by non-const pointer or
-  reference.
-
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/lib/Analysis/ThreadSafety.cpp

@@ -637,6 +637,7 @@ public:
 
   void VisitDeclStmt(const DeclStmt *S);
   void VisitBinaryOperator(const BinaryOperator *BO);
+  void VisitCallExpr(const CallExpr *CE);
 };
 
 } // namespace
@@ -682,6 +683,57 @@ void VarMapBuilder::VisitBinaryOperator(const BinaryOperator *BO) {
   }
 }
 
+// Invalidates local variable definitions if variable escaped.
+void VarMapBuilder::VisitCallExpr(const CallExpr *CE) {
+  const FunctionDecl *FD = CE->getDirectCallee();
+  if (!FD)
+    return;
+
+  // Heuristic for likely-benign functions that pass by mutable reference. This
+  // is needed to avoid a slew of false positives due to mutable reference
+  // passing where the captured reference is usually passed on by-value.
+  if (const IdentifierInfo *II = FD->getIdentifier()) {
+    // Any kind of std::bind-like functions.
+    if (II->isStr("bind") || II->isStr("bind_front"))
+      return;
+  }
+
+  // Invalidate local variable definitions that are passed by non-const
+  // reference or non-const pointer.
+  for (unsigned Idx = 0; Idx < CE->getNumArgs(); ++Idx) {
+    if (Idx >= FD->getNumParams())
+      break;
+
+    const Expr *Arg = CE->getArg(Idx)->IgnoreParenImpCasts();
+    const ParmVarDecl *PVD = FD->getParamDecl(Idx);
+    QualType ParamType = PVD->getType();
+
+    // Potential reassignment if passed by non-const reference / pointer.
+    const ValueDecl *VDec = nullptr;
+    if (ParamType->isReferenceType() &&
+        !ParamType->getPointeeType().isConstQualified()) {
+      if (const auto *DRE = dyn_cast<DeclRefExpr>(Arg))
+        VDec = DRE->getDecl();
+    } else if (ParamType->isPointerType() &&
+               !ParamType->getPointeeType().isConstQualified()) {
+      Arg = Arg->IgnoreParenCasts();
+      if (const auto *UO = dyn_cast<UnaryOperator>(Arg)) {
+        if (UO->getOpcode() == UO_AddrOf) {
+          const Expr *SubE = UO->getSubExpr()->IgnoreParenCasts();
+          if (const auto *DRE = dyn_cast<DeclRefExpr>(SubE))
+            VDec = DRE->getDecl();
+        }
+      }
+    }
+
+    if (VDec)
+      Ctx = VMap->clearDefinition(VDec, Ctx);
+  }
+  // Save the context after the call where escaped variables' definitions (if
+  // they exist) are cleared.
+  VMap->saveContext(CE, Ctx);
+}
+
 // Computes the intersection of two contexts.  The intersection is the
 // set of variables which have the same definition in both contexts;
 // variables with different definitions are discarded.

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

@@ -7494,17 +7494,13 @@ void testPointerAliasNoEscape3() {
   ptr->mu.Unlock();
 }
 
-// Passing an alias by non-const reference or pointer-to-pointer to a function
-// is no longer treated as an escape: dropping call-based alias invalidation
-// eliminates false positives (e.g. annotated acquire/release functions that
-// incidentally take the alias by mutable pointer) at the cost of not warning
-// when the called function actually changes what the alias points to.
 void testPointerAliasEscape1(Foo *f) {
   Foo *ptr = f;
   escapeAlias(0, ptr);
 
   ptr->mu.Lock();
-  f->data = 42;
+  f->data = 42;           // expected-warning{{writing variable 'data' requires holding mutex 'f->mu' exclusively}} \
+                          // expected-note{{found near match 'ptr->mu'}}
   ptr->mu.Unlock();
 }
 
@@ -7513,7 +7509,8 @@ void testPointerAliasEscape2(Foo *f) {
   escapeAlias(0, &ptr);
 
   ptr->mu.Lock();
-  f->data = 42;
+  f->data = 42;           // expected-warning{{writing variable 'data' requires holding mutex 'f->mu' exclusively}} \
+                          // expected-note{{found near match 'ptr->mu'}}
   ptr->mu.Unlock();
 }
 
@@ -7524,7 +7521,8 @@ void testPointerAliasEscape3(Foo *f) {
   escapeAlias(0, &ptr);
 
   ptr->mu.Lock();
-  f->data = 42;
+  f->data = 42;           // expected-warning{{writing variable 'data' requires holding mutex 'f->mu' exclusively}} \
+                          // expected-note{{found near match 'ptr->mu'}}
   ptr->mu.Unlock();
 }
 
@@ -7540,35 +7538,22 @@ void testPointerAliasEscapeAndReset(Foo *f) {
   ptr->mu.Unlock();
 }
 
-// Annotated acquire/release functions that incidentally take the alias by
-// non-const pointer must not produce false positives.
-void lockFooPtr(Foo **Fp) EXCLUSIVE_LOCK_FUNCTION((*Fp)->mu);
-void unlockFooPtr(Foo **Fp) EXCLUSIVE_UNLOCK_FUNCTION((*Fp)->mu);
-
-void testAnnotatedAcquireReleaseNoFalsePositive(Foo *F) {
-  Foo *ptr = F;
-  lockFooPtr(&ptr);
-  F->data = 42;
-  ptr->data = 42;
-  unlockFooPtr(&ptr);
-}
-
 // A function that may do anything to the objects referred to by the inputs.
 void escapeAliasMultiple(void *, void *, void *);
 void testPointerAliasEscapeMultiple(Foo *F) {
   Foo *L;
-  F->mu.Lock();
+  F->mu.Lock(); // expected-note{{mutex acquired here}}
   Foo *Fp = F;
-  escapeAliasMultiple(&L, &L, &Fp);  // Fp alias retained
-  Fp->mu.Unlock();                   // ok: Fp still aliases F
-}
+  escapeAliasMultiple(&L, &L, &Fp);
+  Fp->mu.Unlock(); // expected-warning{{releasing mutex 'Fp->mu' that was not held}}
+} // expected-warning{{mutex 'F->mu' is still held at the end of function}}
 
 void unlockFooWithEscapablePointer(Foo **Fp) EXCLUSIVE_UNLOCK_FUNCTION((*Fp)->mu);
 void testEscapeInvalidationHappensRightAfterTheCall(Foo* F) {
   Foo* L;
   L = F;
   L->mu.Lock();
-  // L's alias is retained across the call.
+  // Release the lock held by 'L' before clearing its definition.
   unlockFooWithEscapablePointer(&L);
 }