Bug 106823 - Failed to recongnize keyword of shader code
Summary: Failed to recongnize keyword of shader code
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: 17.1
Hardware: ARM Linux (All)
: medium normal
Assignee: mesa-dev
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-05 07:32 UTC by Zhaowei Yuan
Modified: 2018-06-06 08:12 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Zhaowei Yuan 2018-06-05 07:32:04 UTC
CTS test case 
"dEQP-GLES2.functional.shaders.keywords.reserved_keywords.double_vertex" checks if GLSL compiler can detect the keyword error within follow vertex shader code:
"
precision mediump float;
attribute highp vec4 dEQP_Position;
void main()
{
	float double = 1.0;
	gl_Position = dEQP_Position;	
}
"

Taking a keyword as a variable's name should be forbidden, but MESA doesn't report any error
Comment 1 Kenneth Graunke 2018-06-05 07:59:38 UTC
That is indeed a bug, GLSL ES 1.0.17 specifies that "double" is a keyword reserved for future use.  The fix is to change glsl_lexer.ll to be

double          TYPE_WITH_ALT(130, 100, 130, 300, yyextra->ARB_gpu_shader_fp64_enable, glsl_type::double_type)

instead of 130, 300, 130, 300.  Want to send a patch or should I?
Comment 2 Zhaowei Yuan 2018-06-05 08:30:32 UTC
(In reply to Kenneth Graunke from comment #1)
> That is indeed a bug, GLSL ES 1.0.17 specifies that "double" is a keyword
> reserved for future use.  The fix is to change glsl_lexer.ll to be
> 
> double          TYPE_WITH_ALT(130, 100, 130, 300,
> yyextra->ARB_gpu_shader_fp64_enable, glsl_type::double_type)
> 
> instead of 130, 300, 130, 300.  Want to send a patch or should I?

Thanks, I've verified it and I'll send a patch later
Comment 3 Mark Janes 2018-06-05 17:01:23 UTC
Mesa passes the GLES3 variant of this test.  Also, the test is listed in the following dEQP source file:

  android/cts/master/src/gles2-failures.txt

Are we sure that this test should pass on GLES2?
Comment 4 Kenneth Graunke 2018-06-05 20:31:21 UTC
(In reply to Mark Janes from comment #3)
> Mesa passes the GLES3 variant of this test.  Also, the test is listed in the
> following dEQP source file:
> 
>   android/cts/master/src/gles2-failures.txt
> 
> Are we sure that this test should pass on GLES2?

Yes.  That just means the test isn't required as part of the Android conformance process (i.e. on the must-pass list).  The spec is pretty clear.
Comment 5 Zhaowei Yuan 2018-06-06 00:59:04 UTC
patch has been posted here:
https://patchwork.freedesktop.org/patch/227593/
please review it
Comment 6 Kenneth Graunke 2018-06-06 08:12:15 UTC
Fixed by:

commit 67f7a16b598513d25319e482359a4c4c6fc1271d
Author: zhaowei yuan <zhaowei.yuan@samsung.com>
Date:   Tue Jun 5 05:33:59 2018 +0800

    glsl: Take 'double' as reserved after GLSL ES 1.0
    
    GLSL ES 1.0.17 specifies that "double" is a keyword reserved
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106823
    Signed-off-by: zhaowei yuan <zhaowei.yuan@samsung.com>
    Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
    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.