Bug 150797

Summary: Fix build warning in bignum.cc
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: New BugsAssignee: Csaba Osztrogonác <ossy>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, jh718.park, ossy
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 150619    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Csaba Osztrogonác 2015-11-02 05:49:18 PST
With GCC 5.2 I got the following warning:

/home/ossy/WebKit/Source/WTF/wtf/dtoa/bignum.cc: In member function ‘void WTF::double_conversion::Bignum::AssignDecimalString(WTF::double_conversion::BufferReference<const char>)’:
/home/ossy/WebKit/Source/WTF/wtf/dtoa/bignum.cc:105:10: error: assuming signed overflow does not occur when assuming that (X + c) < X is always false [-Werror=strict-overflow]
     void Bignum::AssignDecimalString(BufferReference<const char> value) {
          ^
cc1plus: all warnings being treated as errors
Comment 1 Csaba Osztrogonác 2015-11-02 05:49:35 PST
It is already fixed in V8:
https://github.com/v8/v8/commit/e28183b5977a2d2732f7f8b9a2f26637c9566585
Comment 2 Csaba Osztrogonác 2015-11-02 05:52:41 PST
Created attachment 264578 [details]
Patch

It's not the best fix, because it suppresses this warning. But from and digits_to_read are small numbers, it is unlikely to reach overflow in a real life use case.
Comment 3 Csaba Osztrogonác 2015-11-02 05:54:21 PST
Created attachment 264579 [details]
Patch

typo fixed
Comment 4 Geoffrey Garen 2015-11-02 10:15:26 PST
Comment on attachment 264579 [details]
Patch

r=me
Comment 5 Darin Adler 2015-11-02 22:32:56 PST
Comment on attachment 264579 [details]
Patch

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

> Source/WTF/wtf/dtoa/bignum.cc:98
> -        for (int i = from; i < from + digits_to_read; ++i) {
> +        int to = from + digits_to_read;
> +        for (int i = from; i < to; ++i) {
>              int digit = buffer[i] - '0';

Instead of just quieting the warning I would have suggested:

    for (int i = 0; i <digits_to_read; ++i) {
        int digit = buffer[from + i] - '0';

Seems slightly more in the spirit of the warning.
Comment 6 Hunseop Jeong 2015-11-22 16:23:21 PST
*** Bug 151553 has been marked as a duplicate of this bug. ***
Comment 7 Csaba Osztrogonác 2015-11-27 05:10:39 PST
Created attachment 266198 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2015-11-27 05:24:27 PST
Comment on attachment 266198 [details]
Patch for landing

Clearing flags on attachment: 266198

Committed r192779: <http://trac.webkit.org/changeset/192779>
Comment 9 WebKit Commit Bot 2015-11-27 05:24:32 PST
All reviewed patches have been landed.  Closing bug.