On 9/15/18 2:43 AM, Bernd Edlinger wrote: > Hi, > > this is an update on my strlen range patch (V7). Again re-based and > retested to current trunk. > > I am aware that Martin wants to re-factor the interface of get_range_strlen > and have no objections against, but I'd suggest that to be a follow-up patch. > > I might suggest to rename one of the two get_range_strlen functions at the > same time as it is rather confusing to have to count the parameters in order > to tell which function is meant. > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. > > > changelog-range-strlen-v7.txt > > gcc: > 2018-08-26 Bernd Edlinger <bernd.edlin...@hotmail.de> > > * gimple-fold.c (looks_like_a_char_array_without_typecast_p): New > helper function for strlen range estimations. > (get_range_strlen): Use looks_like_a_char_array_without_typecast_p > for warnings, but use GIMPLE semantics otherwise. > * tree-ssa-strlen.c (maybe_set_strlen_range): Use GIMPLE semantics. > (get_min_string_length): Avoid not NUL terminated string literals. The introduction of looks_like_a_char_array_without_typecast_p is probably a good thing. Too much code is already implemented inline within get_range_strlen.
It looks like you added handling of ARRAY_RANGE_REF. I don't know how often they come up in practice, but handling it seems like a reasonable extension to what we're doing. Bonus points if it's triggering with any kind of consistency. I actually prefer Martin's unification of type/fuzzy into a single enumeration to describe the desired behavior. Doing it with two args where some values are mutually exclusive is just asking for trouble. Though I like that you called out the values that are mutually exclusive. I definitely want to look at how your patch and Martin's differ on the handling of flexible array members -- clearly we must avoid setting a range in that case. I'm surprised this didn't trigger a failure in the testsuite though. Martin's work in this space did. The bugfix in get_min_string_length looks like it probably stands on its own. I'm still evaluating the two approaches... jeff
