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 I've bootstrapped and regression tested this along with the other patches in the kit to update STRING_CST semantics. This re-fixes the pr87053 regression. Installed on the trunk.
Jeff
