[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

Tue Aug 8 04:07:49 PDT 2023

donat.nagy added inline comments.

================
Comment at: clang/include/clang/Basic/AttrDocs.td:1189
+``onwership_returns``: Functions with this annotation return dynamic memory.
+The second annotation parameter is the size of the returned memory in bytes.
+
----------------
Szelethus wrote:
> aaron.ballman wrote:
> > I'm a bit confused on this last bit; how is the user supposed to statically know that value? I read this as claiming `my_malloc` returns 1 byte of dynamic storage; can the user tie the allocation to a parameter value instead?
> I share your confusion, I guess. What I've written here is my best understanding of how this works, unfortunately, these annotations were made in ~2010 (when I was still in elementary school) and abandoned since.
> 
> > I read this as claiming my_malloc returns 1 byte of dynamic storage;
> 
> Thats what the corresponding code in the Static Analyzer leads me to believe as well.
> 
> > can the user tie the allocation to a parameter value instead?
> 
> No. 
I'd guess that this annotation was designed for factory functions that construct an instance of a concrete struct type on the heap. Allowing "generic" malloc variants (where the allocation size is specified by a parameter) would be a good additional feature, but I think that the current annotation also has its own uses.

Perhaps it would be good to provide examples like
```
  void __attribute((ownership_takes(malloc, 1))) free_widget(struct widget *);
  struct widget *__attribute((ownership_returns(malloc, sizeof(struct widget)))) create_widget(void);
  void __attribute((ownership_holds(malloc, 1))) hold_widget(struct widget *);
```
that correspond to this use case. (By the way, does the checker handle non-`void*` pointers?)

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

https://reviews.llvm.org/D156787