| Message ID | 52950E33.3080209@mentor.com |
|---|---|
| State | New |
| Headers | show |
On 11/26/13 14:10, Tom de Vries wrote: > On 26-11-13 11:12, Richard Biener wrote: >> On Tue, Nov 26, 2013 at 8:57 AM, Tom de Vries <Tom_deVries@mentor.com> >> wrote: >>> Jason, >>> >>> This patch prevents creating out-of-bounds BIT_FIELD_REFs in 3 >>> locations. >>> >>> It fixes a SIGSEGV (triggered by gimple_fold_indirect_ref_1) in >>> simplify_bitfield_ref. I've added an assert to detect the problematic >>> BIT_FIELD_REF there. >>> >>> Bootstrapped and reg-tested on x86_64. >>> >>> OK for trunk? >> >> Looks obvious to me - btw, instead of asserting in tree-ssa-forwprop.c >> can you adjust the verify_expr BIT_FIELD_REF code so it checks for >> this? >> > > Done. > > And I've move the test-case to c-c++-common. > > Build and reg-tested on x86_64 (ada inclusive). Now redoing build and > test, but with bootstrap build. > > OK for trunk? Yes, OK for the trunk. jeff
On 27-11-13 07:20, Jeff Law wrote: > On 11/26/13 14:10, Tom de Vries wrote: >> On 26-11-13 11:12, Richard Biener wrote: >>> On Tue, Nov 26, 2013 at 8:57 AM, Tom de Vries <Tom_deVries@mentor.com> >>> wrote: >>>> Jason, >>>> >>>> This patch prevents creating out-of-bounds BIT_FIELD_REFs in 3 >>>> locations. >>>> >>>> It fixes a SIGSEGV (triggered by gimple_fold_indirect_ref_1) in >>>> simplify_bitfield_ref. I've added an assert to detect the problematic >>>> BIT_FIELD_REF there. >>>> >>>> Bootstrapped and reg-tested on x86_64. >>>> >>>> OK for trunk? >>> >>> Looks obvious to me - btw, instead of asserting in tree-ssa-forwprop.c >>> can you adjust the verify_expr BIT_FIELD_REF code so it checks for >>> this? >>> >> >> Done. >> >> And I've move the test-case to c-c++-common. >> >> Build and reg-tested on x86_64 (ada inclusive). Now redoing build and >> test, but with bootstrap build. >> >> OK for trunk? > Yes, OK for the trunk. > Committed to trunk. Also ok for 4.8 branch? It's a 4.8/4.9 regression. Thanks, - Tom > jeff >
On Thu, Nov 28, 2013 at 12:23 AM, Tom de Vries <Tom_deVries@mentor.com> wrote: > On 27-11-13 07:20, Jeff Law wrote: >> >> On 11/26/13 14:10, Tom de Vries wrote: >>> >>> On 26-11-13 11:12, Richard Biener wrote: >>>> >>>> On Tue, Nov 26, 2013 at 8:57 AM, Tom de Vries <Tom_deVries@mentor.com> >>>> wrote: >>>>> >>>>> Jason, >>>>> >>>>> This patch prevents creating out-of-bounds BIT_FIELD_REFs in 3 >>>>> locations. >>>>> >>>>> It fixes a SIGSEGV (triggered by gimple_fold_indirect_ref_1) in >>>>> simplify_bitfield_ref. I've added an assert to detect the problematic >>>>> BIT_FIELD_REF there. >>>>> >>>>> Bootstrapped and reg-tested on x86_64. >>>>> >>>>> OK for trunk? >>>> >>>> >>>> Looks obvious to me - btw, instead of asserting in tree-ssa-forwprop.c >>>> can you adjust the verify_expr BIT_FIELD_REF code so it checks for >>>> this? >>>> >>> >>> Done. >>> >>> And I've move the test-case to c-c++-common. >>> >>> Build and reg-tested on x86_64 (ada inclusive). Now redoing build and >>> test, but with bootstrap build. >>> >>> OK for trunk? >> >> Yes, OK for the trunk. >> > > Committed to trunk. > > Also ok for 4.8 branch? It's a 4.8/4.9 regression. Ok if testing succeeds there but please leave out the checking bits. Thanks, Richard. > Thanks, > - Tom > >> jeff >> >
> Ok if testing succeeds there but please leave out the checking bits.
Yes, they (perhaps unsurprisingly) trigger in Ada, I'll investigate.
On Thu, Nov 28, 2013 at 4:39 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> Ok if testing succeeds there but please leave out the checking bits. > > Yes, they (perhaps unsurprisingly) trigger in Ada, I'll investigate. PR59338, I have a fix in testing (restrict checking to non-aggregates). Richard. > -- > Eric Botcazou
> PR59338, I have a fix in testing (restrict checking to non-aggregates).
This will be sufficient for Ada I think, this occurs only when downcasting
because of the infamous VIEW_CONVERT_EXPRs used to implement it (yes, the ones
which carry the TYPE_ALIGN_OK flag).
On Thu, Nov 28, 2013 at 12:23:43AM +0100, Tom de Vries wrote: > Committed to trunk. > > Also ok for 4.8 branch? It's a 4.8/4.9 regression. Ok, but I guess you need to adjust your patch for 4.8 (tree_to_* and tree_fits_* to host_integerp/tree_low_cst), so please make sure you test it before commiting. Jakub
2013-11-25 Tom de Vries <tom@codesourcery.com> Marc Glisse <marc.glisse@inria.fr> PR middle-end/59037 * semantics.c (cxx_fold_indirect_ref): Don't create out-of-bounds BIT_FIELD_REF. * fold-const.c (fold_indirect_ref_1): Don't create out-of-bounds BIT_FIELD_REF. * gimple-fold.c (gimple_fold_indirect_ref): Same. * tree-cfg.c (verify_expr): Give error if BIT_FIELD_REF is out-of-bounds. * c-c++-common/pr59037.c: New testcase. diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 316834c..71daaa2 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -9117,7 +9117,7 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base) unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT; tree index = bitsize_int (indexi); - if (offset/part_widthi <= TYPE_VECTOR_SUBPARTS (op00type)) + if (offset / part_widthi < TYPE_VECTOR_SUBPARTS (op00type)) return fold_build3_loc (loc, BIT_FIELD_REF, type, op00, part_width, index); diff --git a/gcc/fold-const.c b/gcc/fold-const.c index f91673d..0926626 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -16719,7 +16719,7 @@ fold_indirect_ref_1 (location_t loc, tree type, tree op0) unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT; tree index = bitsize_int (indexi); - if (offset/part_widthi <= TYPE_VECTOR_SUBPARTS (op00type)) + if (offset / part_widthi < TYPE_VECTOR_SUBPARTS (op00type)) return fold_build3_loc (loc, BIT_FIELD_REF, type, op00, part_width, index); diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 2902e69..7e9ba65 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -3418,7 +3418,7 @@ gimple_fold_indirect_ref (tree t) unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT; tree index = bitsize_int (indexi); if (offset / part_widthi - <= TYPE_VECTOR_SUBPARTS (TREE_TYPE (addrtype))) + < TYPE_VECTOR_SUBPARTS (TREE_TYPE (addrtype))) return fold_build3 (BIT_FIELD_REF, type, TREE_OPERAND (addr, 0), part_width, index); } diff --git a/gcc/testsuite/c-c++-common/pr59037.c b/gcc/testsuite/c-c++-common/pr59037.c new file mode 100644 index 0000000..fae13c2 --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr59037.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O3" } */ + +typedef int v4si __attribute__ ((vector_size (16))); + +int +main (int argc, char** argv) +{ + v4si x = {0,1,2,3}; + x = (v4si) {(x)[3], (x)[2], (x)[1], (x)[0]}; + return x[4]; +} diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index ab4bb09..f8937c6 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -2712,15 +2712,29 @@ verify_expr (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED) if (TREE_CODE (t) == BIT_FIELD_REF) { - if (!tree_fits_uhwi_p (TREE_OPERAND (t, 1)) - || !tree_fits_uhwi_p (TREE_OPERAND (t, 2))) + tree t0 = TREE_OPERAND (t, 0); + tree t1 = TREE_OPERAND (t, 1); + tree t2 = TREE_OPERAND (t, 2); + tree t0_type = TREE_TYPE (t0); + unsigned HOST_WIDE_INT t0_size = 0; + + if (tree_fits_uhwi_p (TYPE_SIZE (t0_type))) + t0_size = tree_to_uhwi (TYPE_SIZE (t0_type)); + else + { + HOST_WIDE_INT t0_max_size = max_int_size_in_bytes (t0_type); + if (t0_max_size > 0) + t0_size = t0_max_size * BITS_PER_UNIT; + } + if (!tree_fits_uhwi_p (t1) + || !tree_fits_uhwi_p (t2)) { error ("invalid position or size operand to BIT_FIELD_REF"); return t; } if (INTEGRAL_TYPE_P (TREE_TYPE (t)) && (TYPE_PRECISION (TREE_TYPE (t)) - != tree_to_uhwi (TREE_OPERAND (t, 1)))) + != tree_to_uhwi (t1))) { error ("integral result type precision does not match " "field size of BIT_FIELD_REF"); @@ -2729,12 +2743,19 @@ verify_expr (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED) else if (!INTEGRAL_TYPE_P (TREE_TYPE (t)) && TYPE_MODE (TREE_TYPE (t)) != BLKmode && (GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (t))) - != tree_to_uhwi (TREE_OPERAND (t, 1)))) + != tree_to_uhwi (t1))) { error ("mode precision of non-integral result does not " "match field size of BIT_FIELD_REF"); return t; } + if (t0_size != 0 + && tree_to_uhwi (t1) + tree_to_uhwi (t2) > t0_size) + { + error ("position plus size exceeds size of referenced object in " + "BIT_FIELD_REF"); + return t; + } } t = TREE_OPERAND (t, 0);