Re: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478, take 2 and 3)

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Feb 20, 2018 at 08:56:11PM -0700, Jeff Law wrote:
> On 02/20/2018 02:34 PM, Jakub Jelinek wrote:
> > On Tue, Feb 20, 2018 at 01:13:13PM -0700, Martin Sebor wrote:
> >> A safer and even more conservative alternative that should be
> >> equivalent to your approach while avoiding the sprintf regressions
> >> is to add another mode to the function and have it clear *minlen
> >> as an option.  This lets the strlen code obtain the conservative
> >> lower bound without compromising the sprintf warnings.
> > 
> > I fail to see what it would be good for to set *MINLEN to zero and
> > *MAXLEN to all ones for the non-warning use cases, we simply don't know
> > anything about it, both NULL_TREEs i.e. returning false is better.  I'm
> > offering two alternate patches which use
> > fuzzy == 0 for the previous !fuzzy, fuzzy == 1 for conservatively correct
> > code that assumes strlen can't cross field/variable boundaries in
> > compliant programs and fuzzy == 2 which does that + whatever the warning
> > code wants.  Additionally, I've rewritten the COND_EXPR handling, so that
> > it matches exactly the PHI handling.
> > 
> > The first patch doesn't change the 2 argument get_range_strlen and changes
> > gimple_fold_builtin_strlen to use the 6 argument one, the second patch
> > changes also the 2 argument get_range_strlen similarly to what you've done
> > in your patch.
> > 
> > Tested on x86_64-linux and i686-linux, ok for trunk if it passes
> > bootstrap/regtest?  Which one?
> I'd lean towards the second -- essentially trying to keep the 6 operand
> version for internal (recursive) use only.
> 
> In  my mind I'd like to encapsulate the 6 operand version so that it
> can't be directly called and the only public interface is the 3 operand
> version using strict/no-strict.
> 
> Let's go with that.  We can try to clean this up further during gcc-9.

Ok, committed the second patch after bootstrapping/regtesting it on
x86_64-linux and i686-linux.

There is one thing that might still be desirable to change, because right
now the default arg for the non-internal get_range_strlen is the warning
non-conservative one, so if somebody adds another use for code generation it
might break stuff again.  Perhaps we should either flip the default or make
the third argument a required argument and adjust all the 6 callers.

	Jakub