Bug 96320 - [glsl] C-style const arrays trigger assertion in GLSL 4.30
Summary: [glsl] C-style const arrays trigger assertion in GLSL 4.30
Status: RESOLVED MOVED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: All All
: medium normal
Assignee: Matt Turner
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-02 02:52 UTC by Ben Russell
Modified: 2019-09-18 19:45 UTC (History)
1 user (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 Ben Russell 2016-06-02 02:52:00 UTC
Overview:

When constructing a C-style array in a shader when the version is 430, this assertion happens:

glsl/ast_to_hir.cpp:2103: virtual bool ast_expression::has_sequence_subexpression() const: Assertion `!"ast_aggregate: Should never get here."' failed.

The shader compiles successfully when the version is 420.

Steps to reproduce:

1) Put any of these in a shader with a #version 430 directive, and attempt to compile the shader. If you do not hit a syntax error, these should trigger the assertion (if you set the version to 420 they will compile successfully):

const float[] EXAMPLE1 = {1.0, 2.0, 3.0};
const float[] EXAMPLE2 = {1.0, 2.0, 3.0, };
const vec3[] EXAMPLE3 = {vec3(1.0, 2.0, 3.0), vec3(4.0, 5.0, 6.0)};
const vec3[] EXAMPLE4 = {vec3(1.0, 2.0, 3.0), vec3(4.0, 5.0, 6.0), };
const vec3[] EXAMPLE5 = {{1.0, 2.0, 3.0}, {4.0, 5.0, 6.0}};
const vec3[] EXAMPLE6 = {{1.0, 2.0, 3.0}, {4.0, 5.0, 6.0}, };
const vec3[] EXAMPLE7 = {{1.0, 2.0, 3.0, }, {4.0, 5.0, 6.0, }};
const vec3[] EXAMPLE8 = {{1.0, 2.0, 3.0, }, {4.0, 5.0, 6.0, }, };

These compile without any issues on both 420 and 430 (and also earlier versions, for that matter):

const float[] NONC1 = float[](1.0, 2.0, 3.0);
const vec3[] NONC2 = vec3[](vec3(1.0, 2.0, 3.0), vec3(4.0, 5.0, 6.0));

For completeness, this one is a syntax error on 420 and 430:

const vec3[] INVALID1 = vec3[]({1.0, 2.0, 3.0}, {4.0, 5.0, 6.0});

Actual results:

Assertion, followed by an abort.

Expected results:

The shader should compile successfully.

Build Date & Hardware:

Git Revision: 98d40b4d1195ebfaa2fd9ed43755ca6896422c1a
Git Revision Date:   Wed Jun 1 09:21:01 2016 +1000
Build Date: 2016-01-06, around 15:00 NZST (+1200)
GPU: Intel HD 530 (Skylake GT2) - i965 driver
Core GL version: 4.3 (GLSL 4.30)
OS: Void Linux amd64
Comment 1 Matt Turner 2016-06-02 05:50:38 UTC
Weird but cool. I can reproduce. I noticed that if I remove the const qualifier, it compiles.
Comment 2 Dave Airlie 2016-06-06 02:38:03 UTC
I've just returned false instead of the unreachable, and it works here.

I've sent a patch to the list to address it.
Comment 3 Dave Airlie 2016-06-06 03:40:03 UTC
This should be fixed in 
https://cgit.freedesktop.org/mesa/mesa/commit/?id=4336196b7fc61166a36babdabdf5dd004ec9ee55

I'm not 100% sure the fix is perfect, but it works for me!
Comment 4 Matt Turner 2016-06-06 20:59:17 UTC
Interesting. Thanks for the heads up.

Looks like the difference in behavior between 4.20 and 4.30 is due to commit 92635a84a.

Ian, it looks like adding 'const' to a declaration and initializer like

>     float[] EXAMPLE1 = {1.0, 2.0, 3.0};

under #version 430 is enough to trigger the problem.

It seems strange to consider "1.0, 2.0, 3.0" to be a constant expression in the same vein as the example with "float a[2,3]".

So I think we're doing something wrong and returning false is papering over the problem.

I don't think comma separated lists of an aggregate initializer are intended treated as constant expressions.

I'm going to reopen for now.
Comment 5 Matt Turner 2016-06-06 21:38:48 UTC
At the same time, I don't think this needs to be tracked on the release bug.
Comment 6 Ian Romanick 2016-06-06 22:31:32 UTC
Hm... I'm trying to remember how I implemented the C-style initializers.  I *thought* it used different rules in the parser, and those rules generated different productions... much like vec4(1,2,3,4) doesn't generate an ast_sequence.  I'll have to dig in a bit.
Comment 7 GitLab Migration User 2019-09-18 19:45:50 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/811.


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.