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

[Bradley Lowekamp]

We have seen this a couple times before [1]. Unfortunately the windows debug testing takes too long and times out in many cases so we don't have it on the dashboard for many versions.

I would add the if statement as Casey suggests.

Brad


[1] https://github.com/InsightSoftwareConsortium/ITK/commit/d21f4a64c3c04a83d38324316e7e2bffa7c14d14

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

> I am sure m_Data is nullptr. In the debugger, it says so. The value of
> "this" is a valid pointer, and the value of m_Data is 0x00000000.
> 
> The assert is coming from squarely inside a call to std::fill_n with the text:
> 
>    Debug Assertion Failed!
> 
>    Program: C:\WINDOWS\SYSTEM32\MSVCP120D.dll
>    File: C:\Program Files (x86)\Microsoft Visual Studio
>        12.0\VC\INCLUDE\xutility
>    Line: 2713
> 
>    Expression: invalid null pointer
> 
> That line is inside std::fill_n, so ... pretty sure m_Data is nullptr.
> 
> 
> D
> 
> 
> 
> 
> On Fri, Oct 23, 2015 at 1:59 PM, Casey Goodlett <gcasey at gmail.com> wrote:
>> 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
>> 
>> 
> _______________________________________________
> 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