* Fix tree-ssa-strlen handling of partial clobber (PR85814)
@ 2018-05-18 8:41 Richard Sandiford
2018-05-21 20:27 ` Jeff Law
0 siblings, 1 reply; 2+ messages in thread
From: Richard Sandiford @ 2018-05-18 8:41 UTC (permalink / raw)
To: gcc-patches
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.
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;
+}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Fix tree-ssa-strlen handling of partial clobber (PR85814)
2018-05-18 8:41 Fix tree-ssa-strlen handling of partial clobber (PR85814) Richard Sandiford
@ 2018-05-21 20:27 ` Jeff Law
0 siblings, 0 replies; 2+ messages in thread
From: Jeff Law @ 2018-05-21 20:27 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-05-21 20:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-18 8:41 Fix tree-ssa-strlen handling of partial clobber (PR85814) Richard Sandiford
2018-05-21 20:27 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).