Confirming with Kabini, so bug is valid with that year 2012 binary.

 But, i tried to build up to date one from https://github.com/ReactionQuake3/reaction and bug is not there

Smoki, i belive it works. I used it for testing only (this 2012 binary is part of phoronix-test-suite) but it stopped work some time ago. This is my problem, not reaction itself:)
btw thanks for testing

I reported this last July, and Carl Worth came up with a fix in http://lists.freedesktop.org/archives/mesa-dev/2014-July/064438.html (more discussion in http://lists.freedesktop.org/archives/mesa-dev/2014-August/064809.html and following), but apparently the fixes haven't landed yet.

I'm glad to hear that the bug in Reaction Quake has since been fixed.

But I'm concerned to learn that the patches I wrote never landed---that's a bit distressing.

For reference, it looks like my latest patch after all of the discussion is here:

http://lists.freedesktop.org/archives/mesa-dev/2014-August/065087.html

I'll need to track down why that was never committed, (I was probably waiting for review at the time?). And I'll also need to track down what other changes I might have in branches that never got pushed.

That said, I don't think that that change to Mesa would mean that Reaction Quake with the bug would start working. It really was sending a bogus shader to Mesa, and it really was just a bug in Mesa that was causing it to be accepted as valid.
(This is all based on my recollection which might not be fully accurate.)

-Carl

Actually, looking closer at the "v2" portion of my commit message in
the proposed patch:

	v2: Instead of emitting the error as soon as the illegal
	character is lexed, we instead emit an ILLEGAL token to the
	parser. This allows the parser to allow the character as part
	of the replacement list of a macro, (since these are specified
	to allow any character). However, if such a macro is actually
	instantiated, the parser will emit an error when it goes to
	print the illegal character as part of the preprocessed
	output.

It would appear that with this patch applied, even a version of
Reactive Quake with the bug would still work fine, (since, from what I
recall, the bogusly-defined macro is never instantiated).

So, yes, I'll see about getting this patch pushed out as soon as
possible.

-Carl

Hi Carl, it's 2017 here and your patches really lost in time or so:) probably no one care about ReactionQuake3 and for sure no one will play with 2012's. I didn't see any other infected. So for me this should be closed long time ago and if you don't mind, feel free to do it.

Yeah, they definitely got lost in time.  A while back someone pointed them out and I rebased them...I just rebased them again:

https://cgit.freedesktop.org/~kwg/mesa/log/?h=cworth-glcpp-charset-2017

I ran them through shader-db and the various CTSes with no problems.  I hit some Piglit regressions:

piglit.spec.glsl-es-3_00.compiler.utf8-identifier_vert
piglit.spec.glsl-es-3_00.compiler.utf8-function_vert
piglit.spec.glsl-es-3_00.compiler.utf8-used-define_vert

That said...I think the tests are actually working.  It looks like glcpp printing the bogus character in error messages is making Piglit's output parsing explode.

-- 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/805.