On 1/15/19 4:07 AM, Richard Biener wrote:
On Tue, Jan 15, 2019 at 1:08 AM Martin Sebor <mse...@gmail.com> wrote:
The gimple_fold_builtin_memory_op() function folds calls to memcpy
and similar to MEM_REF when the size of the copy is a small power
of 2, but it does so without considering whether the copy might
write (or read) past the end of one of the objects. To detect
these kinds of errors (and help distinguish them from -Westrict)
the folder calls into the wrestrict pass and lets it diagnose them.
Unfortunately, that can lead to false positives for even some fairly
straightforward code that is ultimately found to be unreachable.
PR 88800 is a report of one such problem.
To avoid these false positives the attached patch adjusts
the function to avoid issuing -Warray-bounds for out-of-bounds
calls to memcpy et al. Instead, the patch disables the folding
of such invalid calls (and only those). Those that are not
eliminated during DCE or other subsequent passes are eventually
diagnosed by the wrestrict pass.
Since this change required removing the dependency of the detection
on the warning options (originally done as a micro-optimization to
avoid spending compile-time cycles on something that wasn't needed)
the patch also adds tests to verify that code generation is not
affected as a result of warnings being enabled or disabled. With
the patch as is, the invalid memcpy calls end up emitted (currently
they are folded into equally invalid MEM_REFs). At some point,
I'd like us to consider whether they should be replaced with traps
(possibly under the control of as has been proposed a number of
times in the past. If/when that's done, these tests will need to
be adjusted to look for traps instead.
Tested on x86_64-linux.
I've said in the past that I feel delaying of folding is wrong.
To understand, the PR is about emitting a warning for out-of-bound
accesses in a dead code region?
Yes. I am keeping in my mind your preference of not delaying
the folding of valid code.
If we think delaying/disablign the folding is the way to go the
patch looks OK.
I do, at least for now. I'm taking this as your approval to commit
the patch (please let me know if you didn't mean it that way).
Martin
