Bug 181558 – [WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors

• Component: New Bugs
• Status: RESOLVED FIXED

[Dean Jackson]

[WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors

[Dean Jackson]

<rdar://problem/36189833>

[Dean Jackson]

Created attachment 331115 [details]
Patch

[Eric Carlson]

Comment on attachment 331115 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331115&action=review

> Source/WebCore/ChangeLog:13
> +        accidentally cast a unsigned to a signed.

Nit: "a unsigned" => "an unsigned"

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1830
> +    Checked<unsigned, RecordOverflow> checkedNumElementsRequired = Checked<unsigned>(maxIndex.value());
> +    checkedNumElementsRequired += 1;
> +    if (checkedNumElementsRequired.hasOverflowed())
> +        return false;
> +    numElementsRequired = checkedNumElementsRequired.unsafeGet();
> +    return true;

Nit: you could create a helper function for this since you have the same code in several places. Something like this uncompiled code might work:

    template <typename T> static inline std::optional<T> checkedAddUnsigned(T value, T toAdd)
    {
        Checked<T, RecordOverflow> checkedResult = Checked<T>(value);
        checkedResult += toAdd;
        if (checkedResult.hasOverflowed())
            return std::nullopt;

        return checkedResult.unsafeGet();
    }

...

    auto checkedResult = checkedAddUnsigned(maxIndex.value(), 1);
    if (!checkedResult)
        return false;

    numElementsRequired = checkedResult.value();
    return true;

[EWS Watchlist]

Comment on attachment 331115 [details]
Patch

Attachment 331115 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/6041466

New failing tests:
fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html

[EWS Watchlist]

Created attachment 331131 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 331115 [details]
Patch

Attachment 331115 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/6041190

New failing tests:
fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html

[EWS Watchlist]

Created attachment 331134 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 331115 [details]
Patch

Attachment 331115 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/6043533

New failing tests:
fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html

[EWS Watchlist]

Created attachment 331154 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Dean Jackson]

Committed r226916: <https://trac.webkit.org/changeset/226916>

[Dean Jackson]

There is a good chance some bots will crash on this test. Hello bot-watchers!

The test attempts to create a massive buffer, and should produce an OUT_OF_MEMORY error from GL. However, some hardware seems to crash instead.

If you see new crashes on iOS or High Sierra, that's bad. Please temporarily skip and let me know. We might have to just remove the test.

[Michael Catanzaro]

The test is failing (without crashing) on GTK, see bug #181659. That is, no GL error occurs, as expected. I wonder, does that indicate a problem in the OpenGL implementation...?