On 01/08/2018 04:28 AM, Richard Biener wrote:
On Sat, Jan 6, 2018 at 11:04 PM, Martin Sebor <mse...@gmail.com> wrote:
Bug 83671 - Fix for false positive reported by -Wstringop-overflow
does not work at -O1, points out that the string length range
optimization implemented as a solution for bug 83373 doesn't help
at -O1. The root cause is that the fix was added to the strlen
pass that doesn't run at -O1.
The string length range computation doesn't depend on the strlen
pass, and so the range can be set earlier, in gimple-fold, and
its results made available even at -O1. The attached patch
changes the gimple_fold_builtin_strlen() function to do that.
While testing the change I came across a number of other simple
strlen cases that currently aren't handled, some at -O1, others
at all. I added code to handle some of the simplest of them
and opened bugs to remind us/myself to get back to the rest in
the future (pr83693 and pr83702). The significant enhancement
is handling arrays of arrays with non-constant indices and
pointers to such things, such as in:
char a[2][7];
void f (int i)
{
if (strlen (a[i]) > 6) // eliminated with the patch
abort ();
}
Attached is a near-minimal patch to handle PR 83671.
Please don't use set_range_info form insinde fold_stmt (), this is
IMHO a layering violation.
Why not restrict -Wstrinop-overflow to -O2+?
For simple cases, -Wstrinop-overflow works even without
optimization. Given the notoriety of buffer overflows and
because not all software is or can always be compiled with
-O2 or even with -O1, I think it's important to keep it working.
I initially did consider resolving PR 83671 as won't fix because
at -O1 neither the strlen pass not VRP runs. But as I explained
above, because the range doesn't depend on the strlen pass and
can be determined and set anywhere, regardless of the optimization
level, doing it outside the pass seems like a win-win. Exposing
the range early improves downstream transformations before strlen
runs or even when it doesn't, and as a bonus, the warning also
disappears.
But I don't understand the concern. The range is only set when
the expression is an SSA_NAME, and setting in this function here
seems no different than setting it in one if its callers (which
may be, indirectly. in a pass like strlen). If you have
a suggestion for a better place to set it I'd be happy to move
it, I just don't see the problem.
In case it wasn't clear from my description (and got obscured
by the example above), this change is not about just arrays of
arrays (that's a minor improvement I included in it). It's
bout letting GCC optimize all strlen(array) expressions at -O1,
such as the following:
char a[32];
void f (void)
{
if (strlen (a) > 31)
abort ();
}
Martin
