[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:2436
+                                    CF.getVarDecl()->getLocation());
+          break;
+        }
----------------
aaronpuchert wrote:
> 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.
Yeah, it doesn't find the lock; I assume that's because the first parameter here is not an _actual_ this parameter; I'd have to handle the var decl as a regular parameter.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152504/new/

https://reviews.llvm.org/D152504