Bug 119022 – Remove CheckedInt, use Checked<T, RecordOverflow> instead

• Component: New Bugs
• Status: RESOLVED FIXED

[Zan Dobersek]

Remove CheckedInt, use Checked<T, RecordOverflow> instead

[Zan Dobersek]

Created attachment 207352 [details]
Patch

[Build Bot]

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]

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]

(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 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]

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 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]

Created attachment 207375 [details]
Patch

Reuploading for another review and a rerun through the EWSs.

[Oliver Hunt]

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 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 on attachment 207375 [details]
Patch

Clearing flags on attachment: 207375

Committed r153095: <http://trac.webkit.org/changeset/153095>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.