[vtk-developers] Safer item access in Python wrapped VTK arrays

Mon Aug 21 01:04:06 EDT 2017

I've put together an MR that uses the VTK_EXPECTS() macro to add Python
safety checks to vtkPoints:
https://gitlab.kitware.com/vtk/vtk/merge_requests/3176

If the point Id for GetPoint() or SetPoint() is out of range, then Python
will raise an exception instead of crashing.

 - David

On Thu, Aug 10, 2017 at 12:44 AM, Elvis Stansvik <
elvis.stansvik at orexplore.com> wrote:

> Den 9 aug. 2017 10:54 em skrev "David Gobbi" <david.gobbi at gmail.com>:
>
> 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 <david.gobbi at gmail.com> 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 <lasso at queensu.ca> 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/S
>>> C22/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:david.gobbi at gmail.com]
>>> *Sent:* Friday, August 4, 2017 4:05 PM
>>> *To:* Berk Geveci <berk.geveci at kitware.com>
>>> *Cc:* Andras Lasso <lasso at queensu.ca>; VTK Developers <
>>> vtk-developers at vtk.org>
>>> *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 <berk.geveci at kitware.com>
>>> 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 <david.gobbi at gmail.com>
>>> 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
>>> <https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.vtk.org%2FWiki%2FVTK%2FWrapping_hints&data=02%7C01%7Classo%40queensu.ca%7C37f3749bcbbf4422f0ba08d4db741f07%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636374739163412038&sdata=FtQM8JHWrl8ReFwC3EFD4j0COBmWqm1r6k9b%2BF5tZzU%3D&reserved=0>,
>>> 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 <lasso at queensu.ca> 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/opensou
> rce/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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20170820/1791e039/attachment.html>