Bug 119022 - Remove CheckedInt, use Checked<T, RecordOverflow> instead
Summary: Remove CheckedInt, use Checked<T, RecordOverflow> instead
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-23 15:27 PDT by Zan Dobersek
Modified: 2013-07-24 12:22 PDT (History)
7 users (show)

See Also:


Attachments
Patch (34.33 KB, patch)
2013-07-23 15:30 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
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 Details
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 Details
Patch (34.35 KB, patch)
2013-07-23 23:37 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2013-07-23 15:27:32 PDT
Remove CheckedInt, use Checked<T, RecordOverflow> instead
Comment 1 Zan Dobersek 2013-07-23 15:30:23 PDT
Created attachment 207352 [details]
Patch
Comment 2 Build Bot 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
Comment 3 Build Bot 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
Comment 4 Oliver Hunt 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 :(
Comment 5 Build Bot 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
Comment 6 Build Bot 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
Comment 7 Zan Dobersek 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.
Comment 8 Zan Dobersek 2013-07-23 23:37:13 PDT
Created attachment 207375 [details]
Patch

Reuploading for another review and a rerun through the EWSs.
Comment 9 Oliver Hunt 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
Comment 10 Zan Dobersek 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.
Comment 11 Zan Dobersek 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>
Comment 12 Zan Dobersek 2013-07-24 12:22:00 PDT
All reviewed patches have been landed.  Closing bug.