Bug 223697

Summary: Source/WebCore/page/FrameView.h:990:50: runtime error: signed integer overflow: 65537 * 65537 cannot be represented in type 'int'
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Layout and RenderingAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, darin, ggaren, sam, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=176131
Attachments:
Description Flags
Patch none

Description Chris Dumez 2021-03-24 10:05:10 PDT
Fix bug found by UBSan:
- Source/WebCore/page/FrameView.h:990:50: runtime error: signed integer overflow: 65537 * 65537 cannot be represented in type 'int'
- Source/WebCore/page/FrameView.h:990:50: runtime error: signed integer overflow: 65536 * 65536 cannot be represented in type 'int'
- Source/WebCore/page/FrameView.h:990:50: runtime error: signed integer overflow: 1116300 * 558150 cannot be represented in type 'int'
- Source/WebCore/page/FrameView.h:990:50: runtime error: signed integer overflow: -33554432 * -33554432 cannot be represented in type 'int'
Comment 1 Chris Dumez 2021-03-24 10:13:40 PDT
Created attachment 424150 [details]
Patch
Comment 2 Darin Adler 2021-03-24 10:20:50 PDT
Comment on attachment 424150 [details]
Patch

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

> Source/WebCore/ChangeLog:13
> +        - Source/WebCore/page/FrameView.h:990:50: runtime error: signed integer overflow: -33554432 * -33554432 cannot be represented in type 'int'

Surprised that we are computing area of sizes that have negative width or height.

> Source/WebCore/page/FrameView.h:994
> +    if (UNLIKELY(area.hasOverflowed()))

Makes me wish Checked had a "saturation" mode so we didn’t have to write such extensive code.
Comment 3 Chris Dumez 2021-03-24 10:21:55 PDT
(In reply to Darin Adler from comment #2)
> Comment on attachment 424150 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=424150&action=review
> 
> > Source/WebCore/ChangeLog:13
> > +        - Source/WebCore/page/FrameView.h:990:50: runtime error: signed integer overflow: -33554432 * -33554432 cannot be represented in type 'int'
> 
> Surprised that we are computing area of sizes that have negative width or
> height.

I suspect the values may already overflowed before this function call.

> 
> > Source/WebCore/page/FrameView.h:994
> > +    if (UNLIKELY(area.hasOverflowed()))
> 
> Makes me wish Checked had a "saturation" mode so we didn’t have to write
> such extensive code.
Comment 4 zalan 2021-03-24 10:36:21 PDT
no test case? I am curios how we end up with negative size here.
Comment 5 Darin Adler 2021-03-24 10:37:49 PDT
I believe the test case is "our entire regression test suite run when WebKit is compiled with UBSan".
Comment 6 Chris Dumez 2021-03-24 10:38:07 PDT
(In reply to zalan from comment #4)
> no test case? I am curios how we end up with negative size here.

The UBSan warnings are triggered by our test suite so the values showed in the errors should already be covered by our test suite.
Comment 7 Chris Dumez 2021-03-24 10:38:53 PDT
(In reply to zalan from comment #4)
> no test case? I am curios how we end up with negative size here.

I believe you should be able to add assertions then run the test suite and hopefully find out :)
Comment 8 zalan 2021-03-24 10:39:31 PDT
(In reply to Chris Dumez from comment #6)
> (In reply to zalan from comment #4)
> > no test case? I am curios how we end up with negative size here.
> 
> The UBSan warnings are triggered by our test suite so the values showed in
> the errors should already be covered by our test suite.
Can we figure out what test triggered this? This may have correctness implications as well.
Comment 9 zalan 2021-03-24 10:39:41 PDT
(In reply to Chris Dumez from comment #7)
> (In reply to zalan from comment #4)
> > no test case? I am curios how we end up with negative size here.
> 
> I believe you should be able to add assertions then run the test suite and
> hopefully find out :)
ok
Comment 10 EWS 2021-03-24 12:30:17 PDT
Committed r274958: <https://commits.webkit.org/r274958>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424150 [details].
Comment 11 Radar WebKit Bug Importer 2021-03-24 12:31:15 PDT
<rdar://problem/75799187>