[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions
Aaron Puchert via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 27 12:35:57 PDT 2023
aaronpuchert added inline comments.
================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:2436
+ CF.getVarDecl()->getLocation());
+ break;
+ }
----------------
aaronpuchert wrote:
> tbaeder wrote:
> > This handles the function call, but without the instance parameter. I was wondering how to best do that.
> Should you not simply pass `SxBuilder.createVariable(CF.getVarDecl())` as third parameter in analogy with the `AutomaticObjectDtor` case? It might also make sense to copy the attribute check.
Can you write a test case that relies on passing the variable? Here is an idea:
```
void unlock_scope(Mutex **mu) __attribute__((release_capability(*mu))) {
mutex_exclusive_unlock(*mu);
}
Mutex* const CLEANUP(unlock_scope) scope = &mu1;
mutex_exclusive_lock(*scope);
// Unlock should happen automatically.
```
I think this is mildly more interesting than the cleanup function with an unused parameter.
Unfortunately this is not quite as powerful as a scoped lock in C++, as we don't track the identity `scope == &mu1`. So `guarded_by` won't work with this. But we can at least see warnings on balanced locking/unlocking.
As for proper scoped locking, we could treat some variable initializations like construction of a C++ scoped lock. But let's discuss this separately.
================
Comment at: clang/test/Sema/warn-thread-safety-analysis.c:76-77
+void cleanup_int(int *unused) __attribute__((release_capability(mu1))) {
+ (void)unused;
+ mutex_exclusive_unlock(&mu1);
----------------
tbaeder wrote:
> aaronpuchert wrote:
> >
> > omitting the parameter name in a function definition is a C2x extension
Ah, I didn't know that C gained this only recently.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152504/new/
https://reviews.llvm.org/D152504
More information about the cfe-commits
mailing list