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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 18 07:46:39 PST 2020


aaron.ballman added a comment.

In D92039#2462995 <https://reviews.llvm.org/D92039#2462995>, @vsavchenko wrote:

> In D92039#2462889 <https://reviews.llvm.org/D92039#2462889>, @aaron.ballman wrote:
>
>> Your test cases suggest that this is not inter-procedurally checked; it seems to require all of the APIs to be annotated in order to get the same effect that an inter-procedural check could determine on its own.
>
> I mean, of course, it doesn't have real inter-procedural analysis.  But if everything is annotated or conventional - it is identical to inter-procedural case (like in tests with `indirect` in their names).

Okay, I'm glad to know I wasn't misunderstanding something there!

>> I'm not worried about the leniency and I'm sorry if it sounds like I'm implying that it has to be inter-procedural to land -- that's not the case. What I am worried about is that the way the user runs the frontend is very different than the way the user runs the static analyzer, and migrating the warning from the FE to the static analyzer could leave us with problems. If the intention is that this check will live in the FE as-is (not getting inter-procedural support), then that's fine. But if the intention is to extend the check in the future in a way that might need it to move out of the FE and into the CSA, then I think the check should live in the CSA from the start (even if it doesn't do inter-procedural analysis). It's not clear to me whether "in the first version" implies that you expect a subsequent version to live in the CSA or not.
>
> No, we don't plan on moving it to the CSA.  As I mentioned above, for hardening we can simply warn if `called_once` parameters leak into functions we don't know about (not annotated or not following conventions).

Okay, thank you for verifying. The attribute bits of the patch LG to me (aside from a minor testing request). Someone else should validate the actual analysis bits.



================
Comment at: clang/test/SemaObjC/attr-called-once.m:7
+void test2(double x CALLED_ONCE); // expected-error{{'called_once' attribute only applies to function-like parameters}}
+
+void test3(void (*foo)() CALLED_ONCE);   // no-error
----------------
Can you also add tests that the attribute accepts no arguments and only appertains to parameters?


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