[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 Martin Sebor changed: What|Removed |Added Known to fail|8.0 |10.3.0, 11.2.0, 8.3.0, ||9.3.0 Target Milestone|--- |12.0 Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #17 from Martin Sebor --- Fixed in GCC 12.
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 --- Comment #16 from CVS Commits --- The master branch has been updated by Martin Sebor : https://gcc.gnu.org/g:639ece7abfa3688008cb791aec4c7a1a4f76e59f commit r12-6002-g639ece7abfa3688008cb791aec4c7a1a4f76e59f Author: Martin Sebor Date: Wed Dec 15 08:43:02 2021 -0700 Add new test [PR78969]. gcc/testsuite/ChangeLog: PR tree-optimization/78969 * gcc.dg/tree-ssa/builtin-snprintf-warn-6.c: New test.
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 --- Comment #15 from Martin Sebor --- (In reply to Martin Liška from comment #14) > Martin: Can you please take a look at the last comment? There is very little to go on in comment #13. Please provide a complete test case that reproduces the warning you think is a false positive. The warning for the test case in comment #0 and comment #4 was fixed by r257857 in GCC 8.0.1: r257857 | law | 2018-02-20 13:59:22 -0500 (Tue, 20 Feb 2018) | 8 lines PR middle-end/82123 PR tree-optimization/81592 PR middle-end/79257 * gimple-ssa-sprintf.c (format_integer): Query EVRP range analyzer for range data rather than using global data. * gcc.dg/pr81592.c: New test. * gcc.dg/pr82123.c: New test. The test cases in comment #11 and comment #12 still trigger a warning with GCC 9 so I'm tempted to treat those as separate (and move them to a bug of their own) and resolve this bug as fixed.
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 Martin Liška changed: What|Removed |Added CC||marxin at gcc dot gnu.org --- Comment #14 from Martin Liška --- Martin: Can you please take a look at the last comment?
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 Jeremy Sheaffer changed: What|Removed |Added CC||gcc at sheaffer dot ws --- Comment #13 from Jeremy Sheaffer --- Bug is still present in 8.2.0: snprintf(s[i], 60, "%40s%2d %s by %2d %s", tmp, abs(character_get_y(c[i]) - character_get_y(d->PC)), ((character_get_y(c[i]) - character_get_y(d->PC)) <= 0 ? "North" : "South"), abs(character_get_x(c[i]) - character_get_x(d->PC)), ((character_get_x(c[i]) - character_get_x(d->PC)) <= 0 ? "West" : "East")); The two integer conversion are always <= 2 digits.
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969
Martin Sebor changed:
What|Removed |Added
CC||aldyh at gcc dot gnu.org
--- Comment #12 from Martin Sebor ---
Thanks for the test case. Here's a slightly reduced version of it:
$ cat pr78969.c && gcc -O2 -S -Wall pr78969.c
void f (int, ...);
char d[4];
void g (unsigned i)
{
if (i >= 1000 && i < 1)
__builtin_snprintf (d, 4, "%3d", i / 10); // bogus -Wformat-truncation
else
f (i / 10 % 10);
}
pr78969.c: In function ‘g’:
pr78969.c:8:32: warning: ‘%3d’ directive output may be truncated writing
between 3 and 9 bytes into a region of size 4 [-Wformat-truncation=]
__builtin_snprintf (d, 4, "%3d", i / 10); // bogus -Wformat-truncation
^~~
pr78969.c:8:31: note: directive argument in the range [0, 429496729]
__builtin_snprintf (d, 4, "%3d", i / 10); // bogus -Wformat-truncation
^
pr78969.c:8:5: note: ‘__builtin_snprintf’ output between 4 and 10 bytes into a
destination of size 4
__builtin_snprintf (d, 4, "%3d", i / 10); // bogus -Wformat-truncation
^~~~
It does look like the false positive is due to the same underlying limitation
(the range not being reflected accurately outside the VRP pass). GCC 7 doesn't
warn but GCC 8 does. That's because in GCC 7 the result of the
get_range_info() function is VR_VARYING for the argument while in GCC 8 it
reports a VR_RANGE of [0, 429496729]. So ironically, the false positive is a
side-effect of the improvement in GCC 8's range tracking.
There is work underway to improve the accuracy of the range information even
further that should reduce the rate of these kinds of false positives.
That being said, a few comments:
1) The problem doesn't affect just -Wstringop-truncation but other warnings as
well, including -Wformat-overflow. The latter warning, especially, has proven
to be useful enough despite this limitation that removing either from -Wall
doesn't seem a like good solution.
2) The philosophy behind -Wstringop-truncation is based on the assumption that
snprintf() is being called because truncation is possible, and that most
programs aren't prepared to handle it correctly.
In the test case, since truncation isn't possible, calling snprintf() is
unnecessary (and sprintf() would be sufficient -- though calling sprintf with a
fixed-size buffer just big enough for the output would also cause a false
positive). Otherwise, if truncation were possible, the expectation is that the
caller should detect it by testing the return value from the function and
taking some action (e.g., by aborting).
Until the range improvements I mention above are made, I suggest to assume that
snprintf can truncate and handle the truncation somehow. In comparison to the
runtime cost of the snprintf call, the overhead of checking the return value
and aborting is negligible.
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969
John Lindgren changed:
What|Removed |Added
CC||john at jlindgren dot net
--- Comment #11 from John Lindgren ---
I think I am hitting the same issue in 8.1.0. Input ranges are thoroughly
checked in the below code, and there is no possibility of truncation, yet a
warning is emitted.
>From reading this and other reports, it seems to come down to random chance
whether -Wformat-truncation is reliable or not, as it depends on range data in
the optimizer that was never intended for error-checking diagnostics.
Can you please remove -Wformat-truncation from -Wall until it actually works?
#include
void format_time (char buf[7], int time)
{
if (time < 0) time = 0;
if (time > 359) time = 359;
if (time < 6000)
snprintf (buf, 7, " %02d:%02d", time / 60, time % 60);
else if (time < 6)
snprintf (buf, 7, "%3d:%02d", time / 60, time % 60);
else
snprintf (buf, 7, "%3d:%02d", time / 3600, time / 60 % 60);
}
$ gcc -Wall -O2 -c test.c
test.c: In function ‘format_time’:
test.c:11:32: warning: ‘%02d’ directive output may be truncated writing 2 bytes
into a region of size between 1 and 3 [-Wformat-truncation=]
snprintf (buf, 7, "%3d:%02d", time / 60, time % 60);
^~~~
test.c:11:27: note: directive argument in the range [0, 59]
snprintf (buf, 7, "%3d:%02d", time / 60, time % 60);
^~
test.c:11:9: note: ‘snprintf’ output between 7 and 9 bytes into a destination
of size 7
snprintf (buf, 7, "%3d:%02d", time / 60, time % 60);
^~~
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 Martin Sebor changed: What|Removed |Added Last reconfirmed|2017-05-23 00:00:00 |2018-1-26 Known to fail||8.0 --- Comment #10 from Martin Sebor --- No change in GCC 8.0.
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 Martin Sebor changed: What|Removed |Added CC||shane at paga dot moe --- Comment #9 from Martin Sebor --- *** Bug 80924 has been marked as a duplicate of this bug. ***
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 --- Comment #8 from Jakub Jelinek --- idx_10 addition is a consequence of TODO_update_ssa in vrp1's todo_flags, triggered by jump threading creating the bb6.
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #7 from Jakub Jelinek ---
This is because the PHI at that point is only created during CFG changes at the
end of VRP1 pass.
After creating ASSERT_EXPRs, we still have:
[1.00%]:
goto ; [100.00%]
[99.00%]:
# RANGE [0, 1000] NONZERO 1023
idx_7 = ASSERT_EXPR ;
__builtin_snprintf (p_4(D), 4, "%d", idx_7);
# RANGE [1, 1000] NONZERO 1023
idx_6 = idx_7 + 1;
[100.00%]:
# RANGE [0, 1000] NONZERO 1023
# idx_1 = PHI <0(2), idx_6(3)>
if (idx_1 <= 999)
goto ; [99.00%]
else
goto ; [1.00%]
[1.00%]:
return;
Then VRP1 correctly determines:
idx_1: [0, 1000]
.MEM_2: VARYING
idx_6: [1, 1000]
idx_7: [0, 999] EQUIVALENCES: { idx_1 } (1 elements)
then the ASSERT_EXPRs are removed, which means whenever idx_7 is used we now
use idx_1, and finally the loop is changed, idx_8 and idx_10 is created:
[1.00%]:
goto ; [100.00%]
[99.00%]:
# RANGE [0, 1000] NONZERO 1023
# idx_10 = PHI
__builtin_snprintf (p_4(D), 4, "%d", idx_10);
# RANGE [1, 1000] NONZERO 1023
idx_6 = idx_10 + 1;
[99.00%]:
# RANGE [0, 1000] NONZERO 1023
# idx_1 = PHI
if (idx_1 != 1000)
goto ; [98.99%]
else
goto ; [1.01%]
[1.00%]:
return;
[1.00%]:
# RANGE [0, 1000] NONZERO 1023
# idx_8 = PHI <0(2)>
goto ; [100.00%]
But there is no obvious connection between idx_10 (which indeed nicely could
hold the [0, 999] range) and idx_7 (the already removed ASSERT_EXPR);
similarly, idx_8 could very well have RANGE [0, 0] but doesn't (though in that
case it isn't that big a deal, because it is going to be removed immediately
afterwards).
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 --- Comment #6 from Martin Sebor --- I also found the following discussion about what looks like the same problem: https://patchwork.ffmpeg.org/patch/3630
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969
Martin Sebor changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
Last reconfirmed||2017-05-23
Ever confirmed|0 |1
Known to fail||7.1.0
--- Comment #5 from Martin Sebor ---
(In reply to Sergei Trofimovich from comment #4)
> Found similar false positive on lxc project.
Thanks for the test case. The VRP pass computes the correct range information
for the argument but the range made available outside it via the
get_range_info() function is that of idx_6, not idx_10 below (compile with
-fdump-tree-vrp=/dev/stdout to see the output). It's possible that this
problem is caused by the same underlying limitation as the one in comment #0.
Let me confirm this bug on that basis.
Value ranges after VRP:
idx_1: [1, 999] EQUIVALENCES: { idx_6 } (1 elements)
idx_6: [1, 1000]
idx_10: [0, 999]
.MEM_11: VARYING
Removing basic block 5
f (char * p)
{
int idx;
[1.00%]:
[99.00%]:
# idx_10 = PHI
__builtin_snprintf (p_4(D), 4, "%d", idx_10);
idx_6 = idx_10 + 1;
if (idx_6 != 1000)
goto ; [98.99%]
else
goto ; [1.01%]
[1.00%]:
return;
}
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969
Sergei Trofimovich changed:
What|Removed |Added
CC||slyfox at inbox dot ru
--- Comment #4 from Sergei Trofimovich ---
Found similar false positive on lxc project.
Original snippet of code:
https://github.com/lxc/lxc/blob/5059aae90584d7d80b3494088920da4ba73e2b2a/src/lxc/cgroups/cgfsng.c#L1379-L1395
Simplified version:
$ cat a.c
#include
void f(char * p /* NNN\0" */) {
for (int idx = 0; idx < 1000; idx++) {
// guaranteed to be in [0-999] range
snprintf (p, 4, "%d", idx);
}
}
$ gcc -O2 -c a.c -Wall
a.c: In function 'f':
a.c:6:25: warning: '__builtin___snprintf_chk' output may be truncated before
the last format character [-Wformat-truncation=]
snprintf (p, 4, "%d", idx);
^~~~
/usr/include/bits/stdio2.h:64:10: note: '__builtin___snprintf_chk' output
between 2 and 5 bytes into a destination of size 4
If I change 1000 to 999
for (int idx = 0; idx < 999; idx++) {
no warning will be issued.
Looks like what happens here is that gcc does not distinct between
idx in the for loop itself that has range of [0-999]
and idx outside for loop, which has value range of [1000-1000].
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 --- Comment #3 from Martin Sebor --- Author: msebor Date: Sun Jan 8 23:42:09 2017 New Revision: 244210 URL: https://gcc.gnu.org/viewcvs?rev=244210&root=gcc&view=rev Log: PR tree-optimization/78913 - Probably misleading error reported by -Wformat-length PR middle-end/77708 - -Wformat-length %s warns for snprintf gcc/ChangeLog: PR middle-end/77708 * doc/invoke.texi (Warning Options): Document -Wformat-truncation. * gimple-ssa-sprintf.c (call_info::reval_used, call_info::warnopt): New member functions. (format_directive): Used them. (add_bytes): Same. (pass_sprintf_length::handle_gimple_call): Same. * graphite-sese-to-poly.c (tree_int_to_gmp): Increase buffer size to avoid truncation for any argument. (extract_affine_mul): Same. * tree.c (get_file_function_name): Same. gcc/c-family/ChangeLog: PR middle-end/77708 * c.opt (-Wformat-truncation): New option. gcc/fortran/ChangeLog: PR tree-optimization/78913 PR middle-end/77708 * trans-common.c (build_equiv_decl): Increase buffer size to avoid truncation for any argument. * trans-types.c (gfc_build_logical_type): Same. gcc/testsuite/ChangeLog: PR middle-end/77708 * gcc.dg/tree-ssa/builtin-snprintf-warn-1.c: New test. * gcc.dg/tree-ssa/builtin-snprintf-warn-2.c: New test. * gcc.dg/tree-ssa/builtin-sprintf-warn-6.c: XFAIL test cases failing due to bug 78969. * gcc.dg/format/pr78569.c: Adjust. Added: trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-warn-1.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-snprintf-warn-2.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c.opt trunk/gcc/doc/invoke.texi trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/trans-common.c trunk/gcc/fortran/trans-types.c trunk/gcc/gimple-ssa-sprintf.c trunk/gcc/graphite-sese-to-poly.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/format/pr78569.c trunk/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-6.c trunk/gcc/tree.c
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969 --- Comment #2 from Andrew Pinski --- I think there might be a dup of this bug already. basically VRP does a copy prop of where the assert was. get_range_info is not position sensitive so the range is gone after VRP.
[Bug tree-optimization/78969] bogus snprintf truncation warning due to missing range info
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78969
Martin Sebor changed:
What|Removed |Added
Keywords||diagnostic,
||missed-optimization
--- Comment #1 from Martin Sebor ---
The same underlying problem (lack of range info) can be seen in the VRP dump
for the following test case. The -Walloca-larger-than option is interesting
because the alloca pass that implements it tries to compensate for the missing
range info by deriving it from conditions the alloca() argument is subjected to
(see the alloca_call_type_by_arg function). Although the logic it uses is
quite simple in this case it manages to successfully determine the range on its
own and avoids the warning.
$ cat t.c && gcc -O2 -S -Wall -Walloca-larger-than=255
-fdump-tree-vrp=/dev/stdout t.c
void foo (void*);
void f (unsigned long j)
{
if (j / 256)
return;
foo (__builtin_alloca (j));
}
;; Function f (f, funcdef_no=0, decl_uid=1797, cgraph_uid=0, symbol_order=0)
;; 1 loops found
;;
;; Loop 0
;; header 0, latch 1
;; depth 0, outer -1
;; nodes: 0 1 2 3 4
;; 2 succs { 4 3 }
;; 3 succs { 4 }
;; 4 succs { 1 }
SSA replacement table
N_i -> { O_1 ... O_j } means that N_i replaces O_1, ..., O_j
j_7 -> { j_3(D) }
Incremental SSA update started at block: 2
Number of blocks in CFG: 5
Number of blocks to update: 2 ( 40%)
Value ranges after VRP:
_1: ~[0B, 0B]
.MEM_2: VARYING
j_3(D): VARYING
j_7: [0, 255] EQUIVALENCES: { j_3(D) } (1 elements)
f (long unsigned int j)
{
void * _1;
[100.00%]:
if (j_3(D) > 255)
goto ; [51.01%]
else
goto ; [48.99%]
[48.99%]:
_1 = __builtin_alloca (j_3(D));
foo (_1);
[100.00%]:
return;
}
;; Function f (f, funcdef_no=0, decl_uid=1797, cgraph_uid=0, symbol_order=0)
;; 1 loops found
;;
;; Loop 0
;; header 0, latch 1
;; depth 0, outer -1
;; nodes: 0 1 2 3 4
;; 2 succs { 4 3 }
;; 3 succs { 4 }
;; 4 succs { 1 }
SSA replacement table
N_i -> { O_1 ... O_j } means that N_i replaces O_1, ..., O_j
j_7 -> { j_3(D) }
Incremental SSA update started at block: 2
Number of blocks in CFG: 5
Number of blocks to update: 2 ( 40%)
Value ranges after VRP:
_1: ~[0B, 0B]
.MEM_2: VARYING
j_3(D): VARYING
j_7: [0, 255] EQUIVALENCES: { j_3(D) } (1 elements)
f (long unsigned int j)
{
void * _1;
[100.00%]:
if (j_3(D) > 255)
goto ; [51.01%]
else
goto ; [48.99%]
[48.99%]:
_1 = __builtin_alloca (j_3(D));
foo (_1);
[100.00%]:
return;
}
