On Wed, Jun 13, 2018 at 09:46:28AM +0200, Richard Biener wrote:
> > > > gcc/ChangeLog:
> > > >
> > > > PR c/85931
> > > > * fold-const.c (operand_equal_p): Handle SAVE_EXPR.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > PR c/85931
> > > > * gcc.dg/Wstringop-truncation-3.c: New test.
> > > OK for the trunk. Richi and Jakub have the final say for the branch.
> > >
> > > I'm a bit surprised that you don't just use operand_equal_p for both the
> > > INTEGER_CST and !INTEGER_CST cases when testing dstsz == srcsz
> >
> > It seemed that since the CST case compared the values of the size
> > expressions and the other case their lexicographic form they needed
> > to be different. But operand_equal_p() does value comparison for
> > constants so the conditional can be simplified by using just it
> > for both cases. Thanks for the suggestion! I've made that
> > simplification and committed r261515.
> >
> > Jakub/Richard, can you please review the commit and let me know
> > if you have any concerns with backporting it to the GCC 8 branch?
> > (I will proceed if I don't hear anything this week.)
>
> I'm fine with backporting it.
I'm actually worried about the fold-const.c change and don't understand, why
it has been done at all in the context of this PR.
case SAVE_EXPR:
if (flags & OEP_LEXICOGRAPHIC)
return OP_SAME (0);
return 0;
means it does something different from the state before the patch:
default:
return 0;
only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the
-Wduplicated-branches warning code, so why it is needed for
-Wstringop-truncation warning is unclear to me. More importantly,
especially -fsanitize=undefined has a tendency of creating trees
which contain two or more uses of the same SAVE_EXPR at each level, and very
deep nesting levels of such SAVE_EXPRs, so if we handle it without checking
for duplicates, it results in exponential compile time complexity.
So, if we really want to handle SAVE_EXPR here, we'd need to create some
cache where we'd remember if in the same toplevel operand_equal_p call we've
already compared two particular SAVE_EXPRs and what was the result.
Jakub
