Safer item access in Python wrapped VTK arrays

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Safer item access in Python wrapped VTK arrays

Andras Lasso

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras

 


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

David Gobbi
Hi Andras,

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 - David


On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras



_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

Andras Lasso

Thank you, it would be awesome to have this (and the other proposed hints, too).

 

Andras

 

From: David Gobbi [mailto:[hidden email]]
Sent: Friday, August 4, 2017 1:06 PM
To: Andras Lasso <[hidden email]>
Cc: VTK Developers <[hidden email]>
Subject: Re: Safer item access in Python wrapped VTK arrays

 

Hi Andras,

 

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

 

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

 

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

 

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 

 - David

 

 

On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras

 


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

Berk Geveci-2
In reply to this post by David Gobbi
As much as I support the concept of range checking in the wrappers, this hint will make things look very ugly and non-standard at a time where we are trying to move towards more standard looking C++. Isn't there a way of doing this without mangling the signatures?

On Fri, Aug 4, 2017 at 1:06 PM, David Gobbi <[hidden email]> wrote:
Hi Andras,

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 - David


On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras



_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers




_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

David Gobbi
For a while, we kicked around the idea of having the hints in the comments.  This would be less intrusive than adding C++11 attributes the declarations (i.e. what I've been doing so far).

The C++11 attributes are definitely the cleanest way for the wrappers to store the hints, since the hints become part of the parse tree, so I want to keep them as the primary hinting mechanism.  Comment-based hints could be added as a secondary mechanism, i.e. after a declaration has been parsed we can check the docstring for extra hints, and then add the hints to the parse tree before the wrapper code is generated.

I'll have to search through the archives to remember what kind of syntax we were considering for comment-based hints.

 - David


On Fri, Aug 4, 2017 at 1:30 PM, Berk Geveci <[hidden email]> wrote:
As much as I support the concept of range checking in the wrappers, this hint will make things look very ugly and non-standard at a time where we are trying to move towards more standard looking C++. Isn't there a way of doing this without mangling the signatures?

On Fri, Aug 4, 2017 at 1:06 PM, David Gobbi <[hidden email]> wrote:
Hi Andras,

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 - David


On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras



_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

Andras Lasso

Eventually, C++17 standard contracts may be used to specify these bounds checks. For now, we may use the same format but add them as comments or protected by macros; and when we switch to C++17 then these can be converted to actual contracts.

 

Proposal for contracts: http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4415.pdf

 

Example contract:

T& operator[](size_t i) [[expects: i >= 0 && i < size()]];

 

For now, we could do this in VTK:

T& operator[](size_t i) VTK_CONTRACT([[expects: i >= 0 && i < size()]]);

 

Andras

 

 

From: David Gobbi [mailto:[hidden email]]
Sent: Friday, August 4, 2017 4:05 PM
To: Berk Geveci <[hidden email]>
Cc: Andras Lasso <[hidden email]>; VTK Developers <[hidden email]>
Subject: Re: [vtk-developers] Safer item access in Python wrapped VTK arrays

 

For a while, we kicked around the idea of having the hints in the comments.  This would be less intrusive than adding C++11 attributes the declarations (i.e. what I've been doing so far).

 

The C++11 attributes are definitely the cleanest way for the wrappers to store the hints, since the hints become part of the parse tree, so I want to keep them as the primary hinting mechanism.  Comment-based hints could be added as a secondary mechanism, i.e. after a declaration has been parsed we can check the docstring for extra hints, and then add the hints to the parse tree before the wrapper code is generated.

 

I'll have to search through the archives to remember what kind of syntax we were considering for comment-based hints.

 

 - David

 

 

On Fri, Aug 4, 2017 at 1:30 PM, Berk Geveci <[hidden email]> wrote:

As much as I support the concept of range checking in the wrappers, this hint will make things look very ugly and non-standard at a time where we are trying to move towards more standard looking C++. Isn't there a way of doing this without mangling the signatures?

 

On Fri, Aug 4, 2017 at 1:06 PM, David Gobbi <[hidden email]> wrote:

Hi Andras,

 

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

 

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

 

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

 

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 

 - David

 

 

On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras

 


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

David Gobbi
Hi Andras,

I agree that these contracts are a better choice than inventing something of our own, I'm going to play around with this to see how easy it would be to add them to the wrappers.

The macros can be made a little more compact:
T& operator[](size_t i) VTK_PRECONDITION(i >= 0 && i < size());

Or we could use VTK_EXPECTS(i >= 0 && i < size());

 - David



On Fri, Aug 4, 2017 at 7:39 PM, Andras Lasso <[hidden email]> wrote:

Eventually, C++17 standard contracts may be used to specify these bounds checks. For now, we may use the same format but add them as comments or protected by macros; and when we switch to C++17 then these can be converted to actual contracts.

 

Proposal for contracts: http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4415.pdf

 

Example contract:

T& operator[](size_t i) [[expects: i >= 0 && i < size()]];

 

For now, we could do this in VTK:

T& operator[](size_t i) VTK_CONTRACT([[expects: i >= 0 && i < size()]]);

 

Andras

 

 

From: David Gobbi [mailto:[hidden email]]
Sent: Friday, August 4, 2017 4:05 PM
To: Berk Geveci <[hidden email]>
Cc: Andras Lasso <[hidden email]>; VTK Developers <[hidden email]>
Subject: Re: [vtk-developers] Safer item access in Python wrapped VTK arrays

 

For a while, we kicked around the idea of having the hints in the comments.  This would be less intrusive than adding C++11 attributes the declarations (i.e. what I've been doing so far).

 

The C++11 attributes are definitely the cleanest way for the wrappers to store the hints, since the hints become part of the parse tree, so I want to keep them as the primary hinting mechanism.  Comment-based hints could be added as a secondary mechanism, i.e. after a declaration has been parsed we can check the docstring for extra hints, and then add the hints to the parse tree before the wrapper code is generated.

 

I'll have to search through the archives to remember what kind of syntax we were considering for comment-based hints.

 

 - David

 

 

On Fri, Aug 4, 2017 at 1:30 PM, Berk Geveci <[hidden email]> wrote:

As much as I support the concept of range checking in the wrappers, this hint will make things look very ugly and non-standard at a time where we are trying to move towards more standard looking C++. Isn't there a way of doing this without mangling the signatures?

 

On Fri, Aug 4, 2017 at 1:06 PM, David Gobbi <[hidden email]> wrote:

Hi Andras,

 

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

 

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

 

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

 

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 

 - David

 

 

On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras

 



_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

David Gobbi
Here's what I'm currently thinking about for wrapper hints, please let me know if the syntax is too obtrusive.

The basic precondition contract, similar to proposed C++ contracts:

    float GetValue(vtkIdType i)  VTK_EXPECTS(i >= 0 && i < GetNumberOfValues());
 
And here's the format that I'm currently thinking about for wrapper size hints (to replace our "hints" file):

    double *GetPoint(vtkIdType i)  VTK_SIZEHINT(return[3]);

When used in combination things start to look ugly, but the method signature is still readable:

    void SetTuple(vtkIdType i, float *a)
        VTK_EXPECTS(i >= 0 && i < GetNumberOfTuples())
        VTK_SIZEHINT(a[GetNumberOfComponents()]);

    float *GetTuple(vtkIdType i)
        VTK_EXPECTS(i >= 0 && i < GetNumberOfTuples())
        VTK_SIZEHINT(return[GetNumberOfComponents()]);

Any thoughts?  I'd really like to express the size hint as some kind of contract, but C++ doesn't provide a way to check the number of values pointed to by a pointer.

As an aside: for vtkDataArray, I don't actually intend to add VTK_SIZEHINT as shown above.  Currently the size hints for vtkDataArray are hard-coded into the wrappers, and I'll probably keep them that way in order to keep the C++ headers clean.

 - David


On Mon, Aug 7, 2017 at 6:56 AM, David Gobbi <[hidden email]> wrote:
Hi Andras,

I agree that these contracts are a better choice than inventing something of our own, I'm going to play around with this to see how easy it would be to add them to the wrappers.

The macros can be made a little more compact:
T& operator[](size_t i) VTK_PRECONDITION(i >= 0 && i < size());

Or we could use VTK_EXPECTS(i >= 0 && i < size());

 - David



On Fri, Aug 4, 2017 at 7:39 PM, Andras Lasso <[hidden email]> wrote:

Eventually, C++17 standard contracts may be used to specify these bounds checks. For now, we may use the same format but add them as comments or protected by macros; and when we switch to C++17 then these can be converted to actual contracts.

 

Proposal for contracts: http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4415.pdf

 

Example contract:

T& operator[](size_t i) [[expects: i >= 0 && i < size()]];

 

For now, we could do this in VTK:

T& operator[](size_t i) VTK_CONTRACT([[expects: i >= 0 && i < size()]]);

 

Andras

 

 

From: David Gobbi [mailto:[hidden email]]
Sent: Friday, August 4, 2017 4:05 PM
To: Berk Geveci <[hidden email]>
Cc: Andras Lasso <[hidden email]>; VTK Developers <[hidden email]>
Subject: Re: [vtk-developers] Safer item access in Python wrapped VTK arrays

 

For a while, we kicked around the idea of having the hints in the comments.  This would be less intrusive than adding C++11 attributes the declarations (i.e. what I've been doing so far).

 

The C++11 attributes are definitely the cleanest way for the wrappers to store the hints, since the hints become part of the parse tree, so I want to keep them as the primary hinting mechanism.  Comment-based hints could be added as a secondary mechanism, i.e. after a declaration has been parsed we can check the docstring for extra hints, and then add the hints to the parse tree before the wrapper code is generated.

 

I'll have to search through the archives to remember what kind of syntax we were considering for comment-based hints.

 

 - David

 

 

On Fri, Aug 4, 2017 at 1:30 PM, Berk Geveci <[hidden email]> wrote:

As much as I support the concept of range checking in the wrappers, this hint will make things look very ugly and non-standard at a time where we are trying to move towards more standard looking C++. Isn't there a way of doing this without mangling the signatures?

 

On Fri, Aug 4, 2017 at 1:06 PM, David Gobbi <[hidden email]> wrote:

Hi Andras,

 

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

 

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

 

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

 

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 

 - David

 

 

On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras

 




_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

Andras Lasso

For me, they look OK.

 

VTK_SIZEHINT is not a beautiful syntax, but I understand why it is like this and don't have a better suggestion. How do you specify size hints for multiple arguments?

 

Andras

 

From: [hidden email]
Sent: Wednesday, August 9, 2017 16:53
To: [hidden email]; [hidden email]
Cc: [hidden email]
Subject: Re: [vtk-developers] Safer item access in Python wrapped VTK arrays

 

Here's what I'm currently thinking about for wrapper hints, please let me know if the syntax is too obtrusive.

The basic precondition contract, similar to proposed C++ contracts:

    float GetValue(vtkIdType i)  VTK_EXPECTS(i >= 0 && i < GetNumberOfValues());
 
And here's the format that I'm currently thinking about for wrapper size hints (to replace our "hints" file):

    double *GetPoint(vtkIdType i)  VTK_SIZEHINT(return[3]);

When used in combination things start to look ugly, but the method signature is still readable:

    void SetTuple(vtkIdType i, float *a)
        VTK_EXPECTS(i >= 0 && i < GetNumberOfTuples())
        VTK_SIZEHINT(a[GetNumberOfComponents()]);

    float *GetTuple(vtkIdType i)
        VTK_EXPECTS(i >= 0 && i < GetNumberOfTuples())
        VTK_SIZEHINT(return[GetNumberOfComponents()]);

Any thoughts?  I'd really like to express the size hint as some kind of contract, but C++ doesn't provide a way to check the number of values pointed to by a pointer.

As an aside: for vtkDataArray, I don't actually intend to add VTK_SIZEHINT as shown above.  Currently the size hints for vtkDataArray are hard-coded into the wrappers, and I'll probably keep them that way in order to keep the C++ headers clean.

 - David


On Mon, Aug 7, 2017 at 6:56 AM, David Gobbi <[hidden email]> wrote:
Hi Andras,

I agree that these contracts are a better choice than inventing something of our own, I'm going to play around with this to see how easy it would be to add them to the wrappers.

The macros can be made a little more compact:
T& operator[](size_t i) VTK_PRECONDITION(i >= 0 && i < size());

Or we could use VTK_EXPECTS(i >= 0 && i < size());

 - David



On Fri, Aug 4, 2017 at 7:39 PM, Andras Lasso <[hidden email]> wrote:

Eventually, C++17 standard contracts may be used to specify these bounds checks. For now, we may use the same format but add them as comments or protected by macros; and when we switch to C++17 then these can be converted to actual contracts.

 

Proposal for contracts: http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4415.pdf

 

Example contract:

T& operator[](size_t i) [[expects: i >= 0 && i < size()]];

 

For now, we could do this in VTK:

T& operator[](size_t i) VTK_CONTRACT([[expects: i >= 0 && i < size()]]);

 

Andras

 

 

From: David Gobbi [mailto:[hidden email]]
Sent: Friday, August 4, 2017 4:05 PM
To: Berk Geveci <[hidden email]>
Cc: Andras Lasso <[hidden email]>; VTK Developers <[hidden email]>
Subject: Re: [vtk-developers] Safer item access in Python wrapped VTK arrays

 

For a while, we kicked around the idea of having the hints in the comments.  This would be less intrusive than adding C++11 attributes the declarations (i.e. what I've been doing so far).

 

The C++11 attributes are definitely the cleanest way for the wrappers to store the hints, since the hints become part of the parse tree, so I want to keep them as the primary hinting mechanism.  Comment-based hints could be added as a secondary mechanism, i.e. after a declaration has been parsed we can check the docstring for extra hints, and then add the hints to the parse tree before the wrapper code is generated.

 

I'll have to search through the archives to remember what kind of syntax we were considering for comment-based hints.

 

 - David

 

 

On Fri, Aug 4, 2017 at 1:30 PM, Berk Geveci <[hidden email]> wrote:

As much as I support the concept of range checking in the wrappers, this hint will make things look very ugly and non-standard at a time where we are trying to move towards more standard looking C++. Isn't there a way of doing this without mangling the signatures?

 

On Fri, Aug 4, 2017 at 1:06 PM, David Gobbi <[hidden email]> wrote:

Hi Andras,

 

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

 

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

 

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

 

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 

 - David

 

 

On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras

 




_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

David Gobbi
On Wed, Aug 9, 2017 at 3:45 PM, Andras Lasso <[hidden email]> wrote:

For me, they look OK.

 

VTK_SIZEHINT is not a beautiful syntax, but I understand why it is like this and don't have a better suggestion.How do you specify size hints for multiple arguments?


By having multiple VTK_SIZEHINT()s, one for each argument that requires a hint.

 - David
 

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Safer item access in Python wrapped VTK arrays

Elvis Stansvik
In reply to this post by David Gobbi
Den 9 aug. 2017 10:54 em skrev "David Gobbi" <[hidden email]>:
Here's what I'm currently thinking about for wrapper hints, please let me know if the syntax is too obtrusive.

The basic precondition contract, similar to proposed C++ contracts:

    float GetValue(vtkIdType i)  VTK_EXPECTS(i >= 0 && i < GetNumberOfValues());

This is really just a stylistic nitpick, and a matter of taste of course, so take it or leave it, but I like it when the var is in between the bounds, e.g.

   0 <= i && i < GetNumberOfValues()

As for the hint syntax, I also think what you last posted is readable enough.

Elvis

 
And here's the format that I'm currently thinking about for wrapper size hints (to replace our "hints" file):

    double *GetPoint(vtkIdType i)  VTK_SIZEHINT(return[3]);

When used in combination things start to look ugly, but the method signature is still readable:

    void SetTuple(vtkIdType i, float *a)
        VTK_EXPECTS(i >= 0 && i < GetNumberOfTuples())
        VTK_SIZEHINT(a[GetNumberOfComponents()]);

    float *GetTuple(vtkIdType i)
        VTK_EXPECTS(i >= 0 && i < GetNumberOfTuples())
        VTK_SIZEHINT(return[GetNumberOfComponents()]);

Any thoughts?  I'd really like to express the size hint as some kind of contract, but C++ doesn't provide a way to check the number of values pointed to by a pointer.

As an aside: for vtkDataArray, I don't actually intend to add VTK_SIZEHINT as shown above.  Currently the size hints for vtkDataArray are hard-coded into the wrappers, and I'll probably keep them that way in order to keep the C++ headers clean.

 - David


On Mon, Aug 7, 2017 at 6:56 AM, David Gobbi <[hidden email]> wrote:
Hi Andras,

I agree that these contracts are a better choice than inventing something of our own, I'm going to play around with this to see how easy it would be to add them to the wrappers.

The macros can be made a little more compact:
T& operator[](size_t i) VTK_PRECONDITION(i >= 0 && i < size());

Or we could use VTK_EXPECTS(i >= 0 && i < size());

 - David



On Fri, Aug 4, 2017 at 7:39 PM, Andras Lasso <[hidden email]> wrote:

Eventually, C++17 standard contracts may be used to specify these bounds checks. For now, we may use the same format but add them as comments or protected by macros; and when we switch to C++17 then these can be converted to actual contracts.

 

Proposal for contracts: http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2015/n4415.pdf

 

Example contract:

T& operator[](size_t i) [[expects: i >= 0 && i < size()]];

 

For now, we could do this in VTK:

T& operator[](size_t i) VTK_CONTRACT([[expects: i >= 0 && i < size()]]);

 

Andras

 

 

From: David Gobbi [mailto:[hidden email]]
Sent: Friday, August 4, 2017 4:05 PM
To: Berk Geveci <[hidden email]>
Cc: Andras Lasso <[hidden email]>; VTK Developers <[hidden email]>
Subject: Re: [vtk-developers] Safer item access in Python wrapped VTK arrays

 

For a while, we kicked around the idea of having the hints in the comments.  This would be less intrusive than adding C++11 attributes the declarations (i.e. what I've been doing so far).

 

The C++11 attributes are definitely the cleanest way for the wrappers to store the hints, since the hints become part of the parse tree, so I want to keep them as the primary hinting mechanism.  Comment-based hints could be added as a secondary mechanism, i.e. after a declaration has been parsed we can check the docstring for extra hints, and then add the hints to the parse tree before the wrapper code is generated.

 

I'll have to search through the archives to remember what kind of syntax we were considering for comment-based hints.

 

 - David

 

 

On Fri, Aug 4, 2017 at 1:30 PM, Berk Geveci <[hidden email]> wrote:

As much as I support the concept of range checking in the wrappers, this hint will make things look very ugly and non-standard at a time where we are trying to move towards more standard looking C++. Isn't there a way of doing this without mangling the signatures?

 

On Fri, Aug 4, 2017 at 1:06 PM, David Gobbi <[hidden email]> wrote:

Hi Andras,

 

Yes, this would be possible.  It could be hard-coded into the wrappers, or even better, in the header file a generic hint could be added to the index parameter so that the index would be checked against the size of the array:

 

  float GetValue(vtkIdType idx VTK_RANGECHECK(0,GetNumberOfValues()));

 

I have a list of wrapper hints at http://www.vtk.org/Wiki/VTK/Wrapping_hints, and I can add this "RANGECHECK" hint to the "Proposed hints" section.

 

I'm hoping to find time to implement more of these wrapper hints over the next few weeks.

 

 - David

 

 

On Fri, Aug 4, 2017 at 9:53 AM, Andras Lasso <[hidden email]> wrote:

Hi David,

 

Users who mostly use VTK via Python wrapping are sometimes surprised how easily they can crash the application by accessing out-of-bounds elements in arrays. For example, this crashes an application:

 

>>> a=vtk.vtkStringArray()

>>> print(a.GetValue(0))

 

I first thought that it should be handled by documentation and training, but it would be nicer if the wrappers could handle this.

 

Would it be feasible to add bounds check to VTK array accessors in Python wrappers, so that in case of an out of bounds access, a Python exception would be raised instead of crashing the application?

Andras

 




_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers




_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Loading...