Bug 57753 - leak in loop_analysis
leak in loop_analysis
Status: RESOLVED FIXED
Product: Mesa
Classification: Unclassified
Component: glsl-compiler
git
Other All
: medium normal
Assigned To: Ian Romanick
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-01 01:19 UTC by Dave Airlie
Modified: 2013-02-19 20:46 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Airlie 2012-12-01 01:19:39 UTC
Coverity pointed out
src/glsl/loop_analysis.cpp

loop_analysis::loop_analysis()
{
   this->loops = new loop_state;


This loops is never freed anywhere, probably need a destructor.
Comment 1 Matt Turner 2013-01-30 01:50:12 UTC
Can't just delete loops in the destructor because analyze_loop_variables() returns the field:

loop_state *
analyze_loop_variables(exec_list *instructions)
{
   loop_analysis v;

   v.run(instructions);
   return v.loops;
}

analyze_loop_variables() is only called from one place, and the return value is freed afterward.

glsl_parser_extras.cpp:

loop_state *ls = analyze_loop_variables(ir);
if (ls->loop_found) {
   progress = set_loop_controls(ir, ls) || progress;
   progress = unroll_loops(ir, ls, max_unroll_iterations) || progress;
}
delete ls;

So I think Coverity is mistaken about an admittedly odd case.
Comment 2 Ian Romanick 2013-01-30 06:12:34 UTC
I have a patch that (should) make it not look like a leak.  I'll send it out tomorrow.
Comment 4 Ian Romanick 2013-02-19 20:46:51 UTC
Fixed on master by the following commit.  I don't think this is worth cherry-picking back to stable branches.

commit 82691f12931a022560f8054c8c8e240cd6b2fff4
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Tue Jul 10 16:26:33 2012 -0700

    glsl: Change loop_analysis to not look like a resource leak
    
    Previously the loop_state was allocated in the loop_analysis
    constructor, but not freed in the (nonexistent) destructor.  Moving
    the allocation of the loop_state makes this code appear less sketchy.
    
    Either way, there is no actual leak.  The loop_state is freed by the
    single caller of analyze_loop_variables.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Cc: Dave Airlie <airlied@freedesktop.org>
    Reviewed-by: Matt Turner <mattst88@gmail.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=57753