Closed Bug 1117174 Opened 9 years ago Closed 9 years ago

FreeType is riddled with uninitialized |error| variables

Categories

(Core :: Graphics: Text, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: n.nethercote, Unassigned)

References

Details

I ran cppcheck (see bug 679417) on Firefox and got lots of complaints about
FreeType:

> modules/freetype2/src/base/ftgloadr.c:82: error: Uninitialized variable: error
> modules/freetype2/src/base/ftgloadr.c:180: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:207: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:333: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:1197: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:2452: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:4632: error: Uninitialized variable: error
> modules/freetype2/src/base/ftrfork.c:539: error: Uninitialized variable: error
> modules/freetype2/src/base/ftrfork.c:575: error: Uninitialized variable: error
> modules/freetype2/src/base/ftrfork.c:727: error: Uninitialized variable: error
> modules/freetype2/src/base/ftrfork.c:732: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:1363: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:1461: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:1466: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:1468: error: Uninitialized variable: error
> modules/freetype2/src/base/ftobjs.c:1477: error: Uninitialized variable: error
> modules/freetype2/src/base/ftglyph.c:301: error: Uninitialized variable: error
> modules/freetype2/src/base/ftinit.c:171: error: Uninitialized variable: error
> modules/freetype2/src/base/ftinit.c:183: error: Uninitialized variable: error
> modules/freetype2/src/base/ftstroke.c:812: error: Uninitialized variable: error
> modules/freetype2/src/bzip2/ftbzip2.c:241: error: Uninitialized variable: error
> modules/freetype2/src/cache/ftcmanag.c:405: error: Uninitialized variable: error
> modules/freetype2/src/cache/ftccache.c:346: error: Uninitialized variable: error
> modules/freetype2/src/cache/ftccmap.c:136: error: Uninitialized variable: error
> modules/freetype2/src/cache/ftcimage.c:89: error: Uninitialized variable: error
> modules/freetype2/src/cache/ftcsbits.c:60: error: Uninitialized variable: error
> modules/freetype2/src/cff/cffload.c:220: error: Uninitialized variable: error
> modules/freetype2/src/cff/cffload.c:293: error: Uninitialized variable: error
> modules/freetype2/src/cff/cffparse.c:813: error: Uninitialized variable: error
> modules/freetype2/src/cid/cidparse.c:81: error: Uninitialized variable: error
> modules/freetype2/src/gzip/ftgzip.c:268: error: Uninitialized variable: error
> modules/freetype2/src/gzip/ftgzip.c:364: error: Uninitialized variable: error
> modules/freetype2/src/lzw/ftlzw.c:104: error: Uninitialized variable: error
> modules/freetype2/src/lzw/ftlzw.c:166: error: Uninitialized variable: error
> modules/freetype2/src/pfr/pfrload.c:168: error: Uninitialized variable: error
> modules/freetype2/src/pfr/pfrload.c:216: error: Uninitialized variable: error
> modules/freetype2/src/pfr/pfrload.c:328: error: Uninitialized variable: error
> modules/freetype2/src/psaux/afmparse.c:538: error: Uninitialized variable: error
> modules/freetype2/src/psaux/afmparse.c:605: error: Uninitialized variable: error
> modules/freetype2/src/psaux/afmparse.c:703: error: Uninitialized variable: error
> modules/freetype2/src/psaux/psobjs.c:91: error: Uninitialized variable: error
> modules/freetype2/src/psaux/psobjs.c:128: error: Uninitialized variable: error
> modules/freetype2/src/pshinter/pshalgo.c:241: error: Uninitialized variable: error
> modules/freetype2/src/pshinter/pshglob.c:749: error: Uninitialized variable: error
> modules/freetype2/src/psnames/psmodule.c:372: error: Uninitialized variable: error
> modules/freetype2/src/psnames/psmodule.c:393: error: Uninitialized variable: error
> modules/freetype2/src/raster/ftraster.c:3498: error: Uninitialized variable: error
> modules/freetype2/src/sfnt/ttload.c:148: error: Uninitialized variable: error
> modules/freetype2/src/sfnt/ttload.c:509: error: Uninitialized variable: error
> modules/freetype2/src/sfnt/sfobjs.c:755: error: Uninitialized variable: error
> modules/freetype2/src/sfnt/sfobjs.c:762: error: Uninitialized variable: error
> modules/freetype2/src/sfnt/ttcmap.c:3432: error: Uninitialized variable: error
> modules/freetype2/src/smooth/ftgrays.c:2072: error: Uninitialized variable: error
> modules/freetype2/src/truetype/ttgload.c:146: error: Uninitialized variable: error
> modules/freetype2/src/truetype/ttgload.c:295: error: Uninitialized variable: error
> modules/freetype2/src/truetype/ttobjs.c:141: error: Uninitialized variable: error
> modules/freetype2/src/truetype/ttinterp.c:538: error: Uninitialized variable: error

As far as I can tell these are true positives. For example, from ftgrays.c:

> static int
> gray_raster_new( FT_Memory   memory,
>                  FT_Raster*  araster )
> {
>   FT_Error      error;
>   gray_PRaster  raster = NULL;
> 
>   *araster = 0;
>   if ( !FT_ALLOC( raster, sizeof ( gray_TRaster ) ) )
>   {
>     raster->memory = memory;
>     *araster       = (FT_Raster)raster;
>   }
> 
>   return error;
> }

FT_Error is just an int. It really looks like the author thinks that stack
variables are auto-zeroed. I haven't looked at all the listed cases, but for
the the ones I have cppcheck seems to be correct. Not all of them are as
obviously wrong as this one -- many of them set |error| on some paths but not
others, and then use it.

But maybe there's something subtle going on that I'm missing? Because this
seems like something that some person or test or tool should have caught a long
time ago.
Oh, I missed the last three when pasting:

> modules/freetype2/src/type1/t1load.c:1204: error: Uninitialized variable: error
> modules/freetype2/src/type1/t1parse.c:93: error: Uninitialized variable: error
> modules/freetype2/src/type42/t42parse.c:359: error: Uninitialized variable: error
(In reply to Nicholas Nethercote [:njn] from comment #0)

> But maybe there's something subtle going on that I'm missing?

To take your example from ftgrays.c:

> > static int
> > gray_raster_new( FT_Memory   memory,
> >                  FT_Raster*  araster )
> > {
> >   FT_Error      error;
> >   gray_PRaster  raster = NULL;
> > 
> >   *araster = 0;
> >   if ( !FT_ALLOC( raster, sizeof ( gray_TRaster ) ) )
> >   {
> >     raster->memory = memory;
> >     *araster       = (FT_Raster)raster;
> >   }
> > 
> >   return error;
> > }

Note that FT_ALLOC here is a macro (you'll find it #defined in modules/freetype2/include/internal/ftmemory.h). This will eventually (perhaps via several layers of macros) pass the |error| variable as an outparam to the allocation function that it calls, which will set it to zero (OK) or an error code. So by the time it is returned from here, it will have been set appropriately.

You'll find lots of the same pattern throughout FreeType: macros that assume the existence of an |error| variable in the caller's scope, and pass its address to the underlying functions they're calling. I suspect that explains these complaints, and that they're in fact false positives because the tool is not understanding the (layers of) macros involved.
Gotcha. Macros that implicitly use local variables, sigh...

Thank you for the quick response.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
Group: core-security
You need to log in before you can comment on or make changes to this bug.