False-positive with deref address-of
Closed this issue · 11 comments
See
https://godbolt.org/z/1tWI6O
This lead to reverting https://reviews.llvm.org/D66179.
@Xazax-hun, could you take a look, please?
Sure! I'm working on something else, but will try to look into this in a few hours!
It should be fixed with 4226f4c but I want to run this over some project before creating a PR.
The solution is not perfect, it did introduce some false positives. I think I will take a more conservative approach for now, and catch new issues again once DerefType is available.
When there is no deref type available, we can still have explicit or implicit function annotations. E.g.
the contract on _vector_iterator::_vector_iterator(const _vector_iterator& v) is [[gsl::post(*this, V)]] (*this has same pset as v)
and the the contract on std::reference_wrapper::reference_wrapper(T& t) is *this points to t (how would we spell that in attribute syntax?)
I would suggest to only assume that
- a pointer constructed from a pointer copies its pset
and - a pointer constructed from an owner points into the owner.
If a pointer is constructed from something else. we should rely on explicit or implicit function annotations.
I am not sure if this is sufficient. We do need to consider the case where the pointer is constructed from something else. For example, we have std::begin. We can annotate it to return a pointer that is derived from its argument. This definitely is the case when a Pointer or Owner annotated type is the argument. But I could pass a user defined unannotated type to std::begin. Should the implicit annotation imply the same? I am not sure.
Specifically, the flow-sensitive analysis will not even have a PSet for unannotated types. So if a pointer is constructed from something else and we still use the annotations, it is a behavior that is completely different from the flow-sensitive analysis.
std::begin() with an unannotated type cannot do anything sensible and should disable the analysis (i.e. yield a Unknown pset - or static according to the paper.)
The general problem that you are describing might be that we envision function annotations on templated functions, but we might want to have different annotations based on which types the template is instantiated with (i.e. is T an Owner, Pointer or unannotated)?
I think there are multiple points there.
-
As you mentioned, we might want to have separate annotations for separate template specializations.
-
We cannot rely on the function annotations for unannotated type. E.g. what would even the following code mean:
int f() [[gsl::post(lifetime(Return, {static}))]]
We do not even calculate PSets for unannotated types, so annotations are meaningless for those. This is why I think the following is not true:
If a pointer is constructed from something else. we should rely on explicit or implicit function annotations.
What do you think?
For int f() [[gsl::post(lifetime(Return, {static}))]] I would suggest a warning "annotation makes no sense".
For template<T> T f() [[gsl::post(lifetime(Return, {static}))]] that is instantiated with T = int, I would ignore the annotations but probably not warn, because there might be another instantiation with T= int* where this makes sense (?).
If a pointer is constructed from something else. we should rely on explicit or implicit function annotations.
Maybe I'm confusing here something.
If we have a constructor MyPointer::MyPointer(int i), it makes no sense to have any function annotations. int has no pset. On the other hand if we have a constructor MyPointer::MyPointer(int& i), then the argument is a Pointer - see my answer in #51.
Should be fixed in https://reviews.llvm.org/D66806
Landed.