[Mesa-dev] [PATCH 08/11] glsl: GLSL ES identifiers cannot exceed 1024 characters

[Iago Toral]

On Mon, 2015-01-19 at 19:25 -0800, Ian Romanick wrote:
> On 01/19/2015 04:41 AM, Erik Faye-Lund wrote:
> > On Mon, Jan 19, 2015 at 12:32 PM, Eduardo Lima Mitev <elima at igalia.com> wrote:
> >> From: Iago Toral Quiroga <itoral at igalia.com>
> >>
> >> Fixes the following 2 dEQP tests:
> >> dEQP-GLES3.functional.shaders.keywords.invalid_identifiers.max_length_vertex
> >> dEQP-GLES3.functional.shaders.keywords.invalid_identifiers.max_length_fragment
> >> ---
> >>  src/glsl/glsl_parser.yy | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> >> index 7fb8c38..165419c 100644
> >> --- a/src/glsl/glsl_parser.yy
> >> +++ b/src/glsl/glsl_parser.yy
> >> @@ -362,6 +362,13 @@ any_identifier:
> >>     IDENTIFIER
> >>     | TYPE_IDENTIFIER
> >>     | NEW_IDENTIFIER
> >> +   {
> >> +      if (state->es_shader && strlen($1) > 1024) {
> >> +         _mesa_glsl_error(& @1, state,
> >> +                          "Identifier `%s' exceeds "
> >> +                          "1024 characters", $1);
> >> +      }
> >> +   }
> > 
> > The limit of 1024 characters for identifiers is only specified for
> > OpenGL ES Shading Language versions 3.00 and up, not for version 1.00
> > as OpenGL ES 2.0 uses. Perhaps a spec bug?
> 
> If I had to wager a guess... I'd guess that the limitation was added
> because shipping ES2 implementations already had such a limit (or just
> had bugs with very large identifiers).  Assuming that's the case, this
> check should be safe universally.

Using huge identifiers increases memory requirements for the compiler
(we copy variables and duplicate identifiers all the time during the
compilation process), so I guess this is intended to force developers
into more ES friendly programming habits and avoid situations where the
compiler just runs out of memory in the middle of it.

> However, checking here defeats the purpose of having such a limitation.
>  It seems like we should do this in the lexer... before rallocing a copy
> of the large string.

My guess is that this requirement is more related to the compilation
process than the parser but doing this in the lexer does not harm and
can help in some cases. I'll update the patch accordingly.

Iago

> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev