* [PATCH] sra: Bail out when encountering accesses with negative offsets (PR 96730)
@ 2020-08-24 15:07 Martin Jambor
2020-08-24 18:30 ` Richard Biener
2020-08-24 22:59 ` Jeff Law
0 siblings, 2 replies; 3+ messages in thread
From: Martin Jambor @ 2020-08-24 15:07 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Biener
Hi,
I must admit I was quite surprised to see that SRA does not disqualify
an aggregate from any transformations when it encounters an offset for
which get_ref_base_and_extent returns a negative offset. It may not
matter too much because I sure hope such programs always have
undefined behavior (SRA candidates are local variables on stack) but
it is probably better not to perform weird transformations on them as
build ref model with the new build_reconstructed_reference function
currently happily do for negative offsets (they just copy the existing
expression which is then used as the expression of a "propagated"
access) and of course the compiler must not ICE (as it currently does
because the SRA forest verifier does not like the expression).
Fixed with the following patch which also passed bootstrap and testing
on an x86_64-linux. OK for master and later on for the gcc-10 branch?
Thanks,
Martin
gcc/ChangeLog:
2020-08-24 Martin Jambor <mjambor@suse.cz>
PR tree-optimization/96730
* tree-sra.c (create_access): Disqualify any aggregate with negative
offset access.
(build_ref_for_model): Add assert that offset is non-negative.
gcc/testsuite/ChangeLog:
2020-08-24 Martin Jambor <mjambor@suse.cz>
PR tree-optimization/96730
* gcc.dg/tree-ssa/pr96730.c: New test.
---
gcc/testsuite/gcc.dg/tree-ssa/pr96730.c | 13 +++++++++++++
gcc/tree-sra.c | 6 ++++++
2 files changed, 19 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr96730.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr96730.c b/gcc/testsuite/gcc.dg/tree-ssa/pr96730.c
new file mode 100644
index 00000000000..39a06846529
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr96730.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+struct a {
+ int b;
+ int c;
+} d() {
+ struct a e[9];
+ int f = 3362953455;
+ e[f] = e[6];
+ e[6].c = 1;
+}
+int main() {}
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index fcba7fbdd31..754f41302fc 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -931,6 +931,11 @@ create_access (tree expr, gimple *stmt, bool write)
}
if (size == 0)
return NULL;
+ if (offset < 0)
+ {
+ disqualify_candidate (base, "Encountered a negative offset access.");
+ return NULL;
+ }
if (size < 0)
{
disqualify_candidate (base, "Encountered an unconstrained access.");
@@ -1667,6 +1672,7 @@ build_ref_for_model (location_t loc, tree base, HOST_WIDE_INT offset,
struct access *model, gimple_stmt_iterator *gsi,
bool insert_after)
{
+ gcc_assert (offset >= 0);
if (TREE_CODE (model->expr) == COMPONENT_REF
&& DECL_BIT_FIELD (TREE_OPERAND (model->expr, 1)))
{
--
2.28.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sra: Bail out when encountering accesses with negative offsets (PR 96730)
2020-08-24 15:07 [PATCH] sra: Bail out when encountering accesses with negative offsets (PR 96730) Martin Jambor
@ 2020-08-24 18:30 ` Richard Biener
2020-08-24 22:59 ` Jeff Law
1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2020-08-24 18:30 UTC (permalink / raw)
To: Martin Jambor, GCC Patches
On August 24, 2020 5:07:56 PM GMT+02:00, Martin Jambor <mjambor@suse.cz> wrote:
>Hi,
>
>I must admit I was quite surprised to see that SRA does not disqualify
>an aggregate from any transformations when it encounters an offset for
>which get_ref_base_and_extent returns a negative offset. It may not
>matter too much because I sure hope such programs always have
>undefined behavior (SRA candidates are local variables on stack) but
>it is probably better not to perform weird transformations on them as
>build ref model with the new build_reconstructed_reference function
>currently happily do for negative offsets (they just copy the existing
>expression which is then used as the expression of a "propagated"
>access) and of course the compiler must not ICE (as it currently does
>because the SRA forest verifier does not like the expression).
>
>Fixed with the following patch which also passed bootstrap and testing
>on an x86_64-linux. OK for master and later on for the gcc-10 branch?
OK.
Richard.
>Thanks,
>
>Martin
>
>
>gcc/ChangeLog:
>
>2020-08-24 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/96730
> * tree-sra.c (create_access): Disqualify any aggregate with negative
> offset access.
> (build_ref_for_model): Add assert that offset is non-negative.
>
>gcc/testsuite/ChangeLog:
>
>2020-08-24 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/96730
> * gcc.dg/tree-ssa/pr96730.c: New test.
>---
> gcc/testsuite/gcc.dg/tree-ssa/pr96730.c | 13 +++++++++++++
> gcc/tree-sra.c | 6 ++++++
> 2 files changed, 19 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr96730.c
>
>diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr96730.c
>b/gcc/testsuite/gcc.dg/tree-ssa/pr96730.c
>new file mode 100644
>index 00000000000..39a06846529
>--- /dev/null
>+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr96730.c
>@@ -0,0 +1,13 @@
>+/* { dg-do compile } */
>+/* { dg-options "-O1" } */
>+
>+struct a {
>+ int b;
>+ int c;
>+} d() {
>+ struct a e[9];
>+ int f = 3362953455;
>+ e[f] = e[6];
>+ e[6].c = 1;
>+}
>+int main() {}
>diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
>index fcba7fbdd31..754f41302fc 100644
>--- a/gcc/tree-sra.c
>+++ b/gcc/tree-sra.c
>@@ -931,6 +931,11 @@ create_access (tree expr, gimple *stmt, bool
>write)
> }
> if (size == 0)
> return NULL;
>+ if (offset < 0)
>+ {
>+ disqualify_candidate (base, "Encountered a negative offset
>access.");
>+ return NULL;
>+ }
> if (size < 0)
> {
> disqualify_candidate (base, "Encountered an unconstrained access.");
>@@ -1667,6 +1672,7 @@ build_ref_for_model (location_t loc, tree base,
>HOST_WIDE_INT offset,
> struct access *model, gimple_stmt_iterator *gsi,
> bool insert_after)
> {
>+ gcc_assert (offset >= 0);
> if (TREE_CODE (model->expr) == COMPONENT_REF
> && DECL_BIT_FIELD (TREE_OPERAND (model->expr, 1)))
> {
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sra: Bail out when encountering accesses with negative offsets (PR 96730)
2020-08-24 15:07 [PATCH] sra: Bail out when encountering accesses with negative offsets (PR 96730) Martin Jambor
2020-08-24 18:30 ` Richard Biener
@ 2020-08-24 22:59 ` Jeff Law
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Law @ 2020-08-24 22:59 UTC (permalink / raw)
To: Martin Jambor, GCC Patches; +Cc: Richard Biener
On Mon, 2020-08-24 at 17:07 +0200, Martin Jambor wrote:
> Hi,
>
> I must admit I was quite surprised to see that SRA does not disqualify
> an aggregate from any transformations when it encounters an offset for
> which get_ref_base_and_extent returns a negative offset. It may not
> matter too much because I sure hope such programs always have
> undefined behavior (SRA candidates are local variables on stack) but
> it is probably better not to perform weird transformations on them as
> build ref model with the new build_reconstructed_reference function
> currently happily do for negative offsets (they just copy the existing
> expression which is then used as the expression of a "propagated"
> access) and of course the compiler must not ICE (as it currently does
> because the SRA forest verifier does not like the expression).
>
> Fixed with the following patch which also passed bootstrap and testing
> on an x86_64-linux. OK for master and later on for the gcc-10 branch?
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2020-08-24 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/96730
> * tree-sra.c (create_access): Disqualify any aggregate with negative
> offset access.
> (build_ref_for_model): Add assert that offset is non-negative.
>
> gcc/testsuite/ChangeLog:
>
> 2020-08-24 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/96730
> * gcc.dg/tree-ssa/pr96730.c: New test.
OK
jeff
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-24 22:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-24 15:07 [PATCH] sra: Bail out when encountering accesses with negative offsets (PR 96730) Martin Jambor
2020-08-24 18:30 ` Richard Biener
2020-08-24 22:59 ` 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).