Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Functions that read from out parameters have undefined behavior #3628

Open
magcius opened this issue Feb 29, 2024 · 12 comments
Open

Functions that read from out parameters have undefined behavior #3628

magcius opened this issue Feb 29, 2024 · 12 comments
Assignees

Comments

@magcius
Copy link

magcius commented Feb 29, 2024

void foo(out float a) {
  a += 2.0f;
}

void main() {
  float x = 5.0f;
  foo(x);
  gl_FragColor = vec4(x);
}

This is not an error, and is undefined behavior, but it works fine on some GPUs because their drivers effectively treat out as inout. However, I've encountered issues with other GPU drivers that are more strict. While we probably can't make it an error due to backcompat, maybe a compiler warning should be emitted for this?

@lexaknyazev
Copy link
Member

Given the WebGL robustness restrictions, this should be behave like the following everywhere

void foo(out float a) {
  float _a = 0.0;
  _a += 2.0f;
  a = _a;
}

void main() {
  float x = 5.0f;
  foo(x);
  // x == 2.0
  gl_FragColor = vec4(x);
}

Please specify what do you see on which platforms.

@magcius
Copy link
Author

magcius commented Feb 29, 2024

I saw the behavior on Firefox and Linux. mesa treated out variables as undefined contents, and I suspect that Firefox is passing shaders through to the system GL implementation as-is.

@kdashg
Copy link
Contributor

kdashg commented Feb 29, 2024

You can use WEBGL_debug_shaders.getTranslatedShaderSource() to see what we (and/or other browsers) are passing to the driver.
We do not pass the driver shaders as-is, though it's possible we're mistakenly missing a safety transform that e.g. Chromium is not.

@magcius
Copy link
Author

magcius commented Feb 29, 2024

https://jsbin.com/fiqazoqula/edit?js,output

Tested with Firefox 123.0 on Windows 10, here's what I see as the output. Firefox is generating code that has undefined behavior.

void webgl_550c7d686f02ef30(out highp float webgl_cc4b575f1e53d03d){
  const highp float s15e9 = 0.2;
  (webgl_cc4b575f1e53d03d += s15e9);
}
void main(){
  (gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0));
  highp float webgl_350702f5a0424893 = 0.5;
  webgl_550c7d686f02ef30(webgl_350702f5a0424893);
  (gl_FragColor = vec4(webgl_350702f5a0424893));
}

I don't have access to a Linux machine currently, so I can't test Chrome on OpenGL.

Chrome 124.0.6330.0 on ANGLE D3D11 changes the parameter to be an inout, which does not match your expected robustness transform:

#define GL_USES_FRAG_COLOR
void f_test(inout float _v)
{
(_v += 0.2);
}
@@ PIXEL OUTPUT @@

PS_OUTPUT main(@@ PIXEL MAIN PARAMETERS @@){
@@ MAIN PROLOGUE @@
float _x5628 = {0.5};
f_test(_x5628);
(gl_Color[0] = vec4_ctor(_x5628));
return generateOutput();
}

@magcius
Copy link
Author

magcius commented Mar 1, 2024

A friend ran the script on Chrome 120.0.6099.199 on Linux/OpenGL, which provided the following results:

#version 450
out vec4 webgl_FragColor;
void _utest(out float _uv){
  (_uv = 0.0);
  (_uv += 0.2);
}
void main(){
  (webgl_FragColor = vec4(0.0, 0.0, 0.0, 0.0));
  float _ux = 0.5;
  _utest(_ux);
  (webgl_FragColor = vec4(_ux));
}

So it appears ANGLE's OpenGL shader translator is applying the transform @lexaknyazev expected to see here.

@kenrussell
Copy link
Member

@magcius what's the output of Firefox's about:support on the Windows 10 machine that generated [this output)(https://github.com//issues/3628#issuecomment-1972168018)? I thought Firefox was using ANGLE's D3D11 backend by default there, so it should have similar output to Chrome's on Windows.

@kdashg kdashg self-assigned this Mar 7, 2024
@kdashg
Copy link
Contributor

kdashg commented Mar 7, 2024

I will update the spec with the expected initialize out to 0 behavior.

Bugs should be filed against browsers that don't do this properly already.

@magcius
Copy link
Author

magcius commented Mar 9, 2024

I could upload the about:support log somewhere, but it's fairly large and I don't want to post it directly in this issue. My (perhaps out of date) understanding is that Firefox doesn't use ANGLE's D3D11 backend for WebGL 2, it uses system OpenGL. I believe it uses ANGLE for shader translation, though.

@kenrussell
Copy link
Member

Could you perhaps upload it to pastebin and post the link here? Or if you'd rather not have the info linked here, feel free to email the link to me at kbr at chromium dot org.

@kdashg
Copy link
Contributor

kdashg commented Mar 13, 2024

Firefox has always used ANGLE's ES3->D3D11 implementation as our "GLES" driver on Windows for webgl2, at least by default. We have prefs to use native GL, but we've never "shipped" that config. This is however different from Chrome, in that Chrome effectively has ANGLE map directly webgl2->d3d11 on Windows, whereas Firefox uses ANGLE as a webgl-agnostic GLES driver, and does the webgl->gl(es) implementation within Gecko.

@kdashg
Copy link
Contributor

kdashg commented Mar 13, 2024

For example, this machine's about:support reads:

WebGL 2 Driver Renderer:    Google Inc. (Intel) -- ANGLE (Intel, Intel(R) Arc(TM) A770 Graphics Direct3D11 vs_5_0 ps_5_0, D3D11-31.0.101.5186)
WebGL 2 Driver Version:     OpenGL ES 3.0.0 (ANGLE 2.1.19736 git hash: ddaf44ac75d5)

@kenrussell
Copy link
Member

Per WebGL concall of 2024-03-21: some targeted tests are needed in the WebGL CTS (conformance/glsl/misc/?) for these cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants