Re: Clang Static Analyzer Warnings

[Eric]

What warning flags are you using? I turned on -Weverything and didn't see these.

I've reviewed the code again and I don't believe any of these are actually problems. theCachedLayer is initialized with the value from the instance variable, which is retained by the property setter.

Eric

On Thursday, May 9, 2013 10:14:02 AM UTC-4, Jason Osborn wrote:

I've implemented Core Plot into a large application, and our Clang Scan is showing 3 warnings that I'd like to get some more information on:

1) 

/ios-core-plot-library/framework/Source/CPTPlotSymbol.m

Leak

Memory (Core Foundation/Objective-C)

Potential leak of an object stored into 'theCachedLayer'

-(void)renderInContext:(CGContextRef)context atPoint:(CGPoint)center scale:(CGFloat)scale alignToPixels:(BOOL)alignToPixels

...

502	CGContextDrawLayerInRect(context, CPTRectMake(origin.x, origin.y, layerSize.width, layerSize.height), theCachedLayer);
503	}
504	}
	11 ← Object leaked: object allocated and stored into 'theCachedLayer' is not referenced later in this execution path and has a retain count of +1
505

2)

/ios-core-plot-library/framework/Source/CPTScatterPlot.m

Assigned value is garbage or undefined

Logic error

The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage

488	if ( !drawPointFlags[i] || NSDecimalIsNotANumber(&x) || NSDecimalIsNotANumber(&y) ) {
489	viewPoints[i] = CPTPointMake(NAN, NAN);
490	}

3) 

/ios-core-plot-library/framework/Source/CPTRangePlot.m

Uninitialized argument value

Logic error

Function call argument is an uninitialized value

-(NSUInteger)dataIndexFromInteractionPoint:(CGPoint)point

...

979
980	if ( !isnan(lastViewPoint.left) && (point.x < lastViewPoint.left) ) {
	7 ← Within the expansion of the macro 'isnan': a Function call argument is an uninitialized value
981	result = NSNotFound;
982	}
983	if ( !isnan(lastViewPoint.right) && (point.x > lastViewPoint.right) ) {
984	result = NSNotFound;
985	}
986	if ( !isnan(lastViewPoint.high) && (point.y > lastViewPoint.high) ) {
987	result = NSNotFound;
988	}
989	if ( !isnan(lastViewPoint.low) && (point.y < lastViewPoint.low) ) {
990	result = NSNotFound;
991	}

Items 2 and 3 I'm not as concerned about, however the memory leak described in item 1 seems troubling. 

I noticed several changes around static analyzer reports in the recent 1.2 release, so I was hoping that these Clang Warnings would be resolved by updating to 1.2, but after installing 1.2 we are still seeing the above issues being reported out by our Clang scan.

Have you seen these before, and can you shed some more light on what's going on with these and if it's something we should be concerned about?

Thanks.

[Jason Osborn]

We are using Clang Checker Version 269, with no additional scan-build arguments.

[Eric]

This was fixed back in January and is included in the 1.2 release.

Eric

On Monday, May 13, 2013 6:48:21 PM UTC-4, clin...@gmail.com wrote:

This the "Analyze" option -- Command-Shift-B in Xcode.  It runs the clang analyzer, which does more in-depth consistency checks on the code.  And has some false positives.

In this case, items 2 and 3 are, I'm pretty sure, false positives in clang analyzer.  It does not appear to be aware that assigning an entire structure also initializes each individual structure member.  I just tested with a newer analyzer than the Xcode built-in version (build 274) and these two no longer appear (although other new ones do).

Item 1 however still shows up with the newer version, and for good reason -- it is a real leak.  While it's true that the variable is initialized from the instance variable, the CGLayer is created inline if the instance variable is nil.  CGLayerCreateWithContext() has a retain count of 1, and assigning it to self.cachedLayer = theCachedLayer afterwards increases the retainCount to 2 after setCachedLayer retains it.  The -dealloc method will reduce it to 1, but after that it is leaked.

You need to add a couple lines at the end of that block where the layer is created:

    if ( !theCachedLayer ) {
        CGSize layerSize  = symbolSize;
        ...
        theCachedLayer   = CGLayerCreateWithContext(context, layerSize, NULL);
        ...
        self.cachedLayer = theCachedLayer;
        self.anchorPoint = symbolAnchor;
        CGLayerRelease(theCachedLayer);
        theCachedLayer = self.cachedLayer;
    }

The first will undo the retain from CGLayerCreateWithContext(), after it has been retained as the new instance variable.  The second line is necessary because the compiler (or analyzer) will complain if you use a variable after being released, even though it's really OK.  This leak showed up in the Leaks instrument as a leak of the CPTPlotSymbol itself for some reason, but fixing this fixed that problem.

[clin...@gmail.com]

Whoops, that looks to be correct, sorry about that.  I had thought we had updated to 1.2 already but it looks like something didn't quite work with that update on our end.