From 9c8c7338585ca99db181ce0d1a63654f97beeed0 Mon Sep 17 00:00:00 2001 From: Richard Sandiford Date: Mon, 21 May 2018 22:02:35 +0000 Subject: [PATCH] Fix tree-ssa-strlen handling of partial clobbers (PR85814) 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. 2018-05-21 Richard Sandiford 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. From-SVN: r260488 --- gcc/ChangeLog | 7 +++++++ gcc/testsuite/ChangeLog | 5 +++++ gcc/testsuite/gcc.dg/torture/pr85814.c | 7 +++++++ gcc/tree-ssa-strlen.c | 4 ++-- 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr85814.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 65551262743..db367f758d0 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2018-05-21 Richard Sandiford + + 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. + 2018-05-21 Vladimir Mezentsev PR gcc/84923 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 3a669d78c2a..10b757272c5 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-05-21 Richard Sandiford + + PR tree-optimization/85814 + * gcc.dg/torture/pr85814.c: New test. + 2018-05-21 Paolo Carlini PR c++/84588 diff --git a/gcc/testsuite/gcc.dg/torture/pr85814.c b/gcc/testsuite/gcc.dg/torture/pr85814.c new file mode 100644 index 00000000000..8d16c53e9c0 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr85814.c @@ -0,0 +1,7 @@ +int a; +void b(char *c) +{ + c += 4; + for (int i = 0; i < 4; i++) + a = *c++ = 2; +} diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c index 33004b6870d..556c5bc29fe 100644 --- a/gcc/tree-ssa-strlen.c +++ b/gcc/tree-ssa-strlen.c @@ -795,9 +795,9 @@ get_stridx_plus_constant (strinfo *basesi, unsigned HOST_WIDE_INT off, 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; } -- 2.30.2