[Mesa-dev] [PATCH 2/2] glsl: free or reuse memory allocated for TF varying

[Tapani Pälli]

Hi;

On 11/23/18 10:29 AM, Gert Wollny wrote:
> Hello Tapani,
> 
> since you wrote the original code, could you have a look at this patch?
>

Sure. I was able to reproduce this leak with Piglit's 
"ext_transform_feedback-points" and fix LGTM;

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>


> Many thanks,
> Gert
> 
> Am Freitag, den 16.11.2018, 19:12 +0100 schrieb Gert Wollny:
>> From: Gert Wollny <gert.wollny at collabora.com>
>>
>> When a shader program is de-serialized the gl_shader_program passed
>> in
>> may actually still hold memory allocations for the transform feedback
>> varyings. If that is the case, free the varying names and reallocate
>> the new storage for the names array.
>>
>> This fixes a memory leak:
>> Direct leak of 48 byte(s) in 6 object(s) allocated from:
>>   in malloc (/usr/lib64/gcc/x86_64-pc-linux-
>> gnu/7.3.0/libasan.so+0xdb880)
>>   in transform_feedback_varyings
>> ../../samba/mesa/src/mesa/main/transformfeedback.c:875
>>   in _mesa_TransformFeedbackVaryings
>> ../../samba/mesa/src/mesa/main/transformfeedback.c:985
>>   ...
>> Indirect leak of 42 byte(s) in 6 object(s) allocated from:
>>    in __interceptor_strdup (/usr/lib64/gcc/x86_64-pc-linux-
>> gnu/7.3.0/libasan.so+0x761c8)
>>    in transform_feedback_varyings
>> ../../samba/mesa/src/mesa/main/transformfeedback.c:887
>>    in _mesa_TransformFeedbackVaryings
>> ../../samba/mesa/src/mesa/main/transformfeedback.c:985
>>
>> Fixes: ab2643e4b06f63c93a57624003679903442634a8
>>     glsl: serialize data from glTransformFeedbackVaryings
>>
>> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
>> ---
>>   src/compiler/glsl/serialize.cpp | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/compiler/glsl/serialize.cpp
>> b/src/compiler/glsl/serialize.cpp
>> index 267700e7e7..26d8ec4b75 100644
>> --- a/src/compiler/glsl/serialize.cpp
>> +++ b/src/compiler/glsl/serialize.cpp
>> @@ -360,13 +360,20 @@ read_xfb(struct blob_reader *metadata, struct
>> gl_shader_program *shProg)
>>      if (xfb_stage == ~0u)
>>         return;
>>   
>> +   if (shProg->TransformFeedback.VaryingNames)  {
>> +      for (unsigned i = 0; i < shProg->TransformFeedback.NumVarying;
>> ++i)
>> +         free(shProg->TransformFeedback.VaryingNames[i]);
>> +   }
>> +
>>      /* Data set by glTransformFeedbackVaryings. */
>>      shProg->TransformFeedback.BufferMode =
>> blob_read_uint32(metadata);
>>      blob_copy_bytes(metadata, &shProg-
>>> TransformFeedback.BufferStride,
>>                      sizeof(shProg->TransformFeedback.BufferStride));
>>      shProg->TransformFeedback.NumVarying =
>> blob_read_uint32(metadata);
>> +
>>      shProg->TransformFeedback.VaryingNames = (char **)
>> -      malloc(shProg->TransformFeedback.NumVarying * sizeof(GLchar
>> *));
>> +      realloc(shProg->TransformFeedback.VaryingNames,
>> +             shProg->TransformFeedback.NumVarying * sizeof(GLchar
>> *));
>>      /* Note, malloc used with VaryingNames. */
>>      for (unsigned i = 0; i < shProg->TransformFeedback.NumVarying;
>> i++)
>>         shProg->TransformFeedback.VaryingNames[i] =