On 3/5/20 12:51 AM, Richard Biener wrote:
On Thu, Mar 5, 2020 at 12:49 AM Jeff Law <l...@redhat.com> wrote:
On Wed, 2020-03-04 at 09:22 -0700, Martin Sebor wrote:
I don't remember why the code in -Wrestrict unconditionally overwrites
the statement location rather than only when it's not available, but
I do remember adding conditional code like in your patch in r277076
to deal with missing location on the statement. So either your fix
or something like the hunk below might be the right solution (if we
go with the code below, abstracting it into a utility function might
be nice).
So there's several chunks that are fairly similar to what you referenced in
maybe_warn_pointless_strcmp. Factoring all of them into a single location is
pretty easy.
That also gives us a nice place where we can experiment with "does extraction of
location information from the expression ever help". The answer is, it doesn't,
at least not within our testsuite when run on x86_64.
I'm hesitant to remove the code that extracts the location out of the
expression,
but could be convinced to do so.
Thoughts?
Using anything but the actual stmt location is prone to end up at random places
due to tree sharing issues, CSE and copy propagation. Simply consider
char one[50];
char two[50];
void
test_strncat (void)
{
char *p = one;
(void) __builtin_strcpy (p, "gh");
(void) __builtin_strcpy (two, "ef");
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstringop-overflow="
#pragma GCC diagnostic ignored "-Warray-bounds"
(void) __builtin_strncat (p, two, 99);
Interestingly, while the expression location points to p, the warning
points to the statement:
warning: ‘__builtin_strncat’ forming offset [50, 98] is out of the
bounds [0, 50] of object ‘one’ with type ‘char[50]’ [-Warray-bounds]
14 | (void) __builtin_strncat (p, two, 99);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
As it happens, the %G directive in the warning_at() call replaces
the location passed to it with that of the Gimple call argument to
the %G directive. Removing the %G directive turns the warning into:
warning: ‘__builtin_strncat’ forming offset [50, 98] is out of the
bounds [0, 50] of object ‘one’ with type ‘char[50]’ [-Warray-bounds]
7 | char *p = one;
| ^~~
But the code that checks the scope of #pragma GCC diagnostic uses
the original location passed to warning_at, not the location set
subsequently by the %G directive, and so the two are out of synch.
We've discussed removing the %G/%K directives before and having
the diagnostic machinery always print the inlining context instead.
Let me look into it for GCC 11.
Martin
#pragma GCC diagnostic pop
}
where we happily forward p = &one[0] to both uses injecting
a "faulty" location. Well, it's actually the correct location
computing &one[0] but irrelevant for the actual call.
So the question is why we end up with UNKNOWN_LOCATION
for such call and if why we need to bother emit a diagnostic
at all (and why emitting it for another possibly random location is a good idea
instead of maybe simply emitting it without location).
Richard.
Jeff
