[PATCH] D113622: [analyzer] support ignoring use-after-free checking with reference_counted attribute

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D113622#3194580 <https://reviews.llvm.org/D113622#3194580>, @steakhal wrote:

> Could you please share the results to have look? How can I reproduce and evaluate the effect of this change?

You'll need a project that uses a lot of manual reference counting, typically in C code (in our case it's XNU which is technically open-source but probably not very interesting). I've definitely seen such code in the wild, typically around various interpreters (eg., C APIs for python or javascript interpreters where the interpreted language's values are reference counted and garbage collected by the language so they need to be manually retained during interop) but we don't have much real data on those.

In such projects you'll see code like this:

  struct S {
    int ref_count;
  };
  
  ...
  
  void release(S *s) {
    if (--s->refcount == 0) {
      free(s);
    }
  }

If the static analyzer doesn't know the original reference count, it'll have to assume that the retain count will drop to zero every time such check is made. In particular, it'll assume that even if it did see a prior increment of the reference count (what if the original reference count was equal to -1?). Any subsequent use of the pointer will cause a use-after-free warning. In practice, however, the whole point of having reference counts is to be protected against such situations; reference count is always positive and typically large enough to avoid such use-after-free problems, so these are usually false positives when emitted by MallocChecker.

We have a separate checker, RetainCountChecker, that's specifically designed to deal with such code, to understand the underlying reference counting conventions. Currently RetainCountChecker supports 3 different hardcoded reference counting conventions. However even in just XNU we've seen a lot more separate conventions that are so many that they become impractical to hardcode.

The new attribute currently suppresses MallocChecker warnings but in the future we plan to extend it to actively enable RetainCountChecker to learn the appropriate convention and emit warnings that are more appropriate. The patch doesn't have any effect if the attribute isn't already present so you'll have to actively add it in order to observe any effect.



================
Comment at: clang/include/clang/Basic/Attr.td:1700
+  let Subjects = SubjectList<[Record]>;
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;
----------------
Hmm, I think it's a good idea to provide some documentation.


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

https://reviews.llvm.org/D113622