On 9/13/18 7:30 AM, Bernd Edlinger wrote:
> On 08/31/18 19:45, Bernd Edlinger wrote:
>> On 08/31/18 16:42, Jeff Law wrote:
>>> On 08/31/2018 01:08 AM, Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>>
>>>> when re-testing the new STRING_CST semantic patch here:
>>>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html
>>>>
>>>> I noticed that the (my) fix for PR 87053 does still use
>>>> semantic properties of the STRING_CST that is not compatible
>>>> with the new proposed STRING_CST semantics.
>>>>
>>>> That means that c_strlen needs to handle the case
>>>> where strings are not zero terminated.
>>>>
>>>> When this is fixed, fortran.dg/pr45636.f90 starts to regress,
>>>> because the check in gimple_fold_builtin_memory_op here:
>>>>
>>>>         if (tree_fits_uhwi_p (len)
>>>>             && compare_tree_int (len, MOVE_MAX) <= 0
>>>>             /* ???  Don't transform copies from strings with known length 
>>>> this
>>>>                confuses the tree-ssa-strlen.c.  This doesn't handle
>>>>                the case in gcc.dg/strlenopt-8.c which is XFAILed for that
>>>>                reason.  */
>>>>             && !c_strlen (src, 2))
>>>> does now return NULL_TREE, because the fortran string is not null 
>>>> terminated.
>>>> However that allows the folding which makes the fortran test case fail.
>>>>
>>>> I fixed that by pulling in the c_getstr patch again, and use
>>>> it to make another exception for string constants without any embedded NUL.
>>>> That is what tree-ssa-forwprop.c can handle, and should make this work in
>>>> fortran again.
>>>>
>>>>
>>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>>> Is it OK for trunk?
>>> I've gone the rounds on pr45636 a couple times and it's part of the
>>> reason why there's a FIXME in the bits I committed earlier this week.
>>>
>> Yes, the rationale here is that tree-ssa-forwprop will likely choke if there
>> is a NUL byte in the string:
>>
>>            /* Neither builtin_strncpy_read_str nor builtin_memcpy_read_str
>>               handle embedded '\0's.  */
>>            if (strlen (src_buf) != src_len)
>>              break;
>>
>> Prior to this the c_strlen (src, 2) returned 2, thus this folding was not 
>> done,
>> but since the string does not contain any NUL bytes this returns now NULL.
>> However it is easy to add an exception that triggers only in a fortran string
>> in this way.
>>
>>> I'll look at this closely in conjunction with the (unposted) patch which
>>> addresses the FIXME.
>>>
>>> Jeff
>>>
> Hi,
> 
> this is a minor update to the previous patch version, in that it adds
> the following to c_getstr, in order to be bootstrapped with or without
> the other STRING_CST-v2 semantic patches:
> 
> @@ -14611,6 +14611,10 @@ c_getstr (tree src, unsigned HOST_WIDE_I
>     unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src);
>     unsigned HOST_WIDE_INT string_size = tree_to_uhwi (mem_size);
>   
> +  /* Ideally this would turn into a gcc_checking_assert over time.  */
> +  if (string_length > string_size)
> +    string_length = string_size;
> +
>     const char *string = TREE_STRING_POINTER (src);
>   
>     if (string_length == 0
> 
> 
> So this patch can be boot-strapped alone,
> or together with the following patches:
> 
> [PATCHv2] Handle overlength strings in the C FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html
> 
> [PATCHv2] Handle overlength strings in C++ FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html
> => Apporoved, without the part in vtable-class-hierarchy.c (!)
> 
> [PATCHv2] Handle overlength string literals in the fortan FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01568.html
> => Approved.
> 
> [PATCH] Check the STRING_CSTs in varasm.c
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html
> 
> 
> 
> 
> Bernd.
> 
> 
> patch-c-strlen-v2.diff
> 
> 2018-08-30  Bernd Edlinger  <bernd.edlin...@hotmail.de>
> 
>       * builtins.c (c_strlen): Handle not zero terminated STRING_CSTs
>       correctly.
>       * fold-const.c (c_getstr): Fix function comment.  Remove unused third
>       argument.  Fix range checks.
>       * fold-const.h (c_getstr): Adjust protoype.
>       * gimple-fold.c (gimple_fold_builtin_memory_op): Avoid folding when
>       string is constant but contains no NUL byte.
And I've committed the rest of this patch (a piece of the fold-const.c
changes were already committed).

jeff

Reply via email to