| Message ID | ri68se4kroz.fsf@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | sra: Bail out when encountering accesses with negative offsets (PR 96730) | expand |
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))) > {
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 >
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))) {