On Monday, August 18, 2014 11:49:13 AM Carl Worth wrote:
> We've found that there's a buffer overrun bug in flex that's triggered by
> using alternation in a lookahead pattern.
>
> Fortunately, we don't need to match the exact {NEWLINE} expression to detect
> an empty pragma. It suffices to verify that there are no non-space characters
> before any newline character. So we can use a simple [\r\n] to get the desired
> behavior while avoiding the flex bug.
>
> Fixes Piglit's 16385-consecutive-chars and
> 17000-consecutive-chars-identifier tests.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82472
> Signed-off-by: Carl Worth <[email protected]>
> Approach-suggested-by: Kenneth Graunke <[email protected]>You probably don't need this tag :) > CC: Kenneth Graunke <[email protected]> > --- > > Thanks for chasing down the fix for this regression of mine, Ken. > > I am embarrassed that I clearly didn't run piglit enough while testing my > original branch. > > With your fix above, there is some state that's not updated as it should > be when returning a NEWLINE token, (such as incrementing yylineno, etc.). > I tried to improve things to update all that state, but it proved > problematic, (putting the state updates in a common function doesn't > work because only the outer lexing function has access to local variables > like yylineno). Thanks for catching this. > > The alternate approach here was your recommendation, of course. > > src/glsl/glcpp/glcpp-lex.l | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l > index 98d500e..aaef7b8 100644 > --- a/src/glsl/glcpp/glcpp-lex.l > +++ b/src/glsl/glcpp/glcpp-lex.l > @@ -289,8 +289,14 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]? > } > > /* Swallow empty #pragma directives, (to avoid confusing the > - * downstream compiler). */ > -<HASH>pragma{HSPACE}*/{NEWLINE} { > + * downstream compiler). > + * > + * Note: We use a simple regular expression for the lookahead > + * here. Specifically, we cannot use the complete {NEWLINE} expression > + * since it uses alternation and we've found that there's a flex bug > + * where using alternation in the lookahead portion of a pattern > + * triggers a buffer overrun. / Erm. Shouldn't there be a star here? ^^^ I can't imagine this compiles...or worse, it does, and continues the comment to the */ in the rule below... > +<HASH>pragma{HSPACE}*/[\r\n] { > BEGIN INITIAL; > } With that fixed, and piglit tested, this would get my Reviewed-by.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
