Bug 54193 - output_components uninitialized in fs_visitor::emit_fb_writes()
Summary: output_components uninitialized in fs_visitor::emit_fb_writes()
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: 8.0
Hardware: All All
: medium critical
Assignee: Kenneth Graunke
QA Contact:
URL:
Whiteboard:
Keywords:
: 54258 (view as bug list)
Depends on:
Blocks:
 
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.


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.