Author: Marco Elver Date: 2026-04-01T20:55:28Z New Revision: b75bf1e722b8dfeabe41074819786d7540f7466e
URL: https://github.com/llvm/llvm-project/commit/b75bf1e722b8dfeabe41074819786d7540f7466e DIFF: https://github.com/llvm/llvm-project/commit/b75bf1e722b8dfeabe41074819786d7540f7466e.diff LOG: 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. Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/Analysis/ThreadSafety.cpp clang/test/SemaCXX/warn-thread-safety-analysis.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8fbee3c57074b..061ccd2492c49 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/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 ---------------------------------- diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index 5b2c876d65f24..099e8590b50e3 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -637,6 +637,7 @@ class VarMapBuilder : public ConstStmtVisitor<VarMapBuilder> { 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 diff erent definitions are discarded. diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 22160f6beebad..079e068ca7811 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/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); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
