On 10/12/18 16:55, Jeff Law wrote: > 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 did only want to be consistent with get_inner_reference here, but did not have encountered these, probably only an Ada thing? > 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... > One thing I should mention is, that there is still one place where opportunistic range info influence conde gen. I mean at least with my patch. That is the return value from sprintf is using the range info from the warning, and uses that to set the range info of the result. In try_substitute_return_value, which uses the range info that was from the warnings and feeds that into set_range_info. Bernd. > jeff >
