Closed
Bug 1117174
Opened 9 years ago
Closed 9 years ago
FreeType is riddled with uninitialized |error| variables
Categories
(Core :: Graphics: Text, defect)
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.
Reporter | ||
Comment 1•9 years 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
Comment 2•9 years ago
|
||
(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.
Reporter | ||
Comment 3•9 years ago
|
||
Gotcha. Macros that implicitly use local variables, sigh... Thank you for the quick response.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
Updated•9 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•