Bug 150797 - Fix build warning in bignum.cc
Summary: Fix build warning in bignum.cc
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
: 151553 (view as bug list)
Depends on:
Blocks: 150619
  Show dependency treegraph
 
Reported: 2015-11-02 05:49 PST by Csaba Osztrogonác
Modified: 2015-11-27 05:24 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.28 KB, patch)
2015-11-02 05:52 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (1.28 KB, patch)
2015-11-02 05:54 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch for landing (1.31 KB, patch)
2015-11-27 05:10 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.