On 02/13/2017 04:33 PM, Jeff Law wrote:
On 02/10/2017 10:55 AM, Martin Sebor wrote:
The recent Fedora mass rebuild revealed that the Wformat-truncation=2
checker is still a bit too aggressive and complains about potentially
unbounded strings causing subsequent directives t exceed the INT_MAX
limit. (It's unclear how the build ended up enabling level 2 of
the warning.)
This is because for the purposes of the return value optimization
the pass must assume that such strings really are potentially unbounded
and result in as many as INT_MAX bytes (or more). That doesn't mean
that it should warn on such cases.
The attached patch relaxes the checker to avoid the warning in this
case. Since there's no easy way for a user to suppress the warning,
is this change okay for trunk at this stage?
Martin
gcc-79448.diff
PR middle-end/79448 - unhelpful -Wformat-truncation=2 warning
gcc/testsuite/ChangeLog:
PR middle-end/79448
* gcc.dg/tree-ssa/builtin-snprintf-warn-3.c: New test.
* gcc.dg/tree-ssa/pr79448-2.c: New test.
* gcc.dg/tree-ssa/pr79448.c: New test.
gcc/ChangeLog:
PR middle-end/79448
* gimple-ssa-sprintf.c (format_directive): Avoid issuing INT_MAX
warning for strings of unknown length.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index e6cc31d..bf76162 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -2561,11 +2561,15 @@ format_directive (const
pass_sprintf_length::call_info &info,
/* Raise the total unlikely maximum by the larger of the maximum
and the unlikely maximum. It doesn't matter if the unlikely
maximum overflows. */
+ unsigned HOST_WIDE_INT save = res->range.unlikely;
if (fmtres.range.max < fmtres.range.unlikely)
res->range.unlikely += fmtres.range.unlikely;
else
res->range.unlikely += fmtres.range.max;
+ if (res->range.unlikely < save)
+ res->range.unlikely = HOST_WIDE_INT_M1U;
+
So this looks like you're doing an overflow check -- yet earlier your
comment says "It doesnt' matter if the unlikely maximum overflows". ISTM
that comment needs updating -- if it doesn't matter, then why check for
it and clamp the value?
res->range.min += fmtres.range.min;
res->range.likely += fmtres.range.likely;
@@ -2616,7 +2620,12 @@ format_directive (const
pass_sprintf_length::call_info &info,
/* Has the likely and maximum directive output exceeded INT_MAX? */
bool likelyximax = *dir.beg && res->range.likely > target_int_max ();
- bool maxximax = *dir.beg && res->range.max > target_int_max ();
+ /* Don't consider the maximum to be in excess when it's the result
+ of a string of unknown length (i.e., whose maximum has been set
+ to HOST_WIDE_INT_M1U. */
+ bool maxximax = (*dir.beg
+ && res->range.max > target_int_max ()
+ && res->range.max < HOST_WIDE_INT_MAX);
So your comment mentions HOST_WIDE_INT_M1U as the key for a string of
unknown length. But that doesn't obviously correspond to what the code
checks.
Can you please fix up the two comments. With the comments fixed, this
is OK.
Sure, I updated the comments.
The code alternately uses HOST_WIDE_INT_M1U and HOST_WIDE_INT_MAX as
a stand-in for either a "can't happen" or "unbounded/unknown" size.
It's not fully consistent and should be cleaned up and the uses of
HOST_WIDE_INT should be replaced by a class like wide_int as someone
suggested in the past. If I get to some of the enhancements I'd like
to make in stage 1 (e.g., integrating the pass with tree-ssa-strlen)
I'll see about cleaning this up.
Martin
