RESOLVED FIXED 119022
Remove CheckedInt, use Checked<T, RecordOverflow> instead
https://bugs.webkit.org/show_bug.cgi?id=119022
Summary Remove CheckedInt, use Checked<T, RecordOverflow> instead
Zan Dobersek
Reported 2013-07-23 15:27:32 PDT
Remove CheckedInt, use Checked<T, RecordOverflow> instead
Attachments
Patch (34.33 KB, patch)
2013-07-23 15:30 PDT, Zan Dobersek
no flags
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (795.27 KB, application/zip)
2013-07-23 20:12 PDT, Build Bot
no flags
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (537.83 KB, application/zip)
2013-07-23 22:22 PDT, Build Bot
no flags
Patch (34.35 KB, patch)
2013-07-23 23:37 PDT, Zan Dobersek
no flags
Zan Dobersek
Comment 1 2013-07-23 15:30:23 PDT
Build Bot
Comment 2 2013-07-23 20:12:05 PDT
Comment on attachment 207352 [details] Patch Attachment 207352 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1183425 New failing tests: fast/canvas/webgl/draw-arrays-out-of-bounds.html webgl/conformance/rendering/draw-arrays-out-of-bounds.html
Build Bot
Comment 3 2013-07-23 20:12:06 PDT
Created attachment 207368 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Oliver Hunt
Comment 4 2013-07-23 20:13:17 PDT
(In reply to comment #2) > (From update of attachment 207352 [details]) > Attachment 207352 [details] did not pass mac-ews (mac): > Output: http://webkit-queues.appspot.com/results/1183425 > > New failing tests: > fast/canvas/webgl/draw-arrays-out-of-bounds.html > webgl/conformance/rendering/draw-arrays-out-of-bounds.html okay, that's weird - are we safely computing something that CheckedInt couldn't? or are we failing :(
Build Bot
Comment 5 2013-07-23 22:22:01 PDT
Comment on attachment 207352 [details] Patch Attachment 207352 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1183451 New failing tests: fast/canvas/webgl/draw-arrays-out-of-bounds.html webgl/conformance/rendering/draw-arrays-out-of-bounds.html
Build Bot
Comment 6 2013-07-23 22:22:04 PDT
Created attachment 207372 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Zan Dobersek
Comment 7 2013-07-23 23:31:46 PDT
Comment on attachment 207352 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207352&action=review > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:1958 > + Checked<GC3Dint> checkedFirst(first); > + Checked<GC3Dint> checkedCount(count); > + Checked<GC3Dint> checkedSum = checkedFirst + checkedCount; RecordOverflow is not being used here. When used, the crashes are fixed. Will reupload the patch.
Zan Dobersek
Comment 8 2013-07-23 23:37:13 PDT
Created attachment 207375 [details] Patch Reuploading for another review and a rerun through the EWSs.
Oliver Hunt
Comment 9 2013-07-24 10:05:10 PDT
Comment on attachment 207375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207375&action=review > Source/WebCore/html/canvas/CheckedInt.h:-822 > -typedef CheckedInt<int8_t> CheckedInt8; > -typedef CheckedInt<uint8_t> CheckedUint8; > -typedef CheckedInt<int16_t> CheckedInt16; > -typedef CheckedInt<uint16_t> CheckedUint16; > -typedef CheckedInt<int32_t> CheckedInt32; > -typedef CheckedInt<uint32_t> CheckedUint32; > -typedef CheckedInt<int64_t> CheckedInt64; > -typedef CheckedInt<uint64_t> CheckedUint64; If we wanted we could add these typedefs (with the record variant) to CheckedArithmetic -- they weren't there in the past due to conflicts with these typedefs
Zan Dobersek
Comment 10 2013-07-24 12:04:29 PDT
Comment on attachment 207375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207375&action=review >> Source/WebCore/html/canvas/CheckedInt.h:-822 >> -typedef CheckedInt<uint64_t> CheckedUint64; > > If we wanted we could add these typedefs (with the record variant) to CheckedArithmetic -- they weren't there in the past due to conflicts with these typedefs Cool, I'll put that in a follow-up patch. Thanks for reviewing.
Zan Dobersek
Comment 11 2013-07-24 12:21:53 PDT
Comment on attachment 207375 [details] Patch Clearing flags on attachment: 207375 Committed r153095: <http://trac.webkit.org/changeset/153095>
Zan Dobersek
Comment 12 2013-07-24 12:22:00 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.