llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Ziqing Luo (ziqingluo-90) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/178825.diff 2 Files Affected: - (modified) clang/lib/Analysis/ThreadSafety.cpp (+2-3) - (added) clang/test/Sema/warn-thread-safety-bug-fix.c (+36) ``````````diff 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}} + `````````` </details> https://github.com/llvm/llvm-project/pull/178825 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
