RESOLVED FIXED 181558
[WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors
https://bugs.webkit.org/show_bug.cgi?id=181558
Summary [WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors
Dean Jackson
Reported 2018-01-11 14:03:34 PST
[WebGL] Simulated vertexAttrib0 can sometimes cause OUT_OF_MEMORY errors
Attachments
Patch (16.24 KB, patch)
2018-01-11 14:09 PST, Dean Jackson
eric.carlson: review+
ews-watchlist: commit-queue-
Archive of layout-test-results from ews103 for mac-sierra (2.66 MB, application/zip)
2018-01-11 16:04 PST, EWS Watchlist
no flags
Archive of layout-test-results from ews117 for mac-sierra (3.35 MB, application/zip)
2018-01-11 16:08 PST, EWS Watchlist
no flags
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.67 MB, application/zip)
2018-01-11 18:54 PST, EWS Watchlist
no flags
Dean Jackson
Comment 1 2018-01-11 14:04:50 PST
Dean Jackson
Comment 2 2018-01-11 14:09:55 PST
Eric Carlson
Comment 3 2018-01-11 14:30:15 PST
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 4 2018-01-11 16:04:11 PST
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
Comment 5 2018-01-11 16:04:13 PST
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 6 2018-01-11 16:08:56 PST
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
Comment 7 2018-01-11 16:08:57 PST
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 8 2018-01-11 18:54:34 PST
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
Comment 9 2018-01-11 18:54:35 PST
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
Comment 10 2018-01-12 15:02:16 PST
Dean Jackson
Comment 11 2018-01-12 15:14:40 PST
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
Comment 12 2018-01-15 17:34:33 PST
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...?
Note You need to log in before you can comment on or make changes to this bug.