https://github.com/ziqingluo-90 created https://github.com/llvm/llvm-project/pull/178825
The commit b4c98fcbe1504841203e610c351a3227f36c92a4 introduces alias-analysis and 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. >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] [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}} + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
