| Message ID | 87po1tmlka.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix tree-ssa-strlen handling of partial clobber (PR85814) | expand |
On 05/18/2018 02:09 AM, Richard Sandiford wrote: > In this PR we have: > > c_5 = c_4(D) + 4; > c_12 = c_5 + 1; > *c_5 = 2; > a = 2; // A > c_21 = c_12 + 1; > *c_12 = 2; > a = 2; // B > c_28 = c_21 + 1; > *c_21 = 2; > a = 2; > c_7 = c_28 + 1; > *c_28 = 2; > > where a is a global int. We decide that A can't clobber *c_5 == c_4[4] > because the latter implies that c_4 is an object of 5 bytes or more, > whereas a has exactly 4 bytes. > > The assumption for B and *c_5 is the same, but when considering B and > *c_12, we only follow the definition of c_12 to c_5 + 1 (for good > reason) and so have *c_12 == c_5[1]. We then don't have the same > size guarantee and so assume that B could clobber *c_12. This leads > to a situation in which the strinfo for c_5 is still valid but the > next strinfo (c_12) isn't. We then segfaulted while trying to get > the strinfo for c_21 + 1 == c_5 + 3 because get_stridx_plus_constant > assumed that c_5's next strinfo (c_12) would be valid too. > > And of course it should be valid really. It doesn't make sense for the > string based at c_5 to be valid but a substring of it to be invalid. > I don't think we can guarantee that such weird corner cases never > happen though, even if we tried to avoid this one. > > One possibility would be to mark c_12 as valid on the basis that c_5 > is valid, but I'm not sure the complication is worth it given that it > seems to trigger very rarely. A better optimisation would be to get > the unroller to clean up after itself a bit more... > > Although this particular instance of the bug relies on r249880, I think > we could have similar problems in GCC 7. It would be much harder to > trigger though, especially since it relies on unfolded IR like the above. > > Tested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu. > OK for trunk and GCC 8? > > Richard > > > 2018-05-18 Richard Sandiford <richard.sandiford@linaro.org> > > gcc/ > PR tree-optimization/85814 > * tree-ssa-strlen.c (get_stridx_plus_constant): Cope with > a null return from get_strinfo when unsharing the next > strinfo in the chain. > > gcc/testsuite/ > PR tree-optimization/85814 > * gcc.dg/torture/pr85814.c: New test. OK, including backports. jeff
Index: gcc/tree-ssa-strlen.c =================================================================== --- gcc/tree-ssa-strlen.c 2018-03-13 15:06:00.657514354 +0000 +++ gcc/tree-ssa-strlen.c 2018-05-18 09:05:44.691476297 +0100 @@ -795,9 +795,9 @@ get_stridx_plus_constant (strinfo *bases si = new_strinfo (ptr, idx, build_int_cst (size_type_node, nonzero_chars), basesi->full_string_p); set_strinfo (idx, si); - if (chainsi->next) + if (strinfo *nextsi = get_strinfo (chainsi->next)) { - strinfo *nextsi = unshare_strinfo (get_strinfo (chainsi->next)); + nextsi = unshare_strinfo (nextsi); si->next = nextsi->idx; nextsi->prev = idx; } Index: gcc/testsuite/gcc.dg/torture/pr85814.c =================================================================== --- /dev/null 2018-04-20 16:19:46.369131350 +0100 +++ gcc/testsuite/gcc.dg/torture/pr85814.c 2018-05-18 09:05:44.689476382 +0100 @@ -0,0 +1,7 @@ +int a; +void b(char *c) +{ + c += 4; + for (int i = 0; i < 4; i++) + a = *c++ = 2; +}