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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I think this is the right solution and I'm happy to see it!

We have a few "direct" CFG tests in `test/Analysis/cfg.c` etc.; your patch doesn't seem to be specific to thread safety analysis so it's probably a good idea to add a few direct tests.



================
Comment at: clang/lib/Analysis/CFG.cpp:1945
+      auto DRE =
+          DeclRefExpr::Create(*Context, {}, {}, VD, false, SourceLocation(),
+                              VD->getType(), VK_PRValue);
----------------
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.


================
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,
----------------
Shouldn't there be a unary operator `&` here somewhere? In your example the variable is `int` but the function accepts `int *`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152504