[clang] 37aaff3 - [Thread Analysis] Fix a bug in context update in alias-analysis (#178952)

[via cfe-commits]

Author: Ziqing Luo
Date: 2026-02-13T11:45:44-08:00
New Revision: 37aaff3fef7f0344299ddbf7b19a53c25e7ca951

URL: https://github.com/llvm/llvm-project/commit/37aaff3fef7f0344299ddbf7b19a53c25e7ca951
DIFF: https://github.com/llvm/llvm-project/commit/37aaff3fef7f0344299ddbf7b19a53c25e7ca951.diff

LOG: [Thread Analysis] Fix a bug in context update in alias-analysis (#178952)

[Thread Analysis] Fix a bug in context update in alias-analysis
    
Previously, in 'updateLocalVarMapCtx', context was updated to the one
immediately after the provided statement 'S'.  It is incorrect,
because 'S' hasn't been processed at that point.  This issue could
result in false positives.  For example,
    
```
    void f(Lock_t* F)
    {
        Lock_t* L = F; // 'L' aliases with 'F'
        L->Lock();     // 'L' holds the lock
        // Before the fix, the analyzer saw the definition of 'L' being cleared before 'L' was unlocked.
        unlock(&L);    // unlock (*L)
    }
```
This commit fixes the issue.
    
rdar://169236809

Added: 
    

Modified: 
    clang/lib/Analysis/ThreadSafety.cpp
    clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 4ed3ef1993eda..901624acd6f75 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1727,10 +1727,14 @@ class BuildLockset : public ConstStmtVisitor<BuildLockset> {
   FactSet FSet;
   // The fact set for the function on exit.
   const FactSet &FunctionExitFSet;
+  // Use `LVarCtx` to keep track of the context at each program point, which
+  // will be used to translate variables (by `SExprBuilder::translateVariable`)
+  // to their canonical definitions:
   LocalVariableMap::Context LVarCtx;
   unsigned CtxIndex;
 
-  // To update and adjust the context.
+  // To update the context used in attr-expr translation.  If `S` is non-null,
+  // the context is updated to the program point right after 'S'.
   void updateLocalVarMapCtx(const Stmt *S) {
     if (S)
       LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx);
@@ -2273,9 +2277,8 @@ void BuildLockset::VisitUnaryOperator(const UnaryOperator *UO) {
 void BuildLockset::VisitBinaryOperator(const BinaryOperator *BO) {
   if (!BO->isAssignmentOp())
     return;
-
-  updateLocalVarMapCtx(BO);
   checkAccess(BO->getLHS(), AK_Written);
+  updateLocalVarMapCtx(BO);
 }
 
 /// Whenever we do an LValue to Rvalue cast, we are reading a variable and
@@ -2320,8 +2323,6 @@ void BuildLockset::examineArguments(const FunctionDecl *FD,
 }
 
 void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
-  updateLocalVarMapCtx(Exp);
-
   if (const auto *CE = dyn_cast<CXXMemberCallExpr>(Exp)) {
     const auto *ME = dyn_cast<MemberExpr>(CE->getCallee());
     // ME can be null when calling a method pointer
@@ -2385,9 +2386,9 @@ void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
   }
 
   auto *D = dyn_cast_or_null<NamedDecl>(Exp->getCalleeDecl());
-  if (!D)
-    return;
-  handleCall(Exp, D);
+  if (D)
+    handleCall(Exp, D);
+  updateLocalVarMapCtx(Exp);
 }
 
 void BuildLockset::VisitCXXConstructExpr(const CXXConstructExpr *Exp) {
@@ -2416,8 +2417,6 @@ static const Expr *UnpackConstruction(const Expr *E) {
 }
 
 void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
-  updateLocalVarMapCtx(S);
-
   for (auto *D : S->getDeclGroup()) {
     if (auto *VD = dyn_cast_or_null<VarDecl>(D)) {
       const Expr *E = VD->getInit();
@@ -2437,6 +2436,7 @@ void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
       }
     }
   }
+  updateLocalVarMapCtx(S);
 }
 
 void BuildLockset::VisitMaterializeTemporaryExpr(

diff  --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 30d18517c9a19..b878a37747f2f 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -7493,7 +7493,32 @@ void testPointerAliasEscapeMultiple(Foo *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();
+    // Release the lock held by 'L' before clearing its definition.
+    unlockFooWithEscapablePointer(&L);
+}
+
+void testEscapeInvalidationHappensRightAfterTheCtorCall(Foo* F) {
+  Foo* L = F;
+  MutexLock ScopeLock(&L->mu);
+
+  struct {
+    int DataMember GUARDED_BY(F->mu);
+  } Data;
+
+  Data.DataMember = 0;
+}
+
+void testCleanUpFunctionWithLocalVarUpdated(Foo* F) {
+  F->mu.Lock();
+  Foo * __attribute__((unused, cleanup(unlockFooWithEscapablePointer))) L = F;
+}
+
 void testPointerAliasTryLock1() {
   Foo *ptr = returnsFoo();
   if (ptr->mu.TryLock()) {