Bug 5557 - Memory Leaks in arbprogparse.c
Summary: Memory Leaks in arbprogparse.c
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: x86 (IA32) Windows (All)
: low minor
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-10 01:03 UTC by Roy Walmsley
Modified: 2009-08-24 12:23 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
patch for leak 1 (1.07 KB, patch)
2006-04-28 07:04 UTC, Tilman Sauerbeck
Details | Splinter Review
patch for leak 2 (1.55 KB, patch)
2006-04-28 07:18 UTC, Tilman Sauerbeck
Details | Splinter Review

Description Roy Walmsley 2006-01-10 01:03:44 UTC
There are three areas causing memory leaks within this file, which I will take 
as a single and a double.

On line 3884 a call to grammar check is made during which memory is assigned to 
the variable "parsed". This memory is not released before the subsequent call 
to grammer_fast_check at line 3953. Thereafter the second allocation 
to "parsed" is freed at line 4035. To rectify this a copy of line 4035 should 
be inserted between the two grammar_check calls.

Subsequently while parsing the function "parse_param_use" at line 1934 
calls "mesa_strdup" to duplicate a string to be stored in the 
variable 'param_var->name'. Similarly in the function "parse_source_reg" at 
line 2463 there is a call to "mesa_strdup" to duplicate a string to be stored 
in the variable 'src->name'. Both 'param_var' and 'src' are of type 'struct 
var_cache*'. When this structure is later deleted the variable 'name' is not 
freed, possibly because it could have been allocated by the user rather than by 
mesa internally. I overcame this problem by declaring an additional variable in 
the structure definition commencing at line 519

    GLuint name_allocated;

In the function "var_cache_create" at lines 547ff I added a line to to 
initialise the new variable to 0.

    (**va).name_allocated = 0;

In the two functions referred to above I added additional lines to set the 
variable to 1 when the "_mesa_strdup" function was called.

    param_var->name_allocated = 1;

    src->name_allocated = 1;

Finally, in the function var_cache_destroy at lines 566ff I added the following 
two lines to free any allocated memory

    if ((*va)->name_allocated)
        mesa_free ((*va)->name);

With these changes the memory leaks were eliminated.
Comment 1 Brian Paul 2006-01-10 02:25:34 UTC
Could you just provide a patch with your fixes?  Thanks.
Comment 2 Tilman Sauerbeck 2006-04-28 07:04:40 UTC
Created attachment 5505 [details] [review]
patch for leak 1

This patch should fix the first of the two leaks (I don't have a test case so I
cannot say for sure).
Comment 3 Tilman Sauerbeck 2006-04-28 07:18:56 UTC
Created attachment 5506 [details] [review]
patch for leak 2

The only time the "name" property of struct var_cache is assigned a non-dummy
value is in line 644, and it's not given a newly allocated value, but it's
assigned a pointer into a chunk of memory. So we shouldn't free it, and we
don't have to strdup() the dummy values either.

I don't have a test case, though.
Comment 4 Brian Paul 2006-04-29 01:42:53 UTC
I've checked in your changes.  I also added a comment to the var_cache->name
field that it's not to be freed.
Comment 5 Tilman Sauerbeck 2006-04-29 05:21:44 UTC
The patches most likely fixed the bug; reopen if they didn't.
Comment 6 Adam Jackson 2009-08-24 12:23:39 UTC
Mass version move, cvs -> git


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.