clang static analysis

[Jeremy Huddleston]

The analyzer is correct.  It sees the call to miFillPolyHelper on line 1849 and assumes that pGC can change:

miFillPolyHelper (pDrawable, pGC, pixel, spanData, y, h, left, right, nleft, nright);

because it is not const:

static void
miFillPolyHelper (DrawablePtr pDrawable, GCPtr pGC, unsigned long pixel,
                  SpanDataPtr spanData, int y, int overall_height,
                  PolyEdgePtr left, PolyEdgePtr right,
                  int left_count, int right_count)

My guess is that applying "const" correctly in many places will help the SA avoid false positives like this.


On May 21, 2010, at 10:32, Jamey Sharp wrote:

> On Thu, May 20, 2010 at 11:59 PM, Jeremy Huddleston <jeremyhu at apple.com> wrote:
>> In case anyone's interested, I've posted an updated log from building the XQuartz server with clang SA:
>> 
>> http://people.freedesktop.org/~jeremyhu/clang/2010-05-20-1/
> 
> The first one I examined ("Assigned value is garbage or undefined",
> mi/miwideline.c line 1918) assumes in steps 10 and 11 that
> pGC->lineStyle == LineOnOffDash and pGC->capStyle == CapProjecting,
> but took the false branch for that condition at step 8. Is it often
> that wrong? (It makes the same mistake for line 1804, and the "Branch
> condition evaluates to a garbage value" error is based on a similarly
> contradictory assumption.) A tiny bit of abstract interpretation would
> go a long way here...
> 
> "Assigned value is garbage or undefined" at dix/dixfonts.c line 1231
> is correct, at least, except the real bug is that I didn't delete the
> oldfid variable when I deleted the code that cared what value it had.
> Patch for that shortly.
> 
> I suspect quite a few dead-store warnings would go away if we kill the
> REGION_* macros. One particularly entertaining example, from
> http://people.freedesktop.org/~jeremyhu/clang/2010-05-20-1/report-7FMXPb.html
> 
>    /* This prevents warnings about pScreen not being used. */
>    pGC->pScreen = pScreen = pGC->pScreen;
> 	
> clang says "Although the value stored to 'pScreen' is used in the
> enclosing expression, the value is never actually read from
> 'pScreen'".
> 
> Jamey