On 01/11/2018 04:24 PM, Jeff Law wrote:
On 01/10/2018 01:26 PM, Martin Sebor wrote:
To avoid issuing duplicate warnings for the same function call
in the source code the -Wrestrict warning code makes sure
the no-warning bit is propagated between trees and GIMPLE and
tested before issuing a warning. But the warning also detects
some of the same problems as -Wstringop-overflow, and that
warning was not updated to pay attention to the no-warning bit.
This can in turn lead to two warnings for what boils down to
the same bug. The warnings can be confusing when the first
one references the function as it appears in the source code
and the second one the one it was transformed to by GCC after
the first warning was issued.
The attached patch corrects this oversight by having
the buffer overflow checker test the no-warning bit and skip
issuing a diagnostic. (The function that does the overflow
checking still runs so that it can continue to indicate to its
callers whether an overflow has been detected.)
Bootstrap on x86_64-linux in progress.
Martin
gcc-83508.diff
PR other/83508 - c-c++-common/Wrestrict.c fails since r255836
gcc/testsuite/ChangeLog:
PR other/83508
* gcc.dg/Wstringop-overflow-2.c: New test.
gcc/ChangeLog:
PR other/83508
* builtins.c (check_access): Avoid warning when the no-warning bit
is set.
Would it be better to check for TREE_NO_WARNING at the very start of
check_access rather than sprinkling it at various sites later in the code?
The function is called from a couple of places to check that
there is no overflow and avoid expansion (we discussed this
not too long ago when I enhanced compute_objsize() to handle
ranges). I didn't want to change that (I mention it in the
last sentence above.)
Looking forward, I would like to see these middle-end checkers
used to avoid making certain kinds of transformations that we
know are dangerous (we discussed emitting __builtin_trap or
__builtin_unreachable) for some such cases. I realize they
aren't suitable for it quite it yet and will need work to make
them so (assuming we can agree on that approach), but I mention
it to explain what I was thinking when I sprinkled the tests
in check_access() the way I did.
As an aside, even though I think GCC should issue only one
warning per function, I'm not sure that -Wrestrict should
trump -Wstringop-overflow. It seems like it should be the
other way around because the latter is more severe. That's
also how it worked before -Wrestrict was moved into its own
pass. To make it work like that again, -Wstringop-overflow
would need to run either before -Wrestrict or at the same
time. Maybe it should be moved in GCC 9.
Martin
