Skip to content
This repository has been archived by the owner on Jul 3, 2024. It is now read-only.

False-positive with deref address-of #49

Closed
mgehre opened this issue Aug 22, 2019 · 11 comments
Closed

False-positive with deref address-of #49

mgehre opened this issue Aug 22, 2019 · 11 comments

Comments

@mgehre
Copy link
Owner

mgehre commented Aug 22, 2019

See
https://godbolt.org/z/1tWI6O

This lead to reverting https://reviews.llvm.org/D66179.

@Xazax-hun, could you take a look, please?

@Xazax-hun
Copy link
Collaborator

Sure! I'm working on something else, but will try to look into this in a few hours!

@Xazax-hun
Copy link
Collaborator

It should be fixed with 4226f4c but I want to run this over some project before creating a PR.

@Xazax-hun
Copy link
Collaborator

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.

@mgehre
Copy link
Owner Author

mgehre commented Aug 23, 2019

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.

@Xazax-hun
Copy link
Collaborator

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.

@Xazax-hun
Copy link
Collaborator

Xazax-hun commented Aug 23, 2019

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.

@mgehre
Copy link
Owner Author

mgehre commented Aug 23, 2019

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)?

@Xazax-hun
Copy link
Collaborator

I think there are multiple points there.

  1. As you mentioned, we might want to have separate annotations for separate template specializations.

  2. 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?

@mgehre
Copy link
Owner Author

mgehre commented Aug 24, 2019

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.

@Xazax-hun
Copy link
Collaborator

Should be fixed in https://reviews.llvm.org/D66806

@Xazax-hun
Copy link
Collaborator

Landed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants