[PATCH] avoid issuing -Warray-bounds during folding (PR 88800)

[Richard Biener]

On Thu, Jan 17, 2019 at 2:51 AM Martin Sebor <msebor@gmail.com> wrote:
>
> On 1/16/19 6:14 PM, Jeff Law wrote:
> > On 1/15/19 8:21 AM, Martin Sebor wrote:
> >> On 1/15/19 4:07 AM, Richard Biener wrote:
> >>> On Tue, Jan 15, 2019 at 1:08 AM Martin Sebor <msebor@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).
> > Note we are in stage4, so we're supposed to be addressing regression
> > bugfixes and documentation issues.
> >
> > So  I think Richi needs to be explicit about whether or not he wants
> > this in gcc-9 or if it should defer to gcc-10.
> >
> > I have no technical objections to the patch and would easily ack it in
> > stage1 or stage3.
>
> The warning is a regression introduced in GCC 8.  I was just about
> to commit the fix so please let me know if I should hold off until
> stage 1.

As it fixes a regression it is fine now - I also like the cleaning up
of the helper function to work independently of the -W* state.

Richard.

> Martin