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 <[email protected]> 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 <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
