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