[Bug tree-optimization/94021] -Wformat-truncation false positive due to excessive integer range
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94021 Andrew Pinski changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED Target Milestone|--- |13.0 --- Comment #11 from Andrew Pinski --- GCC 13: # RANGE [irange] int [0, 59] NONZERO 0x3f m_10 = _1 / 60; # RANGE [irange] int [0, 24] NONZERO 0x1f h_9 = _5 / 3600; GCC 12: # RANGE [0, 59] NONZERO 63 m_10 = _1 / 60; # RANGE [0, 596523] NONZERO 1048575 h_9 = _5 / 3600; And the warning is gone in GCC 13 too. And produces: /app/example.cpp:14: snprintfD.1756: objsize = 8, fmtstr = "%s%02i%02i" Directive 1 at offset 0: "%s" Result: 1, 1, 1, 1 (1, 1, 1, 1) Directive 2 at offset 2: "%02i" Result: 2, 2, 2, 2 (3, 3, 3, 3) Directive 3 at offset 6: "%02i" Result: 2, 2, 2, 2 (5, 5, 5, 5) Directive 4 at offset 10: "", length = 1 Substituting 5 for return value.
[Bug tree-optimization/94021] -Wformat-truncation false positive due to excessive integer range
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94021 --- Comment #10 from ishikawa,chiaki --- It would be great if the problem is fixed in later versions. I observe the error with gcc-12 on my computer yet. *BUT* compiling with -O instead of -O2 succeeds !? gcc-12 version. gcc-12 (Debian 12.2.0-14) 12.2.0 Copyright (C) 2022 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. No error with -O !? ishikawa@ip030:/NREF-COMM-CENTRAL/mozilla$ gcc-12 -c -O -Wall /tmp/pr94021.c Error with -O2 as before. ishikawa@ip030:/NREF-COMM-CENTRAL/mozilla$ gcc-12 -c -O2 -Wall /tmp/pr94021.c /tmp/pr94021.c: In function ‘format_utc_offset’: /tmp/pr94021.c:14:45: warning: ‘%02i’ directive output may be truncated writing 2 bytes into a region of size between 1 and 5 [-Wformat-truncation=] 14 | __builtin_snprintf (a, sizeof a, "%s%02i%02i", "+", h, m); | ^~~~ /tmp/pr94021.c:14:38: note: directive argument in the range [0, 59] 14 | __builtin_snprintf (a, sizeof a, "%s%02i%02i", "+", h, m); | ^~~~ /tmp/pr94021.c:14:5: note: ‘__builtin_snprintf’ output between 6 and 10 bytes into a destination of size 8 14 | __builtin_snprintf (a, sizeof a, "%s%02i%02i", "+", h, m); | ^ ishikawa@ip030:/NREF-COMM-CENTRAL/mozilla$
[Bug tree-optimization/94021] -Wformat-truncation false positive due to excessive integer range
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94021
--- Comment #9 from Andrew Macleod ---
(In reply to Jakub Jelinek from comment #6)
> CCing Andrew and Aldy to see what the ranger does or can do, talking about
> I mean, if we have:
> h_1 = x_2 / 3600;
> if (x_2 <= -3599 && x_2 <= 8)
> use (h_1);
> figure out that h_1 is set to x_2 / 3600 and even when that
> SSA_NAME_DEF_STMT is not in a guarded block, its use is in one and so from
> the [-3599, 8] range of x_2 at the point of use derive that h_1 there is
> [0, 24]?
> Surely if it is like:
> if (x_2 <= -3599 && x_2 <= 8)
> {
> h_1 = x_2 / 3600;
> use (h_1);
> }
> I'd expect it to handle it that way.
Certainly we get the latter case. The earlier case is currently...
inconsistent. Something I hope to address in the next release.
if we have precisely:
h_1 = x_2 / 3600;
if (x_2 <= -3599 && x_2 <= 8)
use (h_1);
and if that is calculated in such a way that all of the conditions are
evaluated in a single basic block, then the GORI engine well mark h1 and x_2
both as exports and the evaluator will calculate the desired value for h_1.
Once we start to pull them further apart, the current implementation loses the
ability to recalculate h_1 when we get new ranges for x_2.
I have plans to segregate the def chains from the export lists in blocks, and
allow for greater ability to recalculate things like this.
When I look at #c4 in EVRP, I see:
=== BB 4
:
# x_10 = PHI
h_15 = x_10 / 3600;
_1 = x_10 % 3600;
m_16 = _1 / 60;
h.0_2 = (unsigned int) h_15;
_3 = h.0_2 > 23;
_5 = _3;
if (_5 != 0)
goto ; [INV]
else
goto ; [INV]
_1 : int [0, 3599]
h.0_2 : unsigned int [0, 596523]
x_10 : int [0, +INF]
h_15 : int [0, 596523]
m_16 : int [0, 59]
4->6 (T) h.0_2 : unsigned int [0, 596523]
4->6 (T) _5 : _Bool [1, 1]
4->6 (T) x_10 :int [0, +INF]
4->6 (T) h_15 :int [0, 596523]
4->5 (F) h.0_2 : unsigned int [0, 596523]
4->5 (F) _5 : _Bool [0, 0]
4->5 (F) x_10 :int [0, +INF]
4->5 (F) h_15 :int [0, 596523]
and then later on:
=== BB 8
x_10int [0, +INF]
:
if (x_10 <= 8)
goto ; [INV]
else
goto ; [INV]
8->9 (T) x_10 :int [0, 8]
8->10 (F) x_10 : int [9, +INF]
=== BB 9
:
__builtin_snprintf (, 8, "%s%02i%02i", "+", h_15, m_16);
The defchains already indicate that h_15 is dependant on the value of x_10, and
I am hoping to enable recalculation of h_15 when a dependant range has
changed.. and not just when they are exported from the same block.
so in this case, when we ask for the range of h_15 in BB_9, we should be able
to see that x_10 has a range of int [0, 8] and trigger a recalculation of
h_15 using "current" values. and come up with h_15 = [0,24]
The pieces are all there, but they need to be assembled in a non time consuming
way :-)
It is on the radar for next release.
[Bug tree-optimization/94021] -Wformat-truncation false positive due to excessive integer range
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94021 --- Comment #8 from Martin Sebor --- *** Bug 98281 has been marked as a duplicate of this bug. ***
[Bug tree-optimization/94021] -Wformat-truncation false positive due to excessive integer range
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94021 --- Comment #7 from Martin Sebor --- Since the merge with the sprintf pass, the strlen pass has an instance of EVRP that it passes to sprintf to get range info from (it also uses it itself in places).
[Bug tree-optimization/94021] -Wformat-truncation false positive due to excessive integer range
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94021
Jakub Jelinek changed:
What|Removed |Added
CC||aldyh at gcc dot gnu.org,
||amacleod at redhat dot com
--- Comment #6 from Jakub Jelinek ---
CCing Andrew and Aldy to see what the ranger does or can do, talking about #c4.
So, we could:
--- gcc/match.pd.jj 2020-02-15 12:52:27.800034587 +0100
+++ gcc/match.pd2020-03-04 12:25:12.744289050 +0100
@@ -1551,11 +1551,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* X / C1 op C2 into a simple range test. */
(for cmp (simple_comparison)
(simplify
- (cmp (trunc_div:s @0 INTEGER_CST@1) INTEGER_CST@2)
+ (cmp (trunc_div@3 @0 INTEGER_CST@1) INTEGER_CST@2)
(if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
&& integer_nonzerop (@1)
&& !TREE_OVERFLOW (@1)
- && !TREE_OVERFLOW (@2))
+ && !TREE_OVERFLOW (@2)
+ && single_use (@3))
(with { tree lo, hi; bool neg_overflow;
enum tree_code code = fold_div_compare (cmp, @1, @2, , ,
_overflow); }
and thus not optimize those
h_1 = x_2 / 3600;
h_1 <= 24;
and
h_1 >= 0;
into
x_2 <= 8;
and
x_2 >= -3599;
because we have another h_1 use. But, unfortunately that would pessimize the
case where there are multiple uses, but all of them can be optimized this way
(e.g. like that h_1 <= 24 and h_1 >= 0 uses), we'd keep a useless division.
I'd hope that with this optimization disabled or on #c5 ranger can handle the
testcase right (assuming the warning uses the ranger APIs), but the question is
can or could ranger handle even the case after this fold div compare
optimization? I mean, if we have:
h_1 = x_2 / 3600;
if (x_2 <= -3599 && x_2 <= 8)
use (h_1);
figure out that h_1 is set to x_2 / 3600 and even when that SSA_NAME_DEF_STMT
is not in a guarded block, its use is in one and so from the [-3599, 8]
range of x_2 at the point of use derive that h_1 there is [0, 24]?
Surely if it is like:
if (x_2 <= -3599 && x_2 <= 8)
{
h_1 = x_2 / 3600;
use (h_1);
}
I'd expect it to handle it that way.
[Bug tree-optimization/94021] -Wformat-truncation false positive due to excessive integer range
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94021
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #5 from Jakub Jelinek ---
I don't understand why EVRP would be relevant, strlen pass runs after VRP1.
Anyway, I see two issues. One general problem with SSA_NAME_RANGE_INFO
applying to SSA_NAME and thus it needs to cover ranges in all possible spots
the SSA_NAME is set or used, where during VRP where we have ASSERT_EXPRs we
find the right range but as it is just used in the bb guarded by the
conditional, there is nothing to stick it on (as compared to e.g. if the
sprintf was using h + 1 instead of h, where on the SSA_NAME holding h + 1 we
could add SSA_NAME_RANGE_INFO). For this consider e.g.
char a[8];
void format_utc_offset (int x, int h)
{
if (x < 0)
x = -x;
int m = (x % 3600) / 60;
if (h < 0 || h >= 24 || m < 0 || m >= 60)
__builtin_puts ("");
if (0 <= m && m <= 60 && 0 <= h && h <= 24)
__builtin_snprintf (a, sizeof a, "%s%02i%02i", "+", h, m);
}
where it warns too. This is something that the value ranger work by
Andrew/Aldy could help with.
The other, on the #c4 case, is that match.pd seems to optimize e.g. the h <= 24
into x <= 24 * 3600 + 3599. And that is really weird, because:
/* X / C1 op C2 into a simple range test. */
(for cmp (simple_comparison)
(simplify
(cmp (trunc_div:s @0 INTEGER_CST@1) INTEGER_CST@2)
as it uses :s and so in my understanding shouldn't match because the h SSA_NAME
has multiple uses (e.g. is also used in the snprintf call).
And I bet this problem is what would prevent even the ranger for determining
something useful in this case.
[Bug tree-optimization/94021] -Wformat-truncation false positive due to excessive integer range
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94021
Martin Sebor changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
Keywords||diagnostic
Last reconfirmed||2020-03-03
Component|c |tree-optimization
CC||msebor at gcc dot gnu.org
Blocks||85741
Ever confirmed|0 |1
Summary|-Werror=format-truncation= |-Wformat-truncation false
|seems to cause incorrect|positive due to excessive
|warning, thus error.|integer range
Known to fail||10.0, 9.2.0
--- Comment #4 from Martin Sebor ---
Confirmed with the small test case below. The warning code works correctly but
the integer conditionals are somehow confusing EVRP into thinking h's range is
[0, 596523], which causes the warning to trigger. The dump shows that the
warning thinks the result of the first %02i directive is between 2 and 6
characters:
Directive 2 at offset 2: "%02i"
Result: 2, 6, 6, 6 (3, 7, 7, 7)
$ cat pr94021.c && gcc -O2 -S -Wall -fdump-tree-strlen=/dev/stdout pr94021.c
char a[8];
void format_utc_offset (int x)
{
if (x < 0)
x = -x;
int h = x / 3600, m = (x % 3600) / 60;
if (h < 0 || h >= 24 || m < 0 || m >= 60)
__builtin_puts ("");
if (0 <= m && m <= 60 && 0 <= h && h <= 24)
__builtin_snprintf (a, sizeof a, "%s%02i%02i", "+", h, m);
}
;; Function format_utc_offset (format_utc_offset, funcdef_no=0, decl_uid=1931,
cgraph_uid=1, symbol_order=1)
;; 1 loops found
;;
;; Loop 0
;; header 0, latch 1
;; depth 0, outer -1
;; nodes: 0 1 2 3 4 5 6
;; 2 succs { 3 6 }
;; 3 succs { 6 }
;; 4 succs { 5 }
;; 5 succs { 1 }
;; 6 succs { 4 5 }
pr94021.c:14: __builtin_snprintf: objsize = 8, fmtstr = "%s%02i%02i"
Directive 1 at offset 0: "%s"
Result: 1, 1, 1, 1 (1, 1, 1, 1)
Directive 2 at offset 2: "%02i"
Result: 2, 6, 6, 6 (3, 7, 7, 7)
Directive 3 at offset 6: "%02i"
pr94021.c: In function ‘format_utc_offset’:
pr94021.c:14:45: warning: ‘%02i’ directive output may be truncated writing 2
bytes into a region of size between 1 and 5 [-Wformat-truncation=]
14 | __builtin_snprintf (a, sizeof a, "%s%02i%02i", "+", h, m);
| ^~~~
pr94021.c:14:38: note: directive argument in the range [0, 59]
14 | __builtin_snprintf (a, sizeof a, "%s%02i%02i", "+", h, m);
| ^~~~
Result: 2, 2, 2, 2 (5, 9, 9, 9)
Directive 4 at offset 10: "", length = 1
pr94021.c:14:5: note: ‘__builtin_snprintf’ output between 6 and 10 bytes into a
destination of size 8
14 | __builtin_snprintf (a, sizeof a, "%s%02i%02i", "+", h, m);
| ^
Moving the test for hours etc. being out-of-bounds after the test for them
being in-bounds avoids the warning (and streamlines the code so it seems like
an overall improvement).
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85741
[Bug 85741] [meta-bug] bogus/missing -Wformat-overflow
