283077 - Arithmetic overflow warning in modules/libimg/png/pngrtran.c

Status: RESOLVED INVALID

(Core :: Graphics: ImageLib, defect)

Description

[David Gardiner]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

Running PREfast static code analysis gives the following warning:

pngrtran.c(4029) : warning 297: Arithmetic overflow: 32-bit value is shifted,
then cast to 64-bit value. Cast to a 64-bit value before the shift. Bits may be
lost.
problem occurs in function 'MOZ_PNG_build_gamma_tab'

The line in question is:

 max = (png_uint_32)(fin * (double)((png_uint_32)num << 8));

Not sure if the warning is legitimate or not.

-dave

Reproducible: Always

Steps to Reproduce:

Comment 1

[Brendan Eich [:brendan]]

This is invalid, as is clear from the data flow: num (type int) is set at
http://lxr.mozilla.org/mozilla/source/modules/libimg/png/pngrtran.c#4002 as follows:

    num = (1 << (8 - shift));

where shift is constrained to [0, 7].  So num << 8 does not overflow 32-bits (it
fits in 17 bits worst case).  If PREFast is not modeling data flow well enough
to see this, we shouldn't be filing bugs based on it without a lot more
inspection.  We're using Coverity's SWAT tools to better effect already.

BTW, the (num << 8) expression is loop-invariant at the line fingered by PREFast
so it could be hoisted manually, but perhaps our tier-1 compilers are smart
enough to do that nowadays.

/be