Re: [Bf-committers] PVS-Studio Static Analysis
I think some of these have been fixed already in recent commits from Campbell. On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins wrote: > http://www.viva64.com/en/b/0145/#ID0EO3BK > > It appears that Andrey Karpov has done an analysis of Blender source > code using his PVS-Studio tool. He did this just yesterday, so I > assume the problems he found are still in the source. He offers free > licenses to open source project members (3500 euro software otherwise, > wow). > > I was thinking of investigating the problems he found and seeing if I > could fix them. > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
We have a page with generated CLang static analysis, http://clang.blenderheads.org/trunk/ It probably shows the same bugs as PVS-Studio does. LetterRip On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop wrote: > I think some of these have been fixed already in recent commits from Campbell. > > On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins > wrote: >> http://www.viva64.com/en/b/0145/#ID0EO3BK >> >> It appears that Andrey Karpov has done an analysis of Blender source >> code using his PVS-Studio tool. He did this just yesterday, so I >> assume the problems he found are still in the source. He offers free >> licenses to open source project members (3500 euro software otherwise, >> wow). >> >> I was thinking of investigating the problems he found and seeing if I >> could fix them. >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
Not necessarily; different static analyzers can detect different types of potential problems. The PVS analysis contains some interesting things like the "Misprint in a homogeneous code block" section that I don't think clang's analyzer does. On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote: > We have a page with generated CLang static analysis, > > http://clang.blenderheads.org/trunk/ > > It probably shows the same bugs as PVS-Studio does. > > LetterRip > > On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop > wrote: >> I think some of these have been fixed already in recent commits from >> Campbell. >> >> On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins >> wrote: >>> http://www.viva64.com/en/b/0145/#ID0EO3BK >>> >>> It appears that Andrey Karpov has done an analysis of Blender source >>> code using his PVS-Studio tool. He did this just yesterday, so I >>> assume the problems he found are still in the source. He offers free >>> licenses to open source project members (3500 euro software otherwise, >>> wow). >>> >>> I was thinking of investigating the problems he found and seeing if I >>> could fix them. >>> ___ >>> Bf-committers mailing list >>> Bf-committers@blender.org >>> http://lists.blender.org/mailman/listinfo/bf-committers >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
I'm working through them and fixing what Clang did not catch. I was going to say the same thing as Nicholas, code analysis is such a wide open field with virtually infinite number of things you can check for, that having more than one tool is a good idea. On Tue, Apr 24, 2012 at 6:30 PM, Nicholas Bishop wrote: > Not necessarily; different static analyzers can detect different types > of potential problems. The PVS analysis contains some interesting > things like the "Misprint in a homogeneous code block" section that I > don't think clang's analyzer does. > > On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote: >> We have a page with generated CLang static analysis, >> >> http://clang.blenderheads.org/trunk/ >> >> It probably shows the same bugs as PVS-Studio does. >> >> LetterRip >> >> On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop >> wrote: >>> I think some of these have been fixed already in recent commits from >>> Campbell. >>> >>> On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins >>> wrote: http://www.viva64.com/en/b/0145/#ID0EO3BK It appears that Andrey Karpov has done an analysis of Blender source code using his PVS-Studio tool. He did this just yesterday, so I assume the problems he found are still in the source. He offers free licenses to open source project members (3500 euro software otherwise, wow). I was thinking of investigating the problems he found and seeing if I could fix them. ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers >>> ___ >>> Bf-committers mailing list >>> Bf-committers@blender.org >>> http://lists.blender.org/mailman/listinfo/bf-committers >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
This line in btQuantizedBvh.h confuses me:
int getTriangleIndex() const
{
btAssert(isLeafNode());
unsigned int x=0;
unsigned int y = (~(x&0))<<(31-MAX_NUM_PARTS_IN_BITS);
// Get only the lower bits where the triangle index is stored
return (m_escapeIndexOrTriangleIndex&~(y));
}
Ostensibly the error is that ~0 is -1 (unsigned) and shifting an
unsigned is undefined or unspecified (forget which).
But I'm actually confused why this code is written this way.
x=0, then x&0 == 0, even if x wasn't 0.
Only after that do we get to the undefined behavior.
I guess I could puzzle through it, but somebody else might just know :-)
On Tue, Apr 24, 2012 at 7:30 PM, Jason Wilkins
wrote:
> I'm working through them and fixing what Clang did not catch.
>
> I was going to say the same thing as Nicholas, code analysis is such a
> wide open field with virtually infinite number of things you can check
> for, that having more than one tool is a good idea.
>
> On Tue, Apr 24, 2012 at 6:30 PM, Nicholas Bishop
> wrote:
>> Not necessarily; different static analyzers can detect different types
>> of potential problems. The PVS analysis contains some interesting
>> things like the "Misprint in a homogeneous code block" section that I
>> don't think clang's analyzer does.
>>
>> On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote:
>>> We have a page with generated CLang static analysis,
>>>
>>> http://clang.blenderheads.org/trunk/
>>>
>>> It probably shows the same bugs as PVS-Studio does.
>>>
>>> LetterRip
>>>
>>> On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop
>>> wrote:
I think some of these have been fixed already in recent commits from
Campbell.
On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins
wrote:
> http://www.viva64.com/en/b/0145/#ID0EO3BK
>
> It appears that Andrey Karpov has done an analysis of Blender source
> code using his PVS-Studio tool. He did this just yesterday, so I
> assume the problems he found are still in the source. He offers free
> licenses to open source project members (3500 euro software otherwise,
> wow).
>
> I was thinking of investigating the problems he found and seeing if I
> could fix them.
> ___
> Bf-committers mailing list
> Bf-committers@blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
___
Bf-committers mailing list
Bf-committers@blender.org
http://lists.blender.org/mailman/listinfo/bf-committers
>>> ___
>>> Bf-committers mailing list
>>> Bf-committers@blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-committers
>> ___
>> Bf-committers mailing list
>> Bf-committers@blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers
___
Bf-committers mailing list
Bf-committers@blender.org
http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
You are looking at the code that I fixed yesterday, see my commit message
for details here:
http://lists.blender.org/pipermail/bf-blender-cvs/2012-April/045090.html
>>V610 Undefined behavior. Check the shift operator '<<. The left operand
'(~0)' is negative. extern_bullet btquantizedbvh.h 82
~(0) is signed -1, negative, while ~(x&0) is positive unsigned, so it fixes
the undefined behavior.
Thanks,
Erwin
On 24 April 2012 18:01, Jason Wilkins wrote:
> This line in btQuantizedBvh.h confuses me:
>
> int getTriangleIndex() const
>{
>btAssert(isLeafNode());
>unsigned int x=0;
>unsigned int y = (~(x&0))<<(31-MAX_NUM_PARTS_IN_BITS);
>// Get only the lower bits where the triangle index is
> stored
>return (m_escapeIndexOrTriangleIndex&~(y));
>}
>
> Ostensibly the error is that ~0 is -1 (unsigned) and shifting an
> unsigned is undefined or unspecified (forget which).
>
> But I'm actually confused why this code is written this way.
> x=0, then x&0 == 0, even if x wasn't 0.
>
> Only after that do we get to the undefined behavior.
>
> I guess I could puzzle through it, but somebody else might just know :-)
>
> On Tue, Apr 24, 2012 at 7:30 PM, Jason Wilkins
> wrote:
> > I'm working through them and fixing what Clang did not catch.
> >
> > I was going to say the same thing as Nicholas, code analysis is such a
> > wide open field with virtually infinite number of things you can check
> > for, that having more than one tool is a good idea.
> >
> > On Tue, Apr 24, 2012 at 6:30 PM, Nicholas Bishop
> > wrote:
> >> Not necessarily; different static analyzers can detect different types
> >> of potential problems. The PVS analysis contains some interesting
> >> things like the "Misprint in a homogeneous code block" section that I
> >> don't think clang's analyzer does.
> >>
> >> On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote:
> >>> We have a page with generated CLang static analysis,
> >>>
> >>> http://clang.blenderheads.org/trunk/
> >>>
> >>> It probably shows the same bugs as PVS-Studio does.
> >>>
> >>> LetterRip
> >>>
> >>> On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop
> >>> wrote:
> I think some of these have been fixed already in recent commits from
> Campbell.
>
> On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins
> wrote:
> > http://www.viva64.com/en/b/0145/#ID0EO3BK
> >
> > It appears that Andrey Karpov has done an analysis of Blender source
> > code using his PVS-Studio tool. He did this just yesterday, so I
> > assume the problems he found are still in the source. He offers free
> > licenses to open source project members (3500 euro software
> otherwise,
> > wow).
> >
> > I was thinking of investigating the problems he found and seeing if I
> > could fix them.
> > ___
> > Bf-committers mailing list
> > Bf-committers@blender.org
> > http://lists.blender.org/mailman/listinfo/bf-committers
> ___
> Bf-committers mailing list
> Bf-committers@blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
> >>> ___
> >>> Bf-committers mailing list
> >>> Bf-committers@blender.org
> >>> http://lists.blender.org/mailman/listinfo/bf-committers
> >> ___
> >> Bf-committers mailing list
> >> Bf-committers@blender.org
> >> http://lists.blender.org/mailman/listinfo/bf-committers
> ___
> Bf-committers mailing list
> Bf-committers@blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
>
___
Bf-committers mailing list
Bf-committers@blender.org
http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
Oh, the way it is written now is confusing I think, since it does
several no-ops.
Wouldn't UINT_MAX be better?
On Tue, Apr 24, 2012 at 8:11 PM, Erwin Coumans wrote:
> You are looking at the code that I fixed yesterday, see my commit message
> for details here:
> http://lists.blender.org/pipermail/bf-blender-cvs/2012-April/045090.html
>
>
>>>V610 Undefined behavior. Check the shift operator '<<. The left operand
> '(~0)' is negative. extern_bullet btquantizedbvh.h 82
>
> ~(0) is signed -1, negative, while ~(x&0) is positive unsigned, so it fixes
> the undefined behavior.
> Thanks,
> Erwin
>
>
>
> On 24 April 2012 18:01, Jason Wilkins wrote:
>
>> This line in btQuantizedBvh.h confuses me:
>>
>> int getTriangleIndex() const
>> {
>> btAssert(isLeafNode());
>> unsigned int x=0;
>> unsigned int y = (~(x&0))<<(31-MAX_NUM_PARTS_IN_BITS);
>> // Get only the lower bits where the triangle index is
>> stored
>> return (m_escapeIndexOrTriangleIndex&~(y));
>> }
>>
>> Ostensibly the error is that ~0 is -1 (unsigned) and shifting an
>> unsigned is undefined or unspecified (forget which).
>>
>> But I'm actually confused why this code is written this way.
>> x=0, then x&0 == 0, even if x wasn't 0.
>>
>> Only after that do we get to the undefined behavior.
>>
>> I guess I could puzzle through it, but somebody else might just know :-)
>>
>> On Tue, Apr 24, 2012 at 7:30 PM, Jason Wilkins
>> wrote:
>> > I'm working through them and fixing what Clang did not catch.
>> >
>> > I was going to say the same thing as Nicholas, code analysis is such a
>> > wide open field with virtually infinite number of things you can check
>> > for, that having more than one tool is a good idea.
>> >
>> > On Tue, Apr 24, 2012 at 6:30 PM, Nicholas Bishop
>> > wrote:
>> >> Not necessarily; different static analyzers can detect different types
>> >> of potential problems. The PVS analysis contains some interesting
>> >> things like the "Misprint in a homogeneous code block" section that I
>> >> don't think clang's analyzer does.
>> >>
>> >> On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote:
>> >>> We have a page with generated CLang static analysis,
>> >>>
>> >>> http://clang.blenderheads.org/trunk/
>> >>>
>> >>> It probably shows the same bugs as PVS-Studio does.
>> >>>
>> >>> LetterRip
>> >>>
>> >>> On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop
>> >>> wrote:
>> I think some of these have been fixed already in recent commits from
>> Campbell.
>>
>> On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins
>> wrote:
>> > http://www.viva64.com/en/b/0145/#ID0EO3BK
>> >
>> > It appears that Andrey Karpov has done an analysis of Blender source
>> > code using his PVS-Studio tool. He did this just yesterday, so I
>> > assume the problems he found are still in the source. He offers free
>> > licenses to open source project members (3500 euro software
>> otherwise,
>> > wow).
>> >
>> > I was thinking of investigating the problems he found and seeing if I
>> > could fix them.
>> > ___
>> > Bf-committers mailing list
>> > Bf-committers@blender.org
>> > http://lists.blender.org/mailman/listinfo/bf-committers
>> ___
>> Bf-committers mailing list
>> Bf-committers@blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers
>> >>> ___
>> >>> Bf-committers mailing list
>> >>> Bf-committers@blender.org
>> >>> http://lists.blender.org/mailman/listinfo/bf-committers
>> >> ___
>> >> Bf-committers mailing list
>> >> Bf-committers@blender.org
>> >> http://lists.blender.org/mailman/listinfo/bf-committers
>> ___
>> Bf-committers mailing list
>> Bf-committers@blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers
>>
> ___
> Bf-committers mailing list
> Bf-committers@blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
___
Bf-committers mailing list
Bf-committers@blender.org
http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
The "Uncreated file" is a false alarm. The value of file_ is
initialized by another member function. Probably not the most clear
way to write the code, but not an error. I already contacted viva64
to suggest they tighten up that part of their analysis.
On Tue, Apr 24, 2012 at 8:15 PM, Jason Wilkins
wrote:
> Oh, the way it is written now is confusing I think, since it does
> several no-ops.
>
> Wouldn't UINT_MAX be better?
>
> On Tue, Apr 24, 2012 at 8:11 PM, Erwin Coumans
> wrote:
>> You are looking at the code that I fixed yesterday, see my commit message
>> for details here:
>> http://lists.blender.org/pipermail/bf-blender-cvs/2012-April/045090.html
>>
>>
V610 Undefined behavior. Check the shift operator '<<. The left operand
>> '(~0)' is negative. extern_bullet btquantizedbvh.h 82
>>
>> ~(0) is signed -1, negative, while ~(x&0) is positive unsigned, so it fixes
>> the undefined behavior.
>> Thanks,
>> Erwin
>>
>>
>>
>> On 24 April 2012 18:01, Jason Wilkins wrote:
>>
>>> This line in btQuantizedBvh.h confuses me:
>>>
>>> int getTriangleIndex() const
>>> {
>>> btAssert(isLeafNode());
>>> unsigned int x=0;
>>> unsigned int y = (~(x&0))<<(31-MAX_NUM_PARTS_IN_BITS);
>>> // Get only the lower bits where the triangle index is
>>> stored
>>> return (m_escapeIndexOrTriangleIndex&~(y));
>>> }
>>>
>>> Ostensibly the error is that ~0 is -1 (unsigned) and shifting an
>>> unsigned is undefined or unspecified (forget which).
>>>
>>> But I'm actually confused why this code is written this way.
>>> x=0, then x&0 == 0, even if x wasn't 0.
>>>
>>> Only after that do we get to the undefined behavior.
>>>
>>> I guess I could puzzle through it, but somebody else might just know :-)
>>>
>>> On Tue, Apr 24, 2012 at 7:30 PM, Jason Wilkins
>>> wrote:
>>> > I'm working through them and fixing what Clang did not catch.
>>> >
>>> > I was going to say the same thing as Nicholas, code analysis is such a
>>> > wide open field with virtually infinite number of things you can check
>>> > for, that having more than one tool is a good idea.
>>> >
>>> > On Tue, Apr 24, 2012 at 6:30 PM, Nicholas Bishop
>>> > wrote:
>>> >> Not necessarily; different static analyzers can detect different types
>>> >> of potential problems. The PVS analysis contains some interesting
>>> >> things like the "Misprint in a homogeneous code block" section that I
>>> >> don't think clang's analyzer does.
>>> >>
>>> >> On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote:
>>> >>> We have a page with generated CLang static analysis,
>>> >>>
>>> >>> http://clang.blenderheads.org/trunk/
>>> >>>
>>> >>> It probably shows the same bugs as PVS-Studio does.
>>> >>>
>>> >>> LetterRip
>>> >>>
>>> >>> On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop
>>> >>> wrote:
>>> I think some of these have been fixed already in recent commits from
>>> Campbell.
>>>
>>> On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins
>>> wrote:
>>> > http://www.viva64.com/en/b/0145/#ID0EO3BK
>>> >
>>> > It appears that Andrey Karpov has done an analysis of Blender source
>>> > code using his PVS-Studio tool. He did this just yesterday, so I
>>> > assume the problems he found are still in the source. He offers free
>>> > licenses to open source project members (3500 euro software
>>> otherwise,
>>> > wow).
>>> >
>>> > I was thinking of investigating the problems he found and seeing if I
>>> > could fix them.
>>> > ___
>>> > Bf-committers mailing list
>>> > Bf-committers@blender.org
>>> > http://lists.blender.org/mailman/listinfo/bf-committers
>>> ___
>>> Bf-committers mailing list
>>> Bf-committers@blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-committers
>>> >>> ___
>>> >>> Bf-committers mailing list
>>> >>> Bf-committers@blender.org
>>> >>> http://lists.blender.org/mailman/listinfo/bf-committers
>>> >> ___
>>> >> Bf-committers mailing list
>>> >> Bf-committers@blender.org
>>> >> http://lists.blender.org/mailman/listinfo/bf-committers
>>> ___
>>> Bf-committers mailing list
>>> Bf-committers@blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-committers
>>>
>> ___
>> Bf-committers mailing list
>> Bf-committers@blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers
___
Bf-committers mailing list
Bf-committers@blender.org
http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
for obvious things they mostly find the same issues (even same false positives), but each have a handful that are unique. I've ran blender through cppcheck, splint, sparse and clang-static-checler. the problem is that once you wade through the error logs (mainly false positives), and fixed the actual bugs... Running a second time gives you only the false positives again. If some new bug is added to the source you still needed to read over the false positives (unless your memory is really good). long term we need to have a way to notify us when new errors are added. On Wed, Apr 25, 2012 at 9:30 AM, Nicholas Bishop wrote: > Not necessarily; different static analyzers can detect different types > of potential problems. The PVS analysis contains some interesting > things like the "Misprint in a homogeneous code block" section that I > don't think clang's analyzer does. > > On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote: >> We have a page with generated CLang static analysis, >> >> http://clang.blenderheads.org/trunk/ >> >> It probably shows the same bugs as PVS-Studio does. >> >> LetterRip >> >> On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop >> wrote: >>> I think some of these have been fixed already in recent commits from >>> Campbell. >>> >>> On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins >>> wrote: http://www.viva64.com/en/b/0145/#ID0EO3BK It appears that Andrey Karpov has done an analysis of Blender source code using his PVS-Studio tool. He did this just yesterday, so I assume the problems he found are still in the source. He offers free licenses to open source project members (3500 euro software otherwise, wow). I was thinking of investigating the problems he found and seeing if I could fix them. ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers >>> ___ >>> Bf-committers mailing list >>> Bf-committers@blender.org >>> http://lists.blender.org/mailman/listinfo/bf-committers >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers -- - Campbell ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
I should mention that PVS-Studio contains clang for C++ parsing and preprocessing. As far I can see they have reimplemented some of the clang features (missing in vc++) and have added their own diagnosis patterns. The main feature is visual studio integration, which allows to enable/disable diagnosis options and mark false positives right inside visual studio using GUI. The drawback is impossibility of using it even with VC++ Express Edition (this version does not support extension packages) and of course with non-windows systems. The price of €3500/year also worries me. Be sure not to lock in to a proprietary vendor when the license expires. so basically Andrey Karpov is selling better version of Clang (at least better than native vc++ compiler analyser). But remember 3DMagix and IllusionMage? Of course PVS-Studio is NOT a such scam, but only because Clang is under BSD license and not GPL. The problem is PVS-Studio have never returned a single line of code back to the Clang SVN. As for me I do not support such kind of business. 25 апреля 2012 г. 9:44 пользователь Campbell Barton написал: > for obvious things they mostly find the same issues (even same false > positives), but each have a handful that are unique. > I've ran blender through cppcheck, splint, sparse and clang-static-checler. > > the problem is that once you wade through the error logs (mainly false > positives), and fixed the actual bugs... > Running a second time gives you only the false positives again. > If some new bug is added to the source you still needed to read over > the false positives (unless your memory is really good). > > long term we need to have a way to notify us when new errors are added. > > On Wed, Apr 25, 2012 at 9:30 AM, Nicholas Bishop > wrote: >> Not necessarily; different static analyzers can detect different types >> of potential problems. The PVS analysis contains some interesting >> things like the "Misprint in a homogeneous code block" section that I >> don't think clang's analyzer does. >> >> On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote: >>> We have a page with generated CLang static analysis, >>> >>> http://clang.blenderheads.org/trunk/ >>> >>> It probably shows the same bugs as PVS-Studio does. >>> >>> LetterRip >>> >>> On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop >>> wrote: I think some of these have been fixed already in recent commits from Campbell. On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins wrote: > http://www.viva64.com/en/b/0145/#ID0EO3BK > > It appears that Andrey Karpov has done an analysis of Blender source > code using his PVS-Studio tool. He did this just yesterday, so I > assume the problems he found are still in the source. He offers free > licenses to open source project members (3500 euro software otherwise, > wow). > > I was thinking of investigating the problems he found and seeing if I > could fix them. > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers >>> ___ >>> Bf-committers mailing list >>> Bf-committers@blender.org >>> http://lists.blender.org/mailman/listinfo/bf-committers >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > > > > -- > - Campbell > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
I thought that he had his own tool viva64 that PVS-Studio was based on. I do not really want to argue politics, but I have no problem with the fact that he has added value by integrating it with VS and then released it without source. He honestly may not have even modified clang that much and is just using it as a library. The clang license was specifically chosen so that people could do this kind of thing. I really like the BSD style license :-) I also do not think you can get "locked in" to a tool like this. It checks your source for possible errors. If it stops working then you shrug your shoulders and move on. Microsoft will be releasing a checking tool as part of the next Visual Studio (even the Express version) and I look forward to having more tools to satisfy my nit-picking fetish :-) On Wed, Apr 25, 2012 at 3:35 AM, Lockal S wrote: > I should mention that PVS-Studio contains clang for C++ parsing and > preprocessing. As far I can see they have reimplemented some of the > clang features (missing in vc++) and have added their own diagnosis > patterns. The main feature is visual studio integration, which allows > to enable/disable diagnosis options and mark false positives right > inside visual studio using GUI. The drawback is impossibility of using > it even with VC++ Express Edition (this version does not support > extension packages) and of course with non-windows systems. The price > of EURO 3500/year also worries me. Be sure not to lock in to a proprietary > vendor when the license expires. > > so basically Andrey Karpov is selling better version of Clang (at > least better than native vc++ compiler analyser). But remember 3DMagix > and IllusionMage? Of course PVS-Studio is NOT a such scam, but only > because Clang is under BSD license and not GPL. The problem is > PVS-Studio have never returned a single line of code back to the Clang > SVN. As for me I do not support such kind of business. > > 25 апреля 2012 г. 9:44 пользователь Campbell Barton > написал: >> for obvious things they mostly find the same issues (even same false >> positives), but each have a handful that are unique. >> I've ran blender through cppcheck, splint, sparse and clang-static-checler. >> >> the problem is that once you wade through the error logs (mainly false >> positives), and fixed the actual bugs... >> Running a second time gives you only the false positives again. >> If some new bug is added to the source you still needed to read over >> the false positives (unless your memory is really good). >> >> long term we need to have a way to notify us when new errors are added. >> >> On Wed, Apr 25, 2012 at 9:30 AM, Nicholas Bishop >> wrote: >>> Not necessarily; different static analyzers can detect different types >>> of potential problems. The PVS analysis contains some interesting >>> things like the "Misprint in a homogeneous code block" section that I >>> don't think clang's analyzer does. >>> >>> On Tue, Apr 24, 2012 at 7:20 PM, Tom M wrote: We have a page with generated CLang static analysis, http://clang.blenderheads.org/trunk/ It probably shows the same bugs as PVS-Studio does. LetterRip On Tue, Apr 24, 2012 at 4:15 PM, Nicholas Bishop wrote: > I think some of these have been fixed already in recent commits from > Campbell. > > On Tue, Apr 24, 2012 at 7:11 PM, Jason Wilkins > wrote: >> http://www.viva64.com/en/b/0145/#ID0EO3BK >> >> It appears that Andrey Karpov has done an analysis of Blender source >> code using his PVS-Studio tool. He did this just yesterday, so I >> assume the problems he found are still in the source. He offers free >> licenses to open source project members (3500 euro software otherwise, >> wow). >> >> I was thinking of investigating the problems he found and seeing if I >> could fix them. >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers >>> ___ >>> Bf-committers mailing list >>> Bf-committers@blender.org >>> http://lists.blender.org/mailman/listinfo/bf-committers >> >> >> >> -- >> - Campbell >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers __
Re: [Bf-committers] PVS-Studio Static Analysis
> Running a second time gives you only the false positives again. > If some new bug is added to the source you still needed to read over > the false positives (unless your memory is really good). > > long term we need to have a way to notify us when new errors are added. Sounds like a job for diff? ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
If the source line changes then diff isn't a good enough tool. What is needed is a that these programs need to maintain a database that keeps a signature of each problem that is basically an AST of the area affected and is independent of source line and whitespace changes. Then you could annotate this database with information about false positives instead of your source. Such a database might even be able to keep new false positives from appearing if you duplicate the problem exactly in a different source file. On Wed, Apr 25, 2012 at 11:13 AM, Mr Rasmus Lerchedahl Petersen wrote: >> Running a second time gives you only the false positives again. >> If some new bug is added to the source you still needed to read over >> the false positives (unless your memory is really good). >> >> long term we need to have a way to notify us when new errors are added. > Sounds like a job for diff? > > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
p.s., not that I'm against annotating false positives in source code (or better yet, rewriting in a less suspicious manner). I do not believe in forcing programmers to do things. However, if we ever wanted to require that Blender ship with zero warning flags from these kinds of tools there is going to have to be some kind of centralized way to customize the warnings so that all developers can work against the same template. This is why I like Sonar for Java, you can set up a central web server that tracks all the warnings and everybody can check against that. But it does not do exactly what I suggested (let you mark false-positives in the database). However, it is also a more sensible tool because I've never had it flag something I either did not really have to fix or was not actually a good suggestion to improve the code. C/C++ tools will probably never be that good. On top of that, I compiled Blender with MSVC 2008 warning level 4 and got 14,000 warnings. It would be nice to have a tool that could actually automatically track those. As it is, it seems like a nightmare to even try to see if there is anything useful in those 14k warnings. On Wed, Apr 25, 2012 at 5:57 PM, Jason Wilkins wrote: > If the source line changes then diff isn't a good enough tool. What > is needed is a that these programs need to maintain a database that > keeps a signature of each problem that is basically an AST of the area > affected and is independent of source line and whitespace changes. > Then you could annotate this database with information about false > positives instead of your source. Such a database might even be able > to keep new false positives from appearing if you duplicate the > problem exactly in a different source file. > > On Wed, Apr 25, 2012 at 11:13 AM, Mr Rasmus Lerchedahl Petersen > wrote: >>> Running a second time gives you only the false positives again. >>> If some new bug is added to the source you still needed to read over >>> the false positives (unless your memory is really good). >>> >>> long term we need to have a way to notify us when new errors are added. >> Sounds like a job for diff? >> >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
While its probably possible to setup some `good enough` solution using diff, I tend to agree with Jason - that you need something more comprehensive so some minor edits to a file doesn't give you a lot of changes to the diff to read through that are in fact only line number change or re-ordered functions. Even though you could probably skim-read the diff and tell new additions from line-number offsets - blenders is changing fast enough that doing this every day would become a time waster IMHO. Another issue with diff, clang outputs HTML or PList (some apple xml?) - so diffing this output needs someone to work on a tool for that specifically. Checking if number of reports changed was OK until clang started crashing in some of our source code, so now it gives different bug count every run. http://clang.blenderheads.org/trunk/ (yes, I realize some bug could be added and another removed between revisions - so its not foolproof). On Thu, Apr 26, 2012 at 8:57 AM, Jason Wilkins wrote: > If the source line changes then diff isn't a good enough tool. What > is needed is a that these programs need to maintain a database that > keeps a signature of each problem that is basically an AST of the area > affected and is independent of source line and whitespace changes. > Then you could annotate this database with information about false > positives instead of your source. Such a database might even be able > to keep new false positives from appearing if you duplicate the > problem exactly in a different source file. > > On Wed, Apr 25, 2012 at 11:13 AM, Mr Rasmus Lerchedahl Petersen > wrote: >>> Running a second time gives you only the false positives again. >>> If some new bug is added to the source you still needed to read over >>> the false positives (unless your memory is really good). >>> >>> long term we need to have a way to notify us when new errors are added. >> Sounds like a job for diff? >> >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers -- - Campbell ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
On Thu, Apr 26, 2012 at 9:16 AM, Jason Wilkins wrote: > p.s., not that I'm against annotating false positives in source code > (or better yet, rewriting in a less suspicious manner). I do not > believe in forcing programmers to do things. However, if we ever > wanted to require that Blender ship with zero warning flags from these > kinds of tools there is going to have to be some kind of centralized > way to customize the warnings so that all developers can work against > the same template. > > This is why I like Sonar for Java, you can set up a central web server > that tracks all the warnings and everybody can check against that. > But it does not do exactly what I suggested (let you mark > false-positives in the database). However, it is also a more sensible > tool because I've never had it flag something I either did not really > have to fix or was not actually a good suggestion to improve the code. > C/C++ tools will probably never be that good. > > On top of that, I compiled Blender with MSVC 2008 warning level 4 and > got 14,000 warnings. It would be nice to have a tool that could > actually automatically track those. As it is, it seems like a > nightmare to even try to see if there is anything useful in those 14k > warnings. My first reaction when building blender in MSVC was - "how can anyone use this!" - the output flooded with warnings almost all being near useless, now IIRC the warning level is set lower now but its still not great. A while back I tried to get cmake/gcc building with zero warnings and we are close to being there (some are really hard to get rid of because of differences in external headers across versions). The way I did this was to set -Wall -Wextra, then disable warnings one by one which are things blender does often and are not useful to be warned about on every build. eg: -Wno-unknown-pragmas, -Wno-char-subscripts Another thing that was needed was a way to remove even more warnings for 3rd party libs in extern/ since we dont maintain that code making small edits only get lost when updating the libs. For this there is a CMake macro "remove_strict_flags()", defined in ./build_files/cmake/macros.cmake to remove -Wunused-parameter, -Wstrict-prototypes for example. Locally I use -Werror so if someone commits error causing code into blender It won't build for me, but dont enable this by default since it would get annoying for non developers to build. Theres nothing stopping someone making use of this same methods for MSVC so we get useful warning output. 14,000 warnings sound bad but so many are harmless they just need to be suppressed. > On Wed, Apr 25, 2012 at 5:57 PM, Jason Wilkins > wrote: >> If the source line changes then diff isn't a good enough tool. What >> is needed is a that these programs need to maintain a database that >> keeps a signature of each problem that is basically an AST of the area >> affected and is independent of source line and whitespace changes. >> Then you could annotate this database with information about false >> positives instead of your source. Such a database might even be able >> to keep new false positives from appearing if you duplicate the >> problem exactly in a different source file. >> >> On Wed, Apr 25, 2012 at 11:13 AM, Mr Rasmus Lerchedahl Petersen >> wrote: Running a second time gives you only the false positives again. If some new bug is added to the source you still needed to read over the false positives (unless your memory is really good). long term we need to have a way to notify us when new errors are added. >>> Sounds like a job for diff? >>> >>> ___ >>> Bf-committers mailing list >>> Bf-committers@blender.org >>> http://lists.blender.org/mailman/listinfo/bf-committers > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers -- - Campbell ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
Re: [Bf-committers] PVS-Studio Static Analysis
Interesting problem you have here. While I don't understand all the errors themselves per se, I did take a look at the root web page and a few samples of reports, along with a glimpse at the html it generated. My initial thought would be some sort of regex like parser that would be very similar in theory to the perl "swatch" script, or the logwatch. To summarize the aspects of these that would relate to this: swatch lets you setup regex rules to match or ignore entries in any text file, and can vary the color of the output, ignore it, or perform some action on a match. While logwatch is similar, it basically has a list of ignore rules/regex, and has formatting information for all the different syslog (and friends) log formats that it can parse out the junk (like timestamps and pid's). I figure that if someone felt like spending an evening, they could probably take the 2 key concepts (regex, and ignore rules per file and/or error) from these two programs and toss them into an html scraper script, you could probably get something that could alert you to just the stuff that has changed. For example, looking at a few different reports, I noticed a trend of the same file to have the same error on the same line number. Clearly in such a case, this output should be placed on the ignore list (well, assuming you choose to ignore it), and only report on changes where there is a new "primary key" of sorts, constructed from the uniqueness of the filename, error type and line number. The big problem I see with this, would be maintaining the script in the event that the html of the clang reports changes. This is actually the type of problem that I ran into when making the blender filesize chart, namely that over time, the conventions used to name files and indicate features was different per arch, release and feature set, thus making it near impossible to script or automate anything without hand verification. I suppose the question is, how important would something like this be, who will write it, how will it be written (perl+dbm, python+sqlite, etc etc etc), who will maintain it, who will use and benefit from it. Tbh, it sounds like a can of worms :) Anyways, felt like chiming in, later guys o/ troubled On Wed, Apr 25, 2012 at 9:36 PM, Campbell Barton wrote: > While its probably possible to setup some `good enough` solution using > diff, I tend to agree with Jason - that you need something more > comprehensive so some minor edits to a file doesn't give you a lot of > changes to the diff to read through that are in fact only line number > change or re-ordered functions. > > Even though you could probably skim-read the diff and tell new > additions from line-number offsets - blenders is changing fast enough > that doing this every day would become a time waster IMHO. > > Another issue with diff, clang outputs HTML or PList (some apple xml?) > - so diffing this output needs someone to work on a tool for that > specifically. > > Checking if number of reports changed was OK until clang started > crashing in some of our source code, so now it gives different bug > count every run. > http://clang.blenderheads.org/trunk/ > > (yes, I realize some bug could be added and another removed between > revisions - so its not foolproof). > > > On Thu, Apr 26, 2012 at 8:57 AM, Jason Wilkins > wrote: >> If the source line changes then diff isn't a good enough tool. What >> is needed is a that these programs need to maintain a database that >> keeps a signature of each problem that is basically an AST of the area >> affected and is independent of source line and whitespace changes. >> Then you could annotate this database with information about false >> positives instead of your source. Such a database might even be able >> to keep new false positives from appearing if you duplicate the >> problem exactly in a different source file. >> >> On Wed, Apr 25, 2012 at 11:13 AM, Mr Rasmus Lerchedahl Petersen >> wrote: Running a second time gives you only the false positives again. If some new bug is added to the source you still needed to read over the false positives (unless your memory is really good). long term we need to have a way to notify us when new errors are added. >>> Sounds like a job for diff? >>> >>> ___ >>> Bf-committers mailing list >>> Bf-committers@blender.org >>> http://lists.blender.org/mailman/listinfo/bf-committers >> ___ >> Bf-committers mailing list >> Bf-committers@blender.org >> http://lists.blender.org/mailman/listinfo/bf-committers > > > > -- > - Campbell > ___ > Bf-committers mailing list > Bf-committers@blender.org > http://lists.blender.org/mailman/listinfo/bf-committers ___ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
