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

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

tbaeder added inline comments.


================
Comment at: clang/lib/Analysis/CFG.cpp:1945
+      auto DRE =
+          DeclRefExpr::Create(*Context, {}, {}, VD, false, SourceLocation(),
+                              VD->getType(), VK_PRValue);
----------------
NoQ wrote:
> Is there a way to attach some valid source locations here, like the source location of the attribute's identifier argument?
> 
> I'm worried that the static analyzer may be unable to consume the CFG without them, it typically requires source locations for almost arbitrary statements. I'm still not sure how it'll react to synthetic statements, maybe turning them into a custom `CFGElement` subclass (like destructors) would be safer as it'll let all clients of the CFG to treat these in a custom manner. Otherwise it makes it look like a completely normal call-expression to the clients, which is not necessarily a good thing.
Look like I can't query the attribute for any locations except its entire range; I can also just pass `VD->getLocation()` to all locations here.

I thought doing this without creating fake AST nodes is better as well but I wasn't sure how to do that properly, or if I have to update all consumers of the CFG.


================
Comment at: clang/lib/Analysis/CFG.cpp:1951-1952
+
+      SmallVector<Expr *> Args;
+      Args.push_back(DRE);
+      auto A = CallExpr::Create(*Context, F, Args, FD->getType(), VK_PRValue,
----------------
NoQ wrote:
> Shouldn't there be a unary operator `&` here somewhere? In your example the variable is `int` but the function accepts `int *`.
Hm, I think you are right, but it doesn't make difference for the CFG (at least not for thread anysis), does it? Is there a way for me to test that I pass the right parameter value?


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

https://reviews.llvm.org/D152504