[clang] Thread Safety Analysis: Drop most call-based alias invalidation (PR #190154)
Marco Elver via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 2 04:59:45 PDT 2026
https://github.com/melver created https://github.com/llvm/llvm-project/pull/190154
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.
Replace the broad heuristic (including a special-case carve-outs for std::bind/bind_front) with a narrower rule based on type mismatch: an alias is now only invalidated if its address is passed to a parameter of a different underlying type (e.g., Foo** to void**). Aliases passed by reference are never invalidated.
This type-erasure boundary signals that the pointer is being handled opaquely (such as in generic reallocation or explicit marker functions), providing a semantically stronger signal for invalidation that avoids false positives for common type-preserving calls: the latter are aware of the semantics of the underlying object and typically handle lock transfers correctly (if the aliased object is changed).
v1: https://github.com/llvm/llvm-project/pull/187691
- Compared to v1, this also retains the testPointerAliasEscapeMultiple() test case unmodified, as that was explicitly added with a fix to avoid false negative detection!
Discussion: https://github.com/llvm/llvm-project/pull/183640
>From a799819065e2253f0fa16fa2c2efd740c39ff48e Mon Sep 17 00:00:00 2001
From: Marco Elver <elver at google.com>
Date: Wed, 1 Apr 2026 21:18:42 +0200
Subject: [PATCH] Thread Safety Analysis: Drop most 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.
Replace the broad heuristic (including a special-case carve-outs for
std::bind/bind_front) with a narrower rule based on type mismatch: an
alias is now only invalidated if its address is passed to a parameter of
a different underlying type (e.g., Foo** to void**). Aliases passed by
reference are never invalidated.
This type-erasure boundary signals that the pointer is being handled
opaquely (such as in generic reallocation or explicit marker functions),
providing a semantically stronger signal for invalidation that avoids
false positives for common type-preserving calls: the latter are aware
of the semantics of the underlying object and typically handle lock
transfers correctly (if the aliased object is changed).
v1: https://github.com/llvm/llvm-project/pull/187691
- Compared to v1, this also retains the testPointerAliasEscapeMultiple()
test case unmodified, as that was explicitly added with a fix to avoid
false negative detection!
Discussion: https://github.com/llvm/llvm-project/pull/183640
---
clang/docs/ReleaseNotes.rst | 5 +++
clang/lib/Analysis/ThreadSafety.cpp | 35 +++++++----------
.../SemaCXX/warn-thread-safety-analysis.cpp | 39 +++++++++++++++----
3 files changed, 52 insertions(+), 27 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 061ccd2492c49..d5e918594a4b0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -353,6 +353,11 @@ 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` dropped most 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
----------------------------------
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 099e8590b50e3..08ee4add36458 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -683,23 +683,15 @@ void VarMapBuilder::VisitBinaryOperator(const BinaryOperator *BO) {
}
}
-// Invalidates local variable definitions if variable escaped.
+// Invalidates local variable aliases when passed through a type-mismatching
+// pointer cast (e.g., Foo** to void**). This signals that the callee is
+// treating the pointer opaquely and likely changes its value, such as in a
+// generic reallocation function (e.g. custom_realloc(&ptr, ...)).
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;
@@ -708,20 +700,23 @@ void VarMapBuilder::VisitCallExpr(const CallExpr *CE) {
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() &&
+ if (ParamType->isPointerType() &&
!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 (const auto *DRE = dyn_cast<DeclRefExpr>(SubE)) {
+ const ValueDecl *VD = DRE->getDecl();
+ QualType OriginalTy =
+ VD->getType().getCanonicalType().getUnqualifiedType();
+ QualType PointeeTy = ParamType->getPointeeType()
+ .getCanonicalType()
+ .getUnqualifiedType();
+ if (OriginalTy != PointeeTy)
+ VDec = VD;
+ }
}
}
}
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 079e068ca7811..35d860a50e94c 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -7494,13 +7494,17 @@ 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; // expected-warning{{writing variable 'data' requires holding mutex 'f->mu' exclusively}} \
- // expected-note{{found near match 'ptr->mu'}}
+ f->data = 42;
ptr->mu.Unlock();
}
@@ -7509,8 +7513,7 @@ void testPointerAliasEscape2(Foo *f) {
escapeAlias(0, &ptr);
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;
ptr->mu.Unlock();
}
@@ -7521,8 +7524,7 @@ void testPointerAliasEscape3(Foo *f) {
escapeAlias(0, &ptr);
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;
ptr->mu.Unlock();
}
@@ -7538,6 +7540,29 @@ 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);
+}
+
+void customRealloc(void **p, long);
+void testPointerAliasTypeErasedEscape(Foo *f) {
+ Foo *ptr = f;
+ customRealloc((void**)&ptr, sizeof(*ptr));
+ ptr->mu.Lock();
+ f->data = 42; // expected-warning{{writing variable 'data' requires holding mutex 'f->mu' exclusively}} \
+ // expected-note{{found near match 'ptr->mu'}}
+ ptr->mu.Unlock();
+}
+
// A function that may do anything to the objects referred to by the inputs.
void escapeAliasMultiple(void *, void *, void *);
void testPointerAliasEscapeMultiple(Foo *F) {
@@ -7553,7 +7578,7 @@ 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.
unlockFooWithEscapablePointer(&L);
}
More information about the cfe-commits
mailing list