Fri 25 Aug 2017 02:03:03 PM UTC, comment #7:
I'll work on it. I see three instances.
src/base/ftbitmap.c:229: if ( FT_QALLOC_MULT( buffer, new_pitch, bitmap->rows + ypixels ) )
src/winfonts/winfnt.c:1098: if ( FT_ALLOC_MULT( bitmap->buffer, pitch, bitmap->rows ) )
src/raster/ftrend1.c:197: if ( FT_ALLOC_MULT( bitmap->buffer, pitch, height ) )
I will also see if this one can be converted.
src/smooth/ftsmooth.c:282: if ( FT_ALLOC( bitmap->buffer, (FT_ULong)( pitch * height ) ) )
|
Fri 25 Aug 2017 07:58:35 AM UTC, comment #4:
Patrick, can you provide a patch? There are some other places in FreeType where I can see similar issues with swapped arguments to FT_ALLOC_MULT.
Alexei, I guess the main reasons for not using FT_ALLOC_MULT are that contributors either don't know that this macro exists, or they don't care and directly multiply :-)
|
Fri 25 Aug 2017 02:12:14 AM UTC, comment #3:
I kind of agree with you that it is strange: the FreeType bitmaps are stored row-by-row, yet this one takes the column-by-column view. FT_REALLOC_MULT zeros out additional "columns", luckily it is never used.
I also wonder why FT_ALLOC_MULT was not used in the smooth rasterizer or other places.
|
Thu 24 Aug 2017 07:05:16 PM UTC, comment #2:
You can close this if you want. I was debating whether I should even mention it because I expected to hear that it doesn't matter.
We're taking a hard look at memory allocations because substituting FT2 for our usual font renderer is taking 1/3 more time for a particular test file and 74% more allocation calls.
The crash was in my hacked-in temp/throw-away debug code to see what sort of allocations the FT2 code is doing. It was just an uninit variable on my part. But since my attention was already heavily focused on allocations, I paused when I saw 63 as an item size.
It made me think perhaps the argument was clobbered or uninit because other than seeing 1, 2, 4, or 8 for byte, short, pointers I expected to see nicely rounded structures allocated and 63 ain't.
So again, feel free to close it. Thanks.
|
Wed 23 Aug 2017 10:58:24 PM UTC, original submission:
This is minor, I think, but I noticed it when looking for a crash and saw a funny item_size 63 with new_count 12.
I had some debug code that was checking the arguments to realloc and it found the item_size 63.
There is a macro FT_ALLOC_MULT( ptr, count, item_size ) that is called from ft_raster1_render() like so:
pitch = ( ( width + 15 ) >> 4 ) << 1;
bitmap->pixel_mode = FT_PIXEL_MODE_MONO;
bitmap->width = width;
bitmap->rows = height;
bitmap->pitch = (int)pitch;
if ( FT_ALLOC_MULT( bitmap->buffer, pitch, height ) )
^^^^^^^^^^^^^ swapped
goto Exit;
(gdb) print pitch
$13 = 12
(gdb) print height
$14 = 63
#0 ft_mem_qrealloc (memory=0xefc14f18, item_size=63, cur_count=0, new_count=12, block=0x0, p_error=0xffffc3dc) at ../freetype2/ftutil.c:228
#1 0x082541e9 in ft_mem_realloc (memory=0xefc14f18, item_size=63, cur_count=0, new_count=12, block=0x0,
^^^^^^^^^^^^^ ^^^^^^^^^^^^
p_error=0xffffc454) at ../freetype2/ftutil.c:189
#2 0x0829367b in ft_raster1_render (render=0xefc236c8, slot=0xefcb2558, mode=FT_RENDER_MODE_MONO, origin=0x0) at ../freetype2/ftrend1.c:259
|