1334979 - Fix a truncation leading to an error with gcc 7

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

0:35.60 /root/firefox-gcc-last/toolkit/components/ctypes/tests/jsctypes-test-finalizer.cpp: In function 'char* test_finalizer_acq_string_t(int)':
 0:35.60 /root/firefox-gcc-last/toolkit/components/ctypes/tests/jsctypes-test-finalizer.cpp:231:1: error: '%d' directive output may be truncated writing between 1 and 11 bytes into a region of size 10 [-Werror=format-truncation=]
 0:35.60  test_finalizer_acq_string_t(int i)
 0:35.60  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
 0:35.60 /root/firefox-gcc-last/toolkit/components/ctypes/tests/jsctypes-test-finalizer.cpp:231:1: note: using the range [1, -2147483648] for directive argument
 0:35.60 /root/firefox-gcc-last/toolkit/components/ctypes/tests/jsctypes-test-finalizer.cpp:236:13: note: format output between 2 and 12 bytes into a destination of size 10
 0:35.60      snprintf(buf, 10, "%d", i);
 0:35.60      ~~~~~~~~^~~~~~~~~~~~~~~~~~
Because:
$ echo -n "-2147483648"|wc -c
11

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1334979 - Fix a truncation leading to an error with gcc 7

Review commit: https://reviewboard.mozilla.org/r/108184/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/108184/

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8831635 [details]
Bug 1334979 - Fix a truncation leading to an error with gcc 7

https://reviewboard.mozilla.org/r/108184/#review110856

::: toolkit/components/ctypes/tests/jsctypes-test-finalizer.cpp:236
(Diff revision 1)
>  test_finalizer_acq_string_t(int i)
>  {
>    gFinalizerTestResources[i] = 1;
>    if (!gFinalizerTestNames[i]) {
>      char* buf = new char[10];
> -    snprintf(buf, 10, "%d", i);
> +    snprintf(buf, 11, "%d", i);

1. http://en.cppreference.com/w/c/io/fprintf says that if bufsz is the second argument, at most bufsz-1 elements are written. So to write INT32_MIN, you need 11 chars, thus bufsz should be 12.

2. The line right above needs to be changed too, to allocate 12 chars (including the leading zero character), right?

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8831635 [details]
Bug 1334979 - Fix a truncation leading to an error with gcc 7

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/108184/diff/1-2/

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8831635 [details]
Bug 1334979 - Fix a truncation leading to an error with gcc 7

https://reviewboard.mozilla.org/r/108184/#review111024

Stealing review, assuming it should have been redirected to me. Please update the reviewer in the commit message. Thanks for the patch!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8be8dd1678a5
Fix a truncation leading to an error with gcc 7 r=bbouvier

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8be8dd1678a5