[ITK-dev] Bug in the library, or just in the test..?

[Casey Goodlett]

Hi David,

I'm not sure that m_Data is nullptr.  I assume its an alias for std::vector
of size zero.

I've hit this bug a couple of times.  In visual studio debug mode
&std::vector[0] raises an error for vectors of size zero even if that
address is never used for anything.  The bug is not in fill_n because the
error is raised before fill_n is called.  Calling fill_n with a bogus
pointer and size zero is valid.

On linux the system is happy to generate a bogus pointer thats never used
for anything so no valgrind errors are ever generated.

I recommend checking an if(m_NumElements >0) in pre c++-11.  In c++11 I
recommend using the data() method which was added for precisely this reason.

Hope that helps.


Casey Goodlett

On Fri, Oct 23, 2015 at 1:47 PM, David Cole via Insight-developers <
insight-developers at itk.org> wrote:

> It is definitely related to STL being checked in a Debug Visual Studio
> build... I get a bunch of ASSERT dialogs that pop up, so the test
> times out and hangs there with the dialog up until I kill it manually.
>
> If I ignore through all the dialogs that pop up (click ignore on each
> of them, there are exactly 12 in total), then the test output is
> normal, and it reports that it has PASSED.
>
> I can wait for a fix. I was just asking to see if it was a known
> thing, and find out if anybody was working on it, or if there was a
> general recommendation about what the right fix for it is before I
> attempt to fix it myself.
>
> I'm curious, though, why is using "&this->m_Data[0]" acceptable if
> m_Data is nullptr ...? Why doesn't valgrind pick this up as a problem?
>
>
> D
>
>
>
>
> On Fri, Oct 23, 2015 at 1:33 PM, Luc Hermitte <luc.hermitte at c-s.fr> wrote:
> > Hi,
> >
> > Le 23/10/2015 18:43, David Cole via Insight-developers a écrit :
> >> The itkTransformTest is the only test failing on my Visual Studio 2013
> >> Debug dashboard because of this code:
> >
> > What is the exact error produced when your run the test in verbose mode?
> >
> >
> >> The problem is in the Fill implementation:
> >>
> >>     template< typename TValue >
> >>     void VariableLengthVector< TValue >
> >>     ::Fill(TValue const & v) ITK_NOEXCEPT
> >>     {
> >>       std::fill_n(&this->m_Data[0], m_NumElements, v);
> >>     }
> >>
> >> In this test call, m_Data is nullptr, and m_NumElements is 0, so
> >> clearly,&m_Data[0] is not a useful pointer, and with 0 elements, there
> >> is nothing to fill.
> >>
> >> So.... should the implementation of Fill be changed to be conditional
> >> on the pointer being non-nullptr, or the number of elements being
> >> non-zero, or both? (Are other Fill implementations protected like so?)
> >>
> >> Or should the test be changed to avoid calling Fill when there are no
> >> elements...?
> >
> > I'd say that this is a bug in std::fill_n (or something related to the
> > STL being in CHECKED MODE) and that we should fall back into a plain
> > loop. We should avoid adding such defensive tests (if (!empty) fill_n()).
> >
> > I can take care of this issue (only on Monday). If you don't mind, I'd
> > rather avoid rebasing yet another time and first go through
> > http://review.source.kitware.com/#/c/20253/10 (I could also fix the
> > issue I've introduced in my first patch on VLV in this patch, but this
> > is not the usual patching policy if I understand correctly)
> >
> >
> > Regards,
> > --
> > Luc Hermitte
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.php
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/mailman/listinfo/insight-developers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/insight-developers/attachments/20151023/96b0af77/attachment.html>