On 08/21/2018 10:15 PM, Bernd Edlinger wrote: > On 08/22/18 00:43, Jeff Law wrote: >> [ I'm still digesting, but saw something in this that ought to be broken >> out... ] >> >> On 08/19/2018 09:55 AM, Bernd Edlinger wrote: >>> diff -Npur gcc/tree-ssa-dse.c gcc/tree-ssa-dse.c >>> --- gcc/tree-ssa-dse.c 2018-07-18 21:21:34.000000000 +0200 >>> +++ gcc/tree-ssa-dse.c 2018-08-19 14:29:32.344498771 +0200 >>> @@ -248,6 +248,12 @@ compute_trims (ao_ref *ref, sbitmap live >>> residual handling in mem* and str* functions is usually >>> reasonably efficient. */ >>> *trim_tail = last_orig - last_live; >>> + /* Don't fold away an out of bounds access, as this defeats proper >>> + warnings. */ >>> + if (*trim_tail >>> + && compare_tree_int (TYPE_SIZE_UNIT (TREE_TYPE (ref->base)), >>> + last_orig) <= 0) >>> + *trim_tail = 0; >>> } >>> else >>> *trim_tail = 0; >> This seems like a good change in and of itself and should be able to go >> forward without further review work. Consider this hunk approved, >> along with any testsuite you have which tickles this code (I didn't >> immediately see one attached to this patch. But I could have missed it). >> > > Sorry, for not being clear on this. > > I needed both hunks "Don't fold away an out of bounds access, as this defeats > proper > warnings" to prevent a regression on gcc.dg/Wstringop-overflow-5.c, > and surprise surprise, the xfail in gcc.dg/Wstringop-overflow-6.c suddenly > popped up. > > So without the unsafe range info, gcc.dg/Wstringop-overflow-5.c needs both > hunks > to not regress, but gcc.dg/Wstringop-overflow-6.c only needs the other one I > committed > yesterday. > > So unfortunately I have no test case except gcc.dg/Wstringop-overflow-5.c for > that. > > > Still OK? I almost had a WTF moment mis-parsing Wstringop-overflow-5 thinking it had no dead stores. But it's full of dead stores :-)
It sounds like the testsuite will tickle it when the right set of patches are applied. THere is no distinct test right now. I went ahead and bootstrapped/regression tested the DSE change alone and it doesn't trigger any regressions. I'm going to install it -- I think consensus has formed around not folding an out of bounds access so that we can detect the problem and issue a suitable warning. One less thing to keep track of :-) jeff
