Bug 43564

Created attachment 54169 [details]
Piglit case to verify the special character '%>'

Patch sent to mesa-dev for review:
Subject: [PATCH] glsl: Fix crashes caused by Bison error messages involving "'%'".
Message-Id: <1323338449-8520-1-git-send-email-kenneth@whitecape.org>

Also, for what it's worth, this piglit test could be written as a glslparsertest, rather than a full fledged C program.  It would be much simpler that way.

Created attachment 54261 [details]
a glslparsertest case

(In reply to comment #2)
> Patch sent to mesa-dev for review:
> Subject: [PATCH] glsl: Fix crashes caused by Bison error messages involving
> "'%'".
> Message-Id: <1323338449-8520-1-git-send-email-kenneth@whitecape.org>
> 

The patch works well.

> Also, for what it's worth, this piglit test could be written as a
> glslparsertest, rather than a full fledged C program.  It would be much simpler
> that way.

I rewrote the case as a glslparsertest one, hope it better.

(In reply to comment #2)
> Patch sent to mesa-dev for review:
> Subject: [PATCH] glsl: Fix crashes caused by Bison error messages involving
> "'%'".
> Message-Id: <1323338449-8520-1-git-send-email-kenneth@whitecape.org>
> 
> Also, for what it's worth, this piglit test could be written as a
> glslparsertest, rather than a full fledged C program.  It would be much simpler
> that way.

Oh, I just noticed you had updated the test cases.

commit c87cb98bb4e893e04831bf68231f5ed42e0b5b6f
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Thu Dec 8 01:35:48 2011 -0800

    glsl: Fix crashes caused by Bison error messages involving "'%'".
    
    Invalid shaders containing the character % at an unexpected location
    would cause Bison to call yyerror with a message of:
    
        syntax error, unexpected '%'
    
    Bison expects yyerror() to take a string, while _mesa_glsl_error() is a
    printf-style function.  This hit the classic printf string escape issue:
    
        _mesa_glsl_error(loc, state, "unexpected '%'");       // invalid!
        _mesa_glsl_error(loc, state, "%s", "unexpected '%'"); // correct.
    
    This caused assertion failures after ralloc_asprintf_append called
    vsnprintf to determine the length of the text that would be printed:
    vsnprintf would see the invalid format and return -1, an invalid length.
    
    The solution is to define a proper yyerror() wrapper function that calls
    _mesa_glsl_error with the "%s".  Since we compile with -p "_mesa_glsl",
    yyerror is defined as:
    
        #define yyerror         _mesa_glsl_error
    
    So we have to #undef yyerror in order to be able to declare it.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43564
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
    Acked-by: Paul Berry <stereotype441@gmail.com>

The patch works well and is on the upstream.

Tested-by: Sun Yi <yi.sun@intel.com>