Bug 54193 - output_components uninitialized in fs_visitor::emit_fb_writes()
output_components uninitialized in fs_visitor::emit_fb_writes()
Status: RESOLVED FIXED
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965
8.0
All All
: medium critical
Assigned To: Kenneth Graunke
:
: 54258 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-29 08:26 UTC by Kenneth Graunke
Modified: 2012-09-10 20:54 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Graunke 2012-08-29 08:26:25 UTC
Bug exposed by the FS precompile patch, but shouldn't be caused by it.

>> valgrind ./bin/glslparsertest tests/spec/glsl-1.00/compiler/qualifiers/fn-inout-array-allowed-cstyle.vert pass 1.00 --check-link
==2246== Memcheck, a memory error detector
==2246== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==2246== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==2246== Command: ./bin/glslparsertest tests/spec/glsl-1.00/compiler/qualifiers/fn-inout-array-allowed-cstyle.vert pass 1.00 --check-link
==2246== 
==2246== Conditional jump or move depends on uninitialised value(s)
==2246==    at 0x990A83D: fs_visitor::emit_fb_writes() (brw_fs_visitor.cpp:2188)
==2246==    by 0x98F5CF2: fs_visitor::run() (brw_fs.cpp:1992)
==2246==    by 0x98F617E: brw_wm_fs_emit (brw_fs.cpp:2093)
==2246==    by 0x98CE4E9: do_wm_prog (brw_wm.c:305)
==2246==    by 0x98F674A: brw_fs_precompile(gl_context*, gl_shader_program*) (brw_fs.cpp:2207)
==2246==    by 0x991029D: brw_shader_precompile(gl_context*, gl_shader_program*) (brw_shader.cpp:70)
==2246==    by 0x991089E: brw_link_shader (brw_shader.cpp:215)
==2246==    by 0x9E616DC: _mesa_glsl_link_shader (ir_to_mesa.cpp:3159)
==2246==    by 0x9CD29BE: link_program (shaderapi.c:773)
==2246==    by 0x9CD384B: _mesa_LinkProgramARB (shaderapi.c:1281)
==2246==    by 0x50C312F: stub_glLinkProgram (generated_dispatch.c:13971)
==2246==    by 0x401862: test (glslparsertest.c:187)

This causes tests to chew through a lot of time, since it's looping for some non-deterministic amount of time.
Comment 1 Kenneth Graunke 2012-08-29 09:32:04 UTC
Wow, this totally broke Piglit something awful (at least with precompiles on) :(

Patch on mailing list:
http://lists.freedesktop.org/archives/mesa-dev/2012-August/026488.html
Comment 2 Kenneth Graunke 2012-08-30 02:10:33 UTC
Fixed in master by:

commit 6928bea7ca1f2ed308d8255c6816f44467306255
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Wed Aug 29 01:51:17 2012 -0700

    i965/fs: Initialize output_components[] by filling it with zeros.
    
    Prior to commit 2f1869822, emit_fb_writes() looped from 0 to 3, writing
    all four components of a vec4 color output.  However, that broke for
    smaller output types (float, vec2, or vec3).  To fix that, I introduced
    a new variable (output_components[]) containing the size of the output
    type for each render target.
    
    Unfortunately, I forgot to actually initialize it in the constructor,
    which meant that unless a shader wrote to gl_FragColor, or the specific
    output for each render target, output_components would contain a garbage
    value, and we'd loop for a completely non-deterministic amount of time.
    
    Not actually emitting any color writes seems like the right approach.
    We may still need to emit a render target write (to terminate the
    thread), but don't have to put in any sensible values (the shader didn't
    write anything, after all).
    
    Fixes a regression since 2f18698220d8b27991fab550c4721590d17278e0.
    NOTE: This is a candidate for stable release branches.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54193
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Paul Berry <stereotype441@gmail.com>
    Tested-by: Ian Romanick <idr@freedesktop.org>

Changing version to 8.0 - this needs to get cherry-picked, but I want to let it sit on master for a little while first to make sure it doesn't break things even worse.
Comment 3 Ian Romanick 2012-08-30 17:11:12 UTC
*** Bug 54258 has been marked as a duplicate of this bug. ***
Comment 4 Kenneth Graunke 2012-09-10 20:54:26 UTC
Backported to the 8.0 branch as commit 77e711cfca1a1e8908be7e4a744b2306094c65e0.