[clang] Thread Safety Analysis: Drop call-based alias invalidation (PR #187691)
Marco Elver via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 20 05:57:10 PDT 2026
https://github.com/melver updated https://github.com/llvm/llvm-project/pull/187691
>From e52576a1bc7bacf48e2c9f55776effd5788f9c04 Mon Sep 17 00:00:00 2001
From: Marco Elver <me at marcoelver.com>
Date: Mon, 16 Mar 2026 15:47:15 +0100
Subject: [PATCH] Thread Safety Analysis: Drop call-based alias invalidation
Local variables passed by non-const pointer or reference to a function
were previously invalidated in the LocalVariableMap (VarMapBuilder), on
the assumption that the callee might change what they point to. This
caused false positives when the function also carries ACQUIRE/RELEASE
annotations: handleCall translates those annotations with the pre-call
context, while subsequent guard checks use the post-invalidation
context, producing an expansion mismatch and a spurious warning.
The invalidation rules were a heuristic with significant complexity
(including a special-case carve-out for std::bind/bind_front) and
unclear benefit. Instead of adding more heuristics, drop the
alias-invalidation rules entirely.
---
clang/lib/Analysis/ThreadSafety.cpp | 52 -------------------
.../SemaCXX/warn-thread-safety-analysis.cpp | 50 +++++++++++++-----
2 files changed, 36 insertions(+), 66 deletions(-)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 2c5976af9f61d..8687dbf46d90c 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -637,7 +637,6 @@ class VarMapBuilder : public ConstStmtVisitor<VarMapBuilder> {
void VisitDeclStmt(const DeclStmt *S);
void VisitBinaryOperator(const BinaryOperator *BO);
- void VisitCallExpr(const CallExpr *CE);
};
} // namespace
@@ -683,57 +682,6 @@ 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.
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index a6c9a2236fc45..7a277c47b6109 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -7440,23 +7440,29 @@ 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: the alias is retained and the lock
+// acquired through it continues to cover accesses via the original pointer.
+// This is a deliberate trade-off: 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. Direct
+// reassignment (ptr = ...) is still tracked and will produce a warning.
void testPointerAliasEscape1(Foo *f) {
Foo *ptr = f;
- escapeAlias(0, ptr);
+ escapeAlias(0, ptr); // pass by non-const ref — alias retained
ptr->mu.Lock();
- f->data = 42; // expected-warning{{writing variable 'data' requires holding mutex 'f->mu' exclusively}} \
- // expected-note{{found near match 'ptr->mu'}}
+ f->data = 42; // ok: ptr->mu covers f->mu (alias still live)
ptr->mu.Unlock();
}
void testPointerAliasEscape2(Foo *f) {
Foo *ptr = f;
- escapeAlias(0, &ptr);
+ escapeAlias(0, &ptr); // pass by non-const pointer — alias retained
ptr->mu.Lock();
- f->data = 42; // expected-warning{{writing variable 'data' requires holding mutex 'f->mu' exclusively}} \
- // expected-note{{found near match 'ptr->mu'}}
+ f->data = 42; // ok: alias retained
ptr->mu.Unlock();
}
@@ -7464,11 +7470,10 @@ void testPointerAliasEscape3(Foo *f) {
Foo *ptr;
ptr = f;
- escapeAlias(0, &ptr);
+ escapeAlias(0, &ptr); // alias retained
ptr->mu.Lock();
- f->data = 42; // expected-warning{{writing variable 'data' requires holding mutex 'f->mu' exclusively}} \
- // expected-note{{found near match 'ptr->mu'}}
+ f->data = 42; // ok: alias retained
ptr->mu.Unlock();
}
@@ -7484,22 +7489,39 @@ 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. Previously, passing
+// `&ptr` to such a function would clear ptr's definition, causing the lock
+// fact (recorded with the pre-call expansion) to mismatch the guard check
+// (recorded with the post-call expansion).
+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); // acquires ptr->mu (= F->mu)
+ F->data = 42; // ok: no spurious warning
+ ptr->data = 42; // ok
+ 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(); // expected-note{{mutex acquired here}}
+ F->mu.Lock();
Foo *Fp = 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}}
+ escapeAliasMultiple(&L, &L, &Fp); // Fp alias retained
+ Fp->mu.Unlock(); // ok: Fp still aliases F
+}
void unlockFooWithEscapablePointer(Foo **Fp) EXCLUSIVE_UNLOCK_FUNCTION((*Fp)->mu);
void testEscapeInvalidationHappensRightAfterTheCall(Foo* F) {
Foo* L;
L = F;
L->mu.Lock();
- // Release the lock held by 'L' before clearing its definition.
+ // L's alias is retained across the call; the UNLOCK annotation resolves
+ // (*&L)->mu = L->mu = F->mu, correctly releasing the held lock.
unlockFooWithEscapablePointer(&L);
}
More information about the cfe-commits
mailing list