[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Public WebGL] Possibility of not writing gl_Position is not addressed by WebGL spec



As long as we are discussing this, I feel it's worth mentioning an issue I had previously blamed myself, not the spec, for.

My WebGL framework includes a modularized shader pre-compiler that is smart enough to generate one shader from many distinct sources, in an attempt to improve code reuse across shaders. (The idea is not unlike the WebGL linker, except that with my approach, you can have more than one main() and the pre-compiler will figure out what to do with them all; and each module can run in isolation if the dev wishes to do so.) The unit tests for this part of my framework frequently involved compiling shaders which did not write to gl_Position. Many of them also did not write gl_FragColor. (There was no particular reason _not_ to write to these values -- it's just, writing them was not necessary in order to pass the unit tests in question, so it did not occur to me to write them.)

Most of the time this was fine, but I ran into intermittent crashes that had system-wide effects. As the test suite grew into the hundreds, this became more and more frequent. This was a year or two ago and I don't remember many specifics, but I do remember having to reboot my machine; it was in an unstable state even after shutting down all browsers. I tested primarily on Chrome and Firefox, with a Mac.

My solution was to enforce that the shader builder will always write a default value to gl_FragColor and gl_Position at the beginning of the shader's main(), to be overwritten by the modules as they executed. This resolved the issue, and since my use case seems pretty obscure, I did not see it as worth mentioning.

However, in hindsight I can see this might be an attack vector or at least a major issue if the crashing is not isolated to just my hardware. If it is reproducible in the latest browsers and is a widespread result, it may need to be addressed.


On Wed, Jun 19, 2013 at 4:45 AM, Olli Etuaho <oetuaho@nvidia.com> wrote:

Okay, I get the reasoning behind not enforcing writing gl_Position, and my initial impression that browsers would already do this was incorrect. Angle GLSL happily translates the shader without additions. However, a vertex shader that does not write to gl_Position at all can't perform any useful function besides feature-testing the compiler, so requiring such a shader to compile in the conformance test is still a bit odd to me. Has there been more earlier discussion on this than just http://www.khronos.org/bugzilla/show_bug.cgi?id=807 ?

-Olli
________________________________________
From: zmo@google.com [zmo@google.com] On Behalf Of Zhenyao Mo [zmo@chromium.org]
Sent: Wednesday, June 19, 2013 3:09 AM
To: Kenneth Russell
Cc: Olli Etuaho; public_webgl@khronos.org
Subject: Re: [Public WebGL] Possibility of not writing gl_Position is not addressed by WebGL spec

I think (I need to double check the code) in Angle HLSL output,
gl_Position is just initialized to 0,0,0 in the beginning.

On Tue, Jun 18, 2013 at 4:23 PM, Kenneth Russell <kbr@google.com> wrote:
>
> For complex vertex shaders, it will be difficult or impossible to
> assert that gl_Position is statically written to. It's similarly
> difficult to statically determine whether array indexing expressions
> are in-range, which is why
> http://www.khronos.org/registry/webgl/specs/latest/#4.5 is phrased the
> way it is; it's legal for a WebGL implementation to catch the error
> either at shader compilation time or at run time, so long as
> out-of-range data can't be accessed. The conformance suite enforces
> this behavior.
>
> While it's true that undefined behavior is undesirable, tightening
> this particular area of the spec is extremely problematic. Since there
> doesn't seem to be any associated security issue I think the WebGL
> spec should remain as is.
>
> GLSL ES 1.0.17 section 10.13 is pretty clear that not writing
> gl_Position is not an error, meaning that shader compilation must
> succeed. The same issue with this test has been raised by another
> OpenGL ES vendor recently, and the decision was made by the working
> group that the test should remain as is.
>
> -Ken
>
>
>
> On Tue, Jun 18, 2013 at 8:53 AM, Olli Etuaho <oetuaho@nvidia.com> wrote:
>>
>> Hi all,
>>
>> GLSL ES spec 1.0.17 section 10.13 says that not writing gl_Position results in undefined behavior, which is undesirable in WebGL. Current WebGL implementations do seem to enforce statically writing to gl_Position, but it's not mentioned in the WebGL spec. Should this be explicitly specified in the "Differences Between WebGL and OpenGL ES 2.0" section?
>>
>> There's also a related problem in the conformance test glsl/literals/float_literal.vert.html, where vertex shader does not write gl_Position. I think the test should be fixed by adding writing to gl_Position.
>>
>> A new test could also be written that specifically checks that not writing to gl_Position in a vertex shader will result in a shader compilation error.
>>
>> Regards, Olli
>> -----------------------------------------------------------
>> You are currently subscribed to public_webgl@khronos.org.
>> To unsubscribe, send an email to majordomo@khronos.org with
>> the following command in the body of your email:
>> unsubscribe public_webgl
>> -----------------------------------------------------------
>>
>
> -----------------------------------------------------------
> You are currently subscribed to public_webgl@khronos.org.
> To unsubscribe, send an email to majordomo@khronos.org with
> the following command in the body of your email:
> unsubscribe public_webgl
> -----------------------------------------------------------
>

-----------------------------------------------------------
You are currently subscribed to public_webgl@khronos.org.
To unsubscribe, send an email to majordomo@khronos.org with
the following command in the body of your email:
unsubscribe public_webgl
-----------------------------------------------------------




--
Colin MacKenzie IV
http://twitter.com/sinisterchipmnk