Bug 82585 - geometry shader with optional out variable segfaults
Summary: geometry shader with optional out variable segfaults
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Ian Romanick
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-13 21:49 UTC by pavol
Modified: 2014-12-05 21:42 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Demo of bug (1.91 KB, application/gzip)
2014-08-26 08:25 UTC, pavol
Details
Valgrind output (4.90 KB, text/plain)
2014-08-26 09:02 UTC, pavol
Details
GLSL dump and backtrace (4.72 KB, text/plain)
2014-08-26 09:05 UTC, Michel Dänzer
Details
Geometry shader input Piglit test (5.55 KB, patch)
2014-11-21 20:41 UTC, pavol
Details | Splinter Review
Typos patch (1.98 KB, patch)
2014-11-21 20:42 UTC, pavol
Details | Splinter Review
Patch that may fix the problem (1.49 KB, patch)
2014-12-01 22:08 UTC, Ian Romanick
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description pavol 2014-08-13 21:49:02 UTC
Running this shader in geometry stage, it segfaults:

#version 330

layout(points) in;
layout(points, max_vertices = 1) out;

in gl_PerVertex {
    vec4 gl_Position;
    float gl_PointSize;
    float gl_ClipDistance[];
} gl_in[];

in vec4 v_colour[];
out vec4 g_colour;

void main() {
  for(int i = 0; i < 1; i++) {
    gl_Position = gl_in[i].gl_Position;
    gl_PrimitiveID = gl_PrimitiveIDIn;
    /* TODO report bug */
    g_colour = v_colour[i];
    EmitVertex();
  }
  EndPrimitive();
}


Following message is from Valgrind:
EE ../../../../../../src/gallium/drivers/r600/r600_shader.c:353 tgsi_is_supported - unsupported src 0 (dimension 1)
EE ../../../../../../src/gallium/drivers/r600/r600_shader.c:157 r600_pipe_shader_create - translation from TGSI failed !
EE ../../../../../../src/gallium/drivers/r600/r600_state_common.c:750 r600_shader_select - Failed to build shader variant (type=2) -22
Comment 1 Michel Dänzer 2014-08-26 03:32:59 UTC
Please provide more details about how to reproduce the problem. Ideal would be if you could attach a simple test program which we can compile and run, or at least a command line to reproduce the problem with a standard tool and the geometry shader source code you provided.
Comment 2 pavol 2014-08-26 08:25:54 UTC
Created attachment 105265 [details]
Demo of bug
Comment 3 pavol 2014-08-26 08:28:23 UTC
Program should be valid according GL spec.
Comment 4 Michel Dänzer 2014-08-26 08:58:01 UTC
I'm hitting this assertion failure in the GLSL IR to TGSI translation code first:

../../../src/mesa/state_tracker/st_glsl_to_tgsi.cpp:2157:visit: Assertion `var->data.location != -1' failed.
Comment 5 pavol 2014-08-26 09:02:38 UTC
Created attachment 105268 [details]
Valgrind output

This is output from valgrind, mesa is from here https://launchpad.net/~oibaf/+archive/ubuntu/graphics-drivers
Comment 6 Michel Dänzer 2014-08-26 09:05:00 UTC
Created attachment 105270 [details]
GLSL dump and backtrace
Comment 7 Tapani Pälli 2014-10-20 07:11:55 UTC
With Intel we get following backtrace .. I don't have good enough clue to tell what's wrong but I can see that the reg and reg_offset values clearly go over the size of attribute_map.

--- 8< -----
(gdb) 
#0  0x00007ffff73683e2 in brw::vec4_visitor::lower_attributes_to_hw_regs (this=0xffff, this@entry=0x7fffffffcdf0, attribute_map=attribute_map@entry=0x7fffffffc770, 
    interleaved=false) at brw_vec4.cpp:1501
#1  0x00007ffff736f6a0 in brw::vec4_gs_visitor::setup_payload (this=0x7fffffffcdf0) at brw_vec4_gs_visitor.cpp:134
#2  0x00007ffff73691ec in brw::vec4_visitor::run (this=this@entry=0x7fffffffcdf0) at brw_vec4.cpp:1813
#3  0x00007ffff737077f in brw::brw_gs_emit (brw=brw@entry=0x7ffff6daf040, prog=prog@entry=0xe0e070, c=c@entry=0x7fffffffda50, mem_ctx=mem_ctx@entry=0xae59d0, 
    final_assembly_size=final_assembly_size@entry=0x7fffffffda4c) at brw_vec4_gs_visitor.cpp:648
#4  0x00007ffff736e7b9 in do_gs_prog (brw=brw@entry=0x7ffff6daf040, prog=prog@entry=0xe0e070, gp=gp@entry=0xdab920, key=key@entry=0x7fffffffdd30) at brw_vec4_gs.c:264
#5  0x00007ffff736ebb8 in brw_gs_precompile (ctx=0x7ffff6daf040, prog=0xe0e070) at brw_vec4_gs.c:401
#6  0x00007ffff73607ff in brw_shader_precompile (prog=0xe0e070, ctx=0x7ffff6daf040) at brw_shader.cpp:76
#7  brw_link_shader (ctx=0x7ffff6daf040, shProg=0xe0e070) at brw_shader.cpp:269
#8  0x00007ffff7213a26 in _mesa_glsl_link_shader (ctx=0x7ffff6daf040, prog=0xe0e070) at ../../src/mesa/program/ir_to_mesa.cpp:3038
Comment 8 Kenneth Graunke 2014-11-21 12:03:29 UTC
This is a separate shader objects bug, and is probably related to 79783.  If you modify the demo application to not use SSO, it works fine.
Comment 9 pavol 2014-11-21 20:41:08 UTC
I have attached two patches, one adds piglit test and another fixes some minor typos.
Comment 10 pavol 2014-11-21 20:41:40 UTC
Created attachment 109813 [details] [review]
Geometry shader input Piglit test
Comment 11 pavol 2014-11-21 20:42:23 UTC
Created attachment 109814 [details] [review]
Typos patch
Comment 12 Ian Romanick 2014-12-01 22:08:30 UTC
Created attachment 110323 [details] [review]
Patch that may fix the problem
Comment 14 Ian Romanick 2014-12-03 19:52:33 UTC
Fixed in master by the following commits.  It now occurs to me that I should have tagged these for stable branches. :(

commit a909b995d95892798a189818454905fdefd4bc9b
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Mon Dec 1 14:07:30 2014 -0800

    linker: Assign varying locations geometry shader inputs for SSO
    
    Previously only geometry shader outputs would be assigned locations if
    the geometry shader was the only stage in the linked program.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Cc: pavol@klacansky.com
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82585
    Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

commit 5eca78a00a5de442aaf541a1095d5cfa6b4f45de
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Mon Dec 1 14:16:24 2014 -0800

    linker: Wrap access of producer_var with a NULL check
    
    producer_var could be NULL if consumer_var is not NULL and
    consumer_is_fs is false.  This will occur when the producer is NULL and
    the consumer is the geometry shader for a program that contains only a
    geometry shader.  This will occur starting with the next patch.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Cc: pavol@klacansky.com
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82585
    Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Comment 15 pavol 2014-12-05 21:05:26 UTC
Thank you Ian. Where should I submit the piglit patch please?
Comment 16 Ian Romanick 2014-12-05 21:42:27 UTC
(In reply to pavol from comment #15)
> Thank you Ian. Where should I submit the piglit patch please?

The piglit mailing list:  piglit@lists.freedesktop.org.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.