[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

Valeriy Savchenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 18 00:16:25 PST 2020


vsavchenko added a comment.

In D92039#2458392 <https://reviews.llvm.org/D92039#2458392>, @aaron.ballman wrote:

> 



> There have been efforts to add cross-TU support to the static analyzer, so I'm less worried about cross-TU inter-procedural bugs over the long term as I would expect that situation to be improved.

I'm a bit less optimistic on that front, the whole model of how we do the analysis should be redesigned in order to get true cross-TU inter-procedural analysis.  Additionally, some checkers when essentially inter-procedural rely on function conventions to assume behavior of called functions, the same way it is done with this analysis.

> I think my concern is somewhat different. If the goal is for the semantics to follow the parameter (which I also think is the correct behavior), then I think adding this as a frontend analysis is the incorrect place for the check to go because someday we're going to want the inter-procedural analysis and that will require us to figure out what to do with the frontend bits vs the static analyzer bits. If it starts off in the static analyzer, then the later improvements to the analysis capabilities don't require the user to change the way they interact with the toolchain.
>
> Or do you expect that this analysis is sufficient and you don't expect to ever extend it to be inter-procedural?

As I noted above, in some sense it is already inter-procedural because usually called functions follow completion handler conventions as well.
So, we can actually make it inter-procedural if all involved functions follow conventions.  And we can warn people if they leak `called_once` parameter into a function that doesn't specify what it will do with that parameter.

It was a decision not to do this and to be more lenient in the first version.  You can see that in a lot of cases during the analysis we assume that the user knows what they are doing.  And even in this model the number of reported warnings is pretty high.
We hope that this warning will help to change those cases and, maybe in the future, we can harden the analysis to be more demanding.



================
Comment at: clang/lib/Analysis/CalledOnceCheck.cpp:83
+    //   1. for any Kind K: NoReturn | K == K
+    //   2. for any Kind K: Reported | K == K
+    //   3. for any Kind K: K | K == K
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92039



More information about the cfe-commits mailing list