[clang] [Thread Safety Analysis] Fix a bug of context saving in alias-analysis (PR #178825)
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 30 11:14:23 PST 2026
https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/178825
>From 02dd8f8318f7cccd8f785b5abe5c9c2c866f1333 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Thu, 29 Jan 2026 18:59:23 -0800
Subject: [PATCH 1/2] [Thread Safety Analysis] Fix a bug of context saving in
alias-analysis
The commit b4c98fcbe1504841203e610c351a3227f36c92a4 introduces
alias-analysis. That commit also conservatively invalidates variable
definitions at function calls. For each invalidated argument, it
creates and pushes a context. So if there are multiple arguments being
invalidated, there are more than one context being pushed. However,
the analysis expects one context at the program point of a call,
causing context mismatch. This issue could lead to false negatives.
This commit fixes the issue.
---
clang/lib/Analysis/ThreadSafety.cpp | 5 ++-
clang/test/Sema/warn-thread-safety-bug-fix.c | 36 ++++++++++++++++++++
2 files changed, 38 insertions(+), 3 deletions(-)
create mode 100644 clang/test/Sema/warn-thread-safety-bug-fix.c
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index df4bae89f62df..d89429e104e3e 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -726,11 +726,10 @@ void VarMapBuilder::VisitCallExpr(const CallExpr *CE) {
}
}
- if (VDec && Ctx.lookup(VDec)) {
+ if (VDec)
Ctx = VMap->clearDefinition(VDec, Ctx);
- VMap->saveContext(CE, Ctx);
- }
}
+ VMap->saveContext(CE, Ctx);
}
// Computes the intersection of two contexts. The intersection is the
diff --git a/clang/test/Sema/warn-thread-safety-bug-fix.c b/clang/test/Sema/warn-thread-safety-bug-fix.c
new file mode 100644
index 0000000000000..cedecaa7b8496
--- /dev/null
+++ b/clang/test/Sema/warn-thread-safety-bug-fix.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta %s
+
+typedef int __attribute__((capability("lock"))) lock_t;
+
+typedef struct {
+ lock_t lock;
+} * map_t;
+
+typedef struct task {
+ map_t map;
+} *task_t;
+
+#define ACQUIRES_LOCK(map) \
+ __attribute__((acquire_capability((map)->lock)))
+#define RELEASES_LOCK(map) \
+ __attribute__((release_capability((map)->lock)))
+
+extern void lock_map(map_t map) ACQUIRES_LOCK(map);
+extern void unlock_map_indirect(map_t *mapp) RELEASES_LOCK(*mapp);
+extern void f(void *, void *, void *);
+
+static void saveContexBug(task_t task)
+{
+ map_t map;
+ map = task->map;
+ lock_map(map); // expected-note{{lock acquired here}}
+ map_t *mapp = ↦
+ // Previously, a local-variable-definition-context was created and
+ // pushed for each of the argument below, resulting context
+ // mismatch. The analyzer missed the fact that 'mapp' may no
+ // longer point to the lock. So it does not report an issue at the
+ // 'unlock_map_indirect' call.
+ f(&map, &map, &mapp);
+ unlock_map_indirect(mapp); // expected-warning{{releasing lock 'mapp->lock' that was not held}}
+} // expected-warning{{lock 'task->map->lock' is still held at the end of function}}
+
>From b9742a6924c3a0c39a652a489e77ae15c25a98ad Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Fri, 30 Jan 2026 11:13:45 -0800
Subject: [PATCH 2/2] Address comments
---
clang/test/Sema/warn-thread-safety-bug-fix.c | 36 -------------------
.../SemaCXX/warn-thread-safety-analysis.cpp | 21 +++++++++++
2 files changed, 21 insertions(+), 36 deletions(-)
delete mode 100644 clang/test/Sema/warn-thread-safety-bug-fix.c
diff --git a/clang/test/Sema/warn-thread-safety-bug-fix.c b/clang/test/Sema/warn-thread-safety-bug-fix.c
deleted file mode 100644
index cedecaa7b8496..0000000000000
--- a/clang/test/Sema/warn-thread-safety-bug-fix.c
+++ /dev/null
@@ -1,36 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta %s
-
-typedef int __attribute__((capability("lock"))) lock_t;
-
-typedef struct {
- lock_t lock;
-} * map_t;
-
-typedef struct task {
- map_t map;
-} *task_t;
-
-#define ACQUIRES_LOCK(map) \
- __attribute__((acquire_capability((map)->lock)))
-#define RELEASES_LOCK(map) \
- __attribute__((release_capability((map)->lock)))
-
-extern void lock_map(map_t map) ACQUIRES_LOCK(map);
-extern void unlock_map_indirect(map_t *mapp) RELEASES_LOCK(*mapp);
-extern void f(void *, void *, void *);
-
-static void saveContexBug(task_t task)
-{
- map_t map;
- map = task->map;
- lock_map(map); // expected-note{{lock acquired here}}
- map_t *mapp = ↦
- // Previously, a local-variable-definition-context was created and
- // pushed for each of the argument below, resulting context
- // mismatch. The analyzer missed the fact that 'mapp' may no
- // longer point to the lock. So it does not report an issue at the
- // 'unlock_map_indirect' call.
- f(&map, &map, &mapp);
- unlock_map_indirect(mapp); // expected-warning{{releasing lock 'mapp->lock' that was not held}}
-} // expected-warning{{lock 'task->map->lock' is still held at the end of function}}
-
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index d9efa745b7d59..92afb6e14c8de 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -7675,4 +7675,25 @@ void testLoopConditionalReassignment(Foo *f1, Foo *f2, bool cond) {
f1->data = 42;
ptr->mu.Unlock(); // expected-warning{{releasing mutex 'ptr->mu' that was not held}}
} // expected-warning{{mutex 'f1->mu' is still held at the end of function}}
+
+
+void unlock_Foo(Foo **Fp) __attribute__((release_capability((*Fp)->mu)));
+// A function that may do anything to the objects referred to by the inputs:
+void f(void *, void *, void *);
+
+static void saveContexBug(Foo *F)
+{
+ Foo *L;
+ L = F;
+ L->mu.Lock(); // expected-note{{mutex acquired here}}
+ Foo ** Fp = &L;
+ // Previously, a local-variable-definition-context was created and
+ // pushed for each of the argument below, resulting context
+ // mismatch. The analyzer missed the fact that 'mapp' may no
+ // longer point to the lock. So it does not report an issue at the
+ // 'unlock_map_indirect' call.
+ f(&L, &L, &Fp);
+ unlock_Foo(Fp); // expected-warning{{releasing mutex 'Fp->mu' that was not held}}
+} // expected-warning{{mutex 'F->mu' is still held at the end of function}}
+
} // namespace CapabilityAliases
More information about the cfe-commits
mailing list