| Message ID | ri6v9h2k8x6.fsf@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | sra: Avoid SRAing if there is an aout-of-bounds access (PR 96820) | expand |
On Fri, 28 Aug 2020, Martin Jambor wrote: > Hi, > > the testcase causes and ICE in the SRA verifier on x86_64 when > compiling with -m32 because build_user_friendly_ref_for_offset looks > at an out-of-bounds array_ref within an array_ref which accesses an > offset which does not fit into a signed 32bit integer and turns it > into an array-ref with a negative index. > > The best thing is probably to bail out early when encountering an out > of bounds access to a local stack-allocated aggregate (and let the DSE > just delete such statements) which is what the patch does. > > I also glanced over to the initial candidate vetting routine to make > sure the size would fit into HWI and noticed that it uses unsigned > variants whereas the rest of SRA operates on signed offsets and > sizes (because get_ref_and_extent does) and so changed that for the > sake of consistency. These ancient checks operate on sizes of types > as opposed to DECLs but I hope that any issues potentially arising > from that are basically hypothetical. > > Bootstrapped and tested on x86_64-linux. OK for master and then for > gcc-10 branch? OK. Richard. > Thanks, > > Martin > > > gcc/ChangeLog: > > 2020-08-28 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/96820 > * tree-sra.c (create_access): Disqualify candidates with accesses > beyond the end of the original aggregate. > (maybe_add_sra_candidate): Check that candidate type size fits > signed uhwi for the sake of consistency. > > gcc/testsuite/ChangeLog: > > 2020-08-28 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/96820 > * gcc.dg/tree-ssa/pr96820.c: New test. > --- > gcc/testsuite/gcc.dg/tree-ssa/pr96820.c | 12 ++++++++++++ > gcc/tree-sra.c | 9 +++++++-- > 2 files changed, 19 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr96820.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr96820.c b/gcc/testsuite/gcc.dg/tree-ssa/pr96820.c > new file mode 100644 > index 00000000000..f5c2195f310 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr96820.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O1" } */ > + > +struct a { > + int b; > +}; > +int main() { > + struct a d[][6] = {4}; > + struct a e; > + d[1955249013][1955249013] = e; > + return e.b; > +} > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c > index 754f41302fc..98a6cacbe2a 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -941,6 +941,11 @@ create_access (tree expr, gimple *stmt, bool write) > disqualify_candidate (base, "Encountered an unconstrained access."); > return NULL; > } > + if (offset + size > tree_to_shwi (DECL_SIZE (base))) > + { > + disqualify_candidate (base, "Encountered an access beyond the base."); > + return NULL; > + } > > access = create_access_1 (base, offset, size); > access->expr = expr; > @@ -1880,12 +1885,12 @@ maybe_add_sra_candidate (tree var) > reject (var, "has incomplete type"); > return false; > } > - if (!tree_fits_uhwi_p (TYPE_SIZE (type))) > + if (!tree_fits_shwi_p (TYPE_SIZE (type))) > { > reject (var, "type size not fixed"); > return false; > } > - if (tree_to_uhwi (TYPE_SIZE (type)) == 0) > + if (tree_to_shwi (TYPE_SIZE (type)) == 0) > { > reject (var, "type size is zero"); > return false; >
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr96820.c b/gcc/testsuite/gcc.dg/tree-ssa/pr96820.c new file mode 100644 index 00000000000..f5c2195f310 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr96820.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O1" } */ + +struct a { + int b; +}; +int main() { + struct a d[][6] = {4}; + struct a e; + d[1955249013][1955249013] = e; + return e.b; +} diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 754f41302fc..98a6cacbe2a 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -941,6 +941,11 @@ create_access (tree expr, gimple *stmt, bool write) disqualify_candidate (base, "Encountered an unconstrained access."); return NULL; } + if (offset + size > tree_to_shwi (DECL_SIZE (base))) + { + disqualify_candidate (base, "Encountered an access beyond the base."); + return NULL; + } access = create_access_1 (base, offset, size); access->expr = expr; @@ -1880,12 +1885,12 @@ maybe_add_sra_candidate (tree var) reject (var, "has incomplete type"); return false; } - if (!tree_fits_uhwi_p (TYPE_SIZE (type))) + if (!tree_fits_shwi_p (TYPE_SIZE (type))) { reject (var, "type size not fixed"); return false; } - if (tree_to_uhwi (TYPE_SIZE (type)) == 0) + if (tree_to_shwi (TYPE_SIZE (type)) == 0) { reject (var, "type size is zero"); return false;