Re: [ft-devel] FT_Outline memory overallocation, or at least incorrect cache weighting?

[mpsuzuki]

Thank you for report. Could you tell me the coverage
of your testing (font formats, what kind of tests are
executed, etc)?

Regards,
mpsuzuki

On Tue, 9 Feb 2010 13:55:37 -0800
Paul Messmer <address@hidden> wrote:

>Testing after removing the factor of 2 isn't showing any problems for me.
>
>-- Paul
>
>On Fri, Feb 5, 2010 at 3:48 PM, Paul Messmer <address@hidden> wrote:
>
>> The overallocation is an urgent issue for me, but requires no urgent action
>> on your part.  The bar for me to make a small local fix to FreeType is lower
>> than integrating an entirely new library release.  After looking at the code
>> further, I'm less concerned about the possibility of access of more than
>> n_points FT_Vectors, and I also have some testing I can exercise to give me
>> some confidence the "fix" doesn't break anything... at least in the scope of
>> my usage of FreeType.
>>
>> I wanted to bring this up with freetype-devel in order to get a second
>> opinion about the situation, and so the problem could be fixed in your
>> releases going forward.  As there doesn't appear to be a good test suite for
>> FreeType, your exercising caution with respect to the change is a fine idea
>> as over-allocation can easily hide a latent and non-obvious bug.
>>
>> I'll keep you up to date on whether I encounter any failures after removing
>> the factor of 2 in the allocation.  Thanks.
>>
>> -- Paul
>>
>> On Thu, Feb 4, 2010 at 10:35 PM, <address@hidden> wrote:
>>
>>> Dear Paul,
>>>
>>> On Thu, 4 Feb 2010 15:29:28 -0800
>>> Paul Messmer <address@hidden> wrote:
>>> >The code above seems to believe there are n_points FT_Vectors allocated.
>>> > However,
>>> >
>>> >.../base/ftoutln.c:  FT_Outline_New_Internal()
>>> >
>>> >    if ( FT_NEW_ARRAY( anoutline->points,   numPoints * 2L ) ||
>>> >         FT_NEW_ARRAY( anoutline->tags,     numPoints      ) ||
>>> >         FT_NEW_ARRAY( anoutline->contours, numContours    ) )
>>> >      goto Fail;
>>> >    anoutline->n_points    = (FT_UShort)numPoints;
>>> >
>>> >This seems to be allocating 2*n_points FT_Vectors, so there's a
>>> difference
>>> >between how much memory is actually being used and how much it believes
>>> is
>>> >being used.
>>>
>>> Great thank you for finding the problem.
>>>
>>> Tracking the history of the ftoutln.c, I think it came
>>> from freetype-1 era (!). A "point" in TT_Outline was
>>> described by TT_Vector, like this:
>>>
>>>
>>> http://cvs.savannah.gnu.org/viewvc/freetype/lib/freetype.h?revision=1.72&root=freetype&view=markup
>>>
>>>  struct  TT_Outline_
>>>  {
>>>    TT_Short         n_contours;   /* number of contours in glyph   */
>>>    TT_UShort        n_points;     /* number of points in the glyph */
>>>
>>>    TT_Vector*       points;       /* the outline's points   */
>>>    ...
>>>
>>> But TT_New_Outline() allocated the buffer as the twice
>>> of sizeof ( TT_F26Dot6 ), like this:
>>>
>>>
>>> http://cvs.savannah.gnu.org/viewvc/freetype/lib/ttapi.c?revision=1.55&root=freetype&view=markup
>>>
>>>  FT_EXPORT_FUNC( TT_Error )
>>>  TT_New_Outline( TT_UShort    numPoints,
>>>                  TT_Short     numContours,
>>>                  TT_Outline*  outline )
>>>  {
>>>    ...
>>>
>>>    if ( ALLOC( outline->points,   numPoints*2*sizeof ( TT_F26Dot6 ) ) ||
>>>         ALLOC( outline->flags,    numPoints  *sizeof ( Byte )       ) ||
>>>         ALLOC( outline->contours, numContours*sizeof ( UShort )     ) )
>>>      goto Fail;
>>>
>>> If it were written "sizeof ( FT_Vector )", this problem might not
>>> have occured. This "2" was carried over to FreeType2, because the
>>> initial version of FreeType2 still specified its size by twice of
>>> sizeof( FT_Pos ), like this:
>>>
>>>
>>> http://cvs.savannah.gnu.org/viewvc/freetype2/src/base/ftoutln.c?revision=1.1&root=freetype&view=markup
>>>
>>>    if ( ALLOC_ARRAY( outline->points,   numPoints * 2L, FT_Pos    ) ||
>>>         ALLOC_ARRAY( outline->flags,    numPoints,      FT_Byte   ) ||
>>>         ALLOC_ARRAY( outline->contours, numContours,    FT_UShort ) )
>>>      goto Fail;
>>>
>>> Since freetype-2.1.0, FreeType2 uses FT_NEW_ARRAY() which
>>> automatically calculate the size of array element with
>>> sizeof( *first_arg ), so the factor 2 is not needed anymore,
>>> as you've pointed out. So, this problem has long life since
>>> freetype-2.1.0. The moment how overallocation can be found
>>> at:
>>>
>>>
>>> http://cvs.savannah.gnu.org/viewvc/freetype2/src/base/ftoutln.c?root=freetype&r1=1.44&r2=1.45
>>>
>>> --
>>>
>>> I want to remove this extra factor "2". But you also mentioned
>>> the possibility that the buffer over outline->n_points is used
>>> in some special case, so I hesitate to remove it without careful
>>> check. This over allocation is urgent issue for you? If not, I
>>> want to work for this issue after next official release (expected soon).
>>>
>>> Thank you again for detailed investigation and comment.
>>>
>>> Regards,
>>> mpsuzuki
>>>
>>
>>
>