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 <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). 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. Jeff
