Please review writeup for fixing PR 78809 (inline strcmp for small constant strings)

[Martin Sebor]

On 11/16/2017 03:39 PM, Qing Zhao wrote:
> Hi, Jeff,
> thanks a lot for your comments. please see my reply in below:
>
>
>> On Nov 16, 2017, at 12:47 PM, Jeff Law <law@redhat.com> wrote:
>>
>>>
>>>   B. for strncmp (s1, s2, n) (!)= 0 or strcmp (s1, s2) (!)= 0
>>>      if the result is ONLY used to do a simple equality test against zero, one of "s1" or "s2" is a small constant string, n is a constant, and the other non-constant string is guaranteed to not read beyond the end of the string:
>>>      change strncmp (s1, s2, n) or strcmp (s1, s2) to corresponding memcmp (s1, s2, n);
>> So how to you determine the non-constant string is long enough to avoid
>> reading past its end?  I guess you might get that from range
>> information.  But when it applies it seems reasoanble.  Again, this
>> could be considered a canonicalization step.
>
> In my current local implementation, I used the following routine to get the range info:  (and use the MINMAXLEN[1]+1 for the length of the non-constant string)
>
> /* Determine the minimum and maximum value or string length that ARG
>    refers to and store each in the first two elements of MINMAXLEN.
>    For expressions that point to strings of unknown lengths that are
>    character arrays, use the upper bound of the array as the maximum
>    length.  For example, given an expression like 'x ? array : "xyz"'
>    and array declared as 'char array[8]', MINMAXLEN[0] will be set
>    to 3 and MINMAXLEN[1] to 7, the longest string that could be
>    stored in array.
>    Return true if the range of the string lengths has been obtained
>    from the upper bound of an array at the end of a struct.  Such
>    an array may hold a string that's longer than its upper bound
>    due to it being used as a poor-man's flexible array member.  */
>
> bool
> get_range_strlen (tree arg, tree minmaxlen[2])
> {
> }
>
> However, this routine currently miss a very obvious case as the following:
>
> char s[100] = {'a','b','c','d’};
>
> __builtin_strcmp(s, "abc") != 0
>
> So, I have to change this routine to include such common case.

There was a discussion some time ago about converting CONSTRUCTOR
trees emitted for array initializers like the above to STRING_CST
(see bug 71625 for some background).  I think that would still be
the ideal solution.  Then you wouldn't have to change
get_range_strlen.

Martin