Bug 135049 - Division by zero in fast-dtoa.cc.
Summary: Division by zero in fast-dtoa.cc.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-18 07:36 PDT by peavo
Modified: 2015-01-27 23:03 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.20 KB, patch)
2014-07-18 07:39 PDT, peavo
benjamin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description peavo 2014-07-18 07:36:08 PDT
I occasionally get a divison by zero exception on line 445 in fast-dtoa.cc.

444	        while (*kappa > 0) {
445	            int digit = integrals / divisor;
446	            buffer[*length] = '0' + digit;
Comment 1 peavo 2014-07-18 07:39:27 PDT
Created attachment 235127 [details]
Patch
Comment 2 Darin Adler 2014-07-18 08:26:36 PDT
Comment on attachment 235127 [details]
Patch

Could you also supply a test case? How did you discover this problem?
Comment 3 peavo 2014-07-18 10:32:10 PDT
(In reply to comment #2)
> (From update of attachment 235127 [details])
> Could you also supply a test case? How did you discover this problem?

I don't have a testcase available currently, but I can see if I can come up with one.
Before adding the check, I got the crash fairly often during "normal" browsing.
I don't remember any specific sites, though.
Comment 4 Darin Adler 2014-07-18 12:16:54 PDT
(In reply to comment #3)
> Before adding the check, I got the crash fairly often during "normal" browsing.

I don’t think we have reports of this crash on Mac. I believe Intel processors raise an exception on division by zero, and that’s what Macs use. Is this code that’s not used in the Mac port?
Comment 5 Benjamin Poulain 2014-07-20 23:47:09 PDT
Comment on attachment 235127 [details]
Patch

First, I agree with Darin: this must have a test.

But even with the test, I do not believe this would be the right fix. If divisor is zero, that likely means the input is invalid. If that happen, we should handle that earlier in the stack (or maybe an early return in DigitGen()). I don't think you can just ignore this loop.
Comment 6 peavo 2014-07-21 11:08:26 PDT
(In reply to comment #5)

Thanks for reviewing :)

> (From update of attachment 235127 [details])
> First, I agree with Darin: this must have a test.
> 
> But even with the test, I do not believe this would be the right fix. If divisor is zero, that likely means the input is invalid. If that happen, we should handle that earlier in the stack (or maybe an early return in DigitGen()). I don't think you can just ignore this loop.

Sounds good, I will try get some more details on the crash, but "unfortunately" I haven't seen the crash the last couple of days ...
Comment 7 peavo 2015-01-27 23:03:01 PST
I haven't seen this crash in a long time now, so I believe it has been fixed.