Bug 66779

Summary: Use of uninitialized stack variable with brw_search_cache()
Product: Mesa Reporter: Damien Lespiau <damien.lespiau>
Component: Drivers/DRI/i965Assignee: Paul Berry <stereotype441>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 65654, 67224    

Description Damien Lespiau 2013-07-10 14:44:07 UTC 
I've been running a GL program of mine under valgrind (mesa master) and it did catch something that looks interesting:

==2936== Conditional jump or move depends on uninitialised value(s)
==2936==    at 0x62A8CBB: brw_search_cache (brw_state_cache.c:159)
==2936==    by 0x625B324: brw_blorp_const_color_params::get_wm_prog(brw_context*, brw_blorp_prog_data**) const (brw_blorp_clear.cpp:334)
==2936==    by 0x62DC2AC: gen7_blorp_exec(intel_context*, brw_blorp_params const*) (gen7_blorp.cpp:858)
==2936==    by 0x62551F4: brw_blorp_exec(intel_context*, brw_blorp_params const*) (brw_blorp.cpp:201)
==2936==    by 0x625B609: brw_blorp_resolve_color (brw_blorp_clear.cpp:523)
==2936==    by 0x62494C4: intel_resolve_for_dri2_flush (intel_context.c:122)
==2936==    by 0x62511EE: intelDRI2Flush (intel_screen.c:163)
==2936==    by 0x558B09F: dri2Flush.constprop.5 (dri2_glx.c:560)
==2936==    by 0x558B3F4: dri2SwapBuffers (dri2_glx.c:840)
[...]
==2936==  Uninitialised value was created by a stack allocation
==2936==    at 0x625B320: brw_blorp_const_color_params::get_wm_prog(brw_context*, brw_blorp_prog_data**) const (brw_blorp_clear.cpp:334)

And indeed, the inout_offset parameter of brw_search_cache() is used uninitialized in brw_blorp_const_color_params::get_wm_prog(). A quick git grep reveals that also the case in:

src/mesa/drivers/dri/i965/brw_blorp_blit.cpp:2284:brw_blorp_blit_params::get_wm_prog()
Comment 1 Paul Berry 2013-07-24 16:23:52 UTC 
This is a benign problem since the only effect of not initializing inout_offset prior to calling brw_search_cache() is that the bit corresponding to cache_id in brw->state.dirty.cache may not be set reliably.  This is ok, since the cache_id's used by brw_blorp_const_color_params::get_wm_prog() and brw_blorp_blit_params::get_wm_prog() (BRW_BLORP_CONST_COLOR_PROG and BRW_BLORP_BLIT_PROG, respectively) correspond to dirty bits that are not used.

I'll go ahead and make a fix to master to reduce valgrind noise, but I don't think it's worth cherry-picking the fix to the 9.2 branch, since there's no user-visible bug.
Comment 2 Paul Berry 2013-07-24 16:35:49 UTC 
Patch sent to mesa-dev list for review: http://lists.freedesktop.org/archives/mesa-dev/2013-July/042269.html
Comment 3 Paul Berry 2013-07-25 16:42:40 UTC 
Fixed by commit b8f13fbb856534cbc1345325b74ec47711493dd6
Author: Paul Berry <stereotype441@gmail.com>
Date:   Wed Jul 24 09:24:51 2013 -0700

    i965: Initialize inout_offset parameter to brw_search_cache().
    
    Two callers of brw_search_cache() weren't initializing that function's
    inout_offset parameter: brw_blorp_const_color_params::get_wm_prog()
    and brw_blorp_const_color_params::get_wm_prog().
    
    That's a benign problem, since the only effect of not initializing
    inout_offset prior to calling brw_search_cache() is that the bit
    corresponding to cache_id in brw->state.dirty.cache may not be set
    reliably.  This is ok, since the cache_id's used by
    brw_blorp_const_color_params::get_wm_prog() and
    brw_blorp_blit_params::get_wm_prog() (BRW_BLORP_CONST_COLOR_PROG and
    BRW_BLORP_BLIT_PROG, respectively) correspond to dirty bits that are
    not used.
    
    However, failing to initialize this parameter causes valgrind to
    complain.  So let's go ahead and fix it to reduce valgrind noise.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66779
    
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

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.