[vtk-developers] Help with vtkDataSetSurfaceFilter.cxx:2144...

[Sean McBride]

On Mon, 16 Dec 2013 16:36:14 -0500, Berk Geveci said:

>I looked into this. It may actually be a false positive. That code is a
>little tricky. Even though ptArray is 4 big in the declaration of the
>class, the code actually reinterprets a byte array as a pointer to
>vtkFastGeomQuadStruct
>after allocating the right size. It essentially allocates
>sizeof(vtkFastGeomQuadStruct)
>+ sizeof(vtkIdType) * (numPts - 4). So there is the right extra space as
>padding and things should work as long as ptArray is the last member of the
>struct. However, the sanitizer may rightfully not like this.  Can you point
>out which tests are causing the warning so I can track this down more? There
>is a bug in there but it is very unlikely to occur. Even if it is a false
>positive, we may still want to fix it just to have a clean sanitizer output.

Berk,

Thanks for looking into it!

I see now the allocation and reinterpret_cast you speak of (in NewFastGeomQuad()), and that would explain why this code actually works in practice.

I'm not enough of a language lawyer to say for sure, but I'd wager heavily it is still undefined behaviour to do what it's doing.

This issue is the cause of most failures on my 'torture' dashboard, here:
<http://open.cdash.org/viewTest.php?onlyfailed&buildid=3140644>

Sorting by run duration, the simplest may be "vtkCommonDataModelCxx-TestPolyhedron1":
<http://open.cdash.org/testDetails.php?test=217686018&build=3140644>

If this were C99, I'd suggest using a 'flexible array member' but C++ doesn't have those...  I'm sure you'll find a suitable refactoring!

Cheers,

-- 
____________________________________________________________
Sean McBride, B. Eng                 sean at rogue-research.com
Rogue Research                        www.rogue-research.com 
Mac Software Developer              Montréal, Québec, Canada