⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 280904 in webkit


Ignore:
Timestamp:
Aug 11, 2021, 4:20:55 AM (5 years ago)
Author:
commit-queue@webkit.org
Message:

Cherry-pick ANGLE: Revise WebGL's shaderSource validation
​https://bugs.webkit.org/show_bug.cgi?id=228951

Patch by Kimmo Kinnunen <​kkinnunen@apple.com> on 2021-08-11
Reviewed by Kenneth Russell.

Source/ThirdParty/ANGLE:

Cherry-pick ANGLE commit: b4fd46288aa65d61dc9c7140c7d1cdba3f4cdf9a
From: Kenneth Russell <​kbr@chromium.org>
Date: Wed, 27 Jan 2021 15:56:58 -0800
Revise WebGL's shaderSource validation.

Per discussion in the WebGL working group, shaderSource no longer
generates INVALID_VALUE for sources containing characters outside the
ESSL character set. Compilation and/or linking is still specified to
fail when illegal constructs are used.

With this change, ​https://github.com/KhronosGroup/WebGL/pull/3206
passes with the passthrough command decoder.

Revise WebGL compatibility tests to follow the new rules.

  • src/libANGLE/validationES2.cpp:

(gl::ValidateShaderSource):

  • src/tests/gl_tests/WebGLCompatibilityTest.cpp:

LayoutTests:

Fixes tests:
webgl/1.0.x/conformance/misc/invalid-passed-params.html
webgl/1.0.x/conformance/glsl/bugs/character-set.html
webgl/2.0.y/conformance/misc/invalid-passed-params.html
webgl/2.0.y/conformance/glsl/bugs/character-set.html

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r280889 r280904  
     12021-08-11  Kimmo Kinnunen  <kkinnunen@apple.com>
     2
     3        Cherry-pick ANGLE: Revise WebGL's shaderSource validation
     4        https://bugs.webkit.org/show_bug.cgi?id=228951
     5
     6        Reviewed by Kenneth Russell.
     7
     8        Fixes tests:
     9        webgl/1.0.x/conformance/misc/invalid-passed-params.html
     10        webgl/1.0.x/conformance/glsl/bugs/character-set.html
     11        webgl/2.0.y/conformance/misc/invalid-passed-params.html
     12        webgl/2.0.y/conformance/glsl/bugs/character-set.html
     13
     14        * TestExpectations:
     15
    1162021-08-11  Cathie Chen  <cathiechen@igalia.com>
    217

  • trunk/LayoutTests/TestExpectations

    r280889 r280904  
    36423642# Explicitly enable tests which we have fixed and do not have corresponding 1.0.3 test functionality.
    36433643webgl/1.0.x/conformance/canvas/to-data-url-test.html [ Pass ]
     3644webgl/1.0.x/conformance/misc/invalid-passed-params.html [ Pass ]
     3645webgl/1.0.x/conformance/glsl/bugs/character-set.html [ Pass ]
    36443646
    36453647# WebGL conformance test suite 2.0.1 is skipped until 2.0.0 is retired.
    … …  
    36483650# Explicitly enable tests which we have fixed and do not have corresponding 2.0.y test functionality.
    36493651webgl/2.0.y/conformance/canvas/to-data-url-test.html [ Pass ]
     3652webgl/2.0.y/conformance/misc/invalid-passed-params.html [ Pass ]
     3653webgl/2.0.y/conformance/glsl/bugs/character-set.html [ Pass ]
     3654
     3655# WebGL 1.0.3 and 2.0.0 tests where behavior is obsolete and WebKit contains implementation
     3656# and tests for the new behavior. Should be removed once 1.0.3 and 2.0.0 are retired.
     3657webgl/1.0.3/conformance/glsl/misc/shader-with-define-line-continuation.frag.html [ Skip ]
     3658webgl/1.0.3/conformance/misc/invalid-passed-params.html [ Skip ]
     3659webgl/2.0.0/conformance/glsl/misc/shader-with-define-line-continuation.frag.html [ Skip ]
     3660webgl/2.0.0/conformance/misc/invalid-passed-params.html [ Skip ]
     3661fast/canvas/webgl/invalid-passed-params.html [ Skip ]
    36503662
    36513663# pre-wrap progression. Other rendering engines agree with the result.

  • trunk/Source/ThirdParty/ANGLE/ChangeLog

    r280349 r280904  
     12021-08-11  Kimmo Kinnunen  <kkinnunen@apple.com>
     2
     3        Cherry-pick ANGLE: Revise WebGL's shaderSource validation
     4        https://bugs.webkit.org/show_bug.cgi?id=228951
     5
     6        Reviewed by Kenneth Russell.
     7
     8        Cherry-pick ANGLE commit: b4fd46288aa65d61dc9c7140c7d1cdba3f4cdf9a
     9        From: Kenneth Russell <kbr@chromium.org>
     10        Date: Wed, 27 Jan 2021 15:56:58 -0800
     11        Revise WebGL's shaderSource validation.
     12
     13        Per discussion in the WebGL working group, shaderSource no longer
     14        generates INVALID_VALUE for sources containing characters outside the
     15        ESSL character set. Compilation and/or linking is still specified to
     16        fail when illegal constructs are used.
     17
     18        With this change, https://github.com/KhronosGroup/WebGL/pull/3206
     19        passes with the passthrough command decoder.
     20
     21        Revise WebGL compatibility tests to follow the new rules.
     22
     23        * src/libANGLE/validationES2.cpp:
     24        (gl::ValidateShaderSource):
     25        * src/tests/gl_tests/WebGLCompatibilityTest.cpp:
     26
    1272021-07-23  Dean Jackson  <dino@apple.com>
    228

  • trunk/Source/ThirdParty/ANGLE/src/libANGLE/validationES2.cpp

    r270351 r280904  
    806806}
    807807
    808 bool IsValidESSLShaderSourceString(const char *str, size_t len, bool lineContinuationAllowed)
    809 {
    810     enum class ParseState
    811     {
    812         // Have not seen an ASCII non-whitespace character yet on
    813         // this line. Possible that we might see a preprocessor
    814         // directive.
    815         BEGINING_OF_LINE,
    816 
    817         // Have seen at least one ASCII non-whitespace character
    818         // on this line.
    819         MIDDLE_OF_LINE,
    820 
    821         // Handling a preprocessor directive. Passes through all
    822         // characters up to the end of the line. Disables comment
    823         // processing.
    824         IN_PREPROCESSOR_DIRECTIVE,
    825 
    826         // Handling a single-line comment. The comment text is
    827         // replaced with a single space.
    828         IN_SINGLE_LINE_COMMENT,
    829 
    830         // Handling a multi-line comment. Newlines are passed
    831         // through to preserve line numbers.
    832         IN_MULTI_LINE_COMMENT
    833     };
    834 
    835     ParseState state = ParseState::BEGINING_OF_LINE;
    836     size_t pos       = 0;
    837 
    838     while (pos < len)
    839     {
    840         char c    = str[pos];
    841         char next = pos + 1 < len ? str[pos + 1] : 0;
    842 
    843         // Check for newlines
    844         if (c == '\n' || c == '\r')
    845         {
    846             if (state != ParseState::IN_MULTI_LINE_COMMENT)
    847             {
    848                 state = ParseState::BEGINING_OF_LINE;
    849             }
    850 
    851             pos++;
    852             continue;
    853         }
    854 
    855         switch (state)
    856         {
    857             case ParseState::BEGINING_OF_LINE:
    858                 if (c == ' ')
    859                 {
    860                     // Maintain the BEGINING_OF_LINE state until a non-space is seen
    861                     pos++;
    862                 }
    863                 else if (c == '#')
    864                 {
    865                     state = ParseState::IN_PREPROCESSOR_DIRECTIVE;
    866                     pos++;
    867                 }
    868                 else
    869                 {
    870                     // Don't advance, re-process this character with the MIDDLE_OF_LINE state
    871                     state = ParseState::MIDDLE_OF_LINE;
    872                 }
    873                 break;
    874 
    875             case ParseState::MIDDLE_OF_LINE:
    876                 if (c == '/' && next == '/')
    877                 {
    878                     state = ParseState::IN_SINGLE_LINE_COMMENT;
    879                     pos++;
    880                 }
    881                 else if (c == '/' && next == '*')
    882                 {
    883                     state = ParseState::IN_MULTI_LINE_COMMENT;
    884                     pos++;
    885                 }
    886                 else if (lineContinuationAllowed && c == '\\' && (next == '\n' || next == '\r'))
    887                 {
    888                     // Skip line continuation characters
    889                 }
    890                 else if (!IsValidESSLCharacter(c))
    891                 {
    892                     return false;
    893                 }
    894                 pos++;
    895                 break;
    896 
    897             case ParseState::IN_PREPROCESSOR_DIRECTIVE:
    898                 // Line-continuation characters may not be permitted.
    899                 // Otherwise, just pass it through. Do not parse comments in this state.
    900                 if (!lineContinuationAllowed && c == '\\')
    901                 {
    902                     return false;
    903                 }
    904                 pos++;
    905                 break;
    906 
    907             case ParseState::IN_SINGLE_LINE_COMMENT:
    908                 // Line-continuation characters are processed before comment processing.
    909                 // Advance string if a new line character is immediately behind
    910                 // line-continuation character.
    911                 if (c == '\\' && (next == '\n' || next == '\r'))
    912                 {
    913                     pos++;
    914                 }
    915                 pos++;
    916                 break;
    917 
    918             case ParseState::IN_MULTI_LINE_COMMENT:
    919                 if (c == '*' && next == '/')
    920                 {
    921                     state = ParseState::MIDDLE_OF_LINE;
    922                     pos++;
    923                 }
    924                 pos++;
    925                 break;
    926         }
    927     }
    928 
    929     return true;
    930 }
    931 
    932808bool ValidateWebGLNamePrefix(const Context *context, const GLchar *name)
    933809{
    … …  
    49494825    }
    49504826
    4951     // The WebGL spec (section 6.20) disallows strings containing invalid ESSL characters for
    4952     // shader-related entry points
    4953     if (context->getExtensions().webglCompatibility)
    4954     {
    4955         for (GLsizei i = 0; i < count; i++)
    4956         {
    4957             size_t len =
    4958                 (length && length[i] >= 0) ? static_cast<size_t>(length[i]) : strlen(string[i]);
    4959 
    4960             // Backslash as line-continuation is allowed in WebGL 2.0.
    4961             if (!IsValidESSLShaderSourceString(string[i], len,
    4962                                                context->getClientVersion() >= ES_3_0))
    4963             {
    4964                 context->validationError(GL_INVALID_VALUE, kShaderSourceInvalidCharacters);
    4965                 return false;
    4966             }
    4967         }
    4968     }
    4969 
    49704827    Shader *shaderObject = GetValidShader(context, shader);
    49714828    if (!shaderObject)

  • trunk/Source/ThirdParty/ANGLE/src/tests/gl_tests/WebGLCompatibilityTest.cpp

    r270351 r280904  
    21572157    {
    21582158        std::string invalidAttribName = validAttribName + invalidChar;
    2159         const char *invalidVert[]     = {
    2160             "attribute float ",
    2161             invalidAttribName.c_str(),
    2162             R"(;,
     2159        std::string invalidVert       = "attribute float ";
     2160        invalidVert += invalidAttribName;
     2161        invalidVert += R"(;,
    21632162void main(),
    21642163{,
    21652164    gl_Position = vec4(1.0);,
    2166 })",
    2167         };
    2168 
    2169         GLuint shader = glCreateShader(GL_VERTEX_SHADER);
    2170         glShaderSource(shader, static_cast<GLsizei>(ArraySize(invalidVert)), invalidVert, nullptr);
    2171         EXPECT_GL_ERROR(GL_INVALID_VALUE);
    2172         glDeleteShader(shader);
    2173     }
    2174 }
    2175 
    2176 // Test that line continuation is handled correctly when valdiating shader source
     2165})";
     2166        GLuint program = CompileProgram(invalidVert.c_str(), essl1_shaders::fs::Red());
     2167        EXPECT_EQ(0u, program);
     2168    }
     2169}
     2170
     2171// Test that line continuation is handled correctly when validating shader source
    21772172TEST_P(WebGLCompatibilityTest, ShaderSourceLineContinuation)
    21782173{
    2179     // Verify that a line continuation character (i.e. backslash) cannot be used
    2180     // within a preprocessor directive in a ES2 context.
    2181     ANGLE_SKIP_TEST_IF(getClientMajorVersion() >= 3);
     2174    // With recent changes to WebGL's shader source validation in
     2175    // https://github.com/KhronosGroup/WebGL/pull/3206 and follow-ons,
     2176    // the backslash character can be used in both WebGL 1.0 and 2.0
     2177    // contexts.
    21822178
    21832179    const char *validVert =
    2184         R"(#define foo this is a test
    2185 precision mediump float;
    2186 void main()
    2187 {
    2188     gl_Position = vec4(1.0);
    2189 })";
    2190 
    2191     const char *invalidVert =
    21922180        R"(#define foo this \
    21932181    is a test
    … …  
    21982186})";
    21992187
    2200     GLuint shader = glCreateShader(GL_VERTEX_SHADER);
    2201     glShaderSource(shader, 1, &validVert, nullptr);
    2202     EXPECT_GL_NO_ERROR();
    2203 
    2204     glShaderSource(shader, 1, &invalidVert, nullptr);
    2205     EXPECT_GL_ERROR(GL_INVALID_VALUE);
    2206     glDeleteShader(shader);
     2188    GLuint program = CompileProgram(validVert, essl1_shaders::fs::Red());
     2189    EXPECT_NE(0u, program);
     2190    glDeleteProgram(program);
    22072191}
    22082192
    … …  
    22322216})";
    22332217
    2234     GLuint shader = glCreateShader(GL_VERTEX_SHADER);
    2235     glShaderSource(shader, 1, &validVert, nullptr);
    2236     EXPECT_GL_NO_ERROR();
    2237     glShaderSource(shader, 1, &invalidVert, nullptr);
    2238     EXPECT_GL_ERROR(GL_INVALID_VALUE);
    2239     glDeleteShader(shader);
     2218    GLuint program = CompileProgram(validVert, essl3_shaders::fs::Red());
     2219    EXPECT_NE(0u, program);
     2220    glDeleteProgram(program);
     2221
     2222    program = CompileProgram(invalidVert, essl3_shaders::fs::Red());
     2223    EXPECT_EQ(0u, program);
    22402224}
    22412225
Note: See TracChangeset for help on using the changeset viewer.